Move file-reading into walker loop

Author: Mark-Simulacrum

Diff for: src/tools/tidy/src/bins.rs

@@ -25,9 +25,9 @@ pub fn check(path: &Path, bad: &mut bool) {
         }
     }
 
-    super::walk(path,
+    super::walk_no_read(path,
                 &mut |path| super::filter_dirs(path) || path.ends_with("src/etc"),
-                &mut |entry, _contents| {
+                &mut |entry| {
         let file = entry.path();
         let filename = file.file_name().unwrap().to_string_lossy();
         let extensions = [".py", ".sh"];

Diff for: src/tools/tidy/src/errors.rs

@@ -4,25 +4,19 @@
 //! statistics about the error codes.
 
 use std::collections::HashMap;
-use std::fs::File;
-use std::io::prelude::*;
 use std::path::Path;
 
 pub fn check(path: &Path, bad: &mut bool) {
-    let mut contents = String::new();
     let mut map: HashMap<_, Vec<_>> = HashMap::new();
     super::walk(path,
                 &mut |path| super::filter_dirs(path) || path.ends_with("src/test"),
-                &mut |entry, _contents| {
+                &mut |entry, contents| {
         let file = entry.path();
         let filename = file.file_name().unwrap().to_string_lossy();
         if filename != "error_codes.rs" {
             return
         }
 
-        contents.truncate(0);
-        t!(t!(File::open(file)).read_to_string(&mut contents));
-
         // In the `register_long_diagnostics!` macro, entries look like this:
         //
         // ```

Diff for: src/tools/tidy/src/features.rs

@@ -11,8 +11,7 @@
 
 use std::collections::HashMap;
 use std::fmt;
-use std::fs::{self, File};
-use std::io::prelude::*;
+use std::fs;
 use std::path::Path;
 
 use regex::Regex;
@@ -58,13 +57,11 @@ pub fn check(path: &Path, bad: &mut bool, verbose: bool) {
     let lib_features = get_and_check_lib_features(path, bad, &features);
     assert!(!lib_features.is_empty());
 
-    let mut contents = String::new();
-
     super::walk_many(&[&path.join("test/ui"),
                        &path.join("test/ui-fulldeps"),
                        &path.join("test/compile-fail")],
                      &mut |path| super::filter_dirs(path),
-                     &mut |entry, _contents| {
+                     &mut |entry, contents| {
         let file = entry.path();
         let filename = file.file_name().unwrap().to_string_lossy();
         if !filename.ends_with(".rs") || filename == "features.rs" ||
@@ -75,9 +72,6 @@ pub fn check(path: &Path, bad: &mut bool, verbose: bool) {
         let filen_underscore = filename.replace('-',"_").replace(".rs","");
         let filename_is_gate_test = test_filen_gate(&filen_underscore, &mut features);
 
-        contents.truncate(0);
-        t!(t!(File::open(&file), &file).read_to_string(&mut contents));
-
         for (i, line) in contents.lines().enumerate() {
             let mut err = |msg: &str| {
                 tidy_error!(bad, "{}:{}: {}", file.display(), i + 1, msg);
@@ -369,20 +363,16 @@ fn get_and_check_lib_features(base_src_path: &Path,
 
 fn map_lib_features(base_src_path: &Path,
                     mf: &mut dyn FnMut(Result<(&str, Feature), &str>, &Path, usize)) {
-    let mut contents = String::new();
     super::walk(base_src_path,
                 &mut |path| super::filter_dirs(path) || path.ends_with("src/test"),
-                &mut |entry, _contents| {
+                &mut |entry, contents| {
         let file = entry.path();
         let filename = file.file_name().unwrap().to_string_lossy();
         if !filename.ends_with(".rs") || filename == "features.rs" ||
            filename == "diagnostic_list.rs" {
             return;
         }
 
-        contents.truncate(0);
-        t!(t!(File::open(&file), &file).read_to_string(&mut contents));
-
         let mut becoming_feature: Option<(String, Feature)> = None;
         for (i, line) in contents.lines().enumerate() {
             macro_rules! err {

Diff for: src/tools/tidy/src/lib.rs

@@ -67,6 +67,7 @@ fn filter_dirs(path: &Path) -> bool {
     skip.iter().any(|p| path.ends_with(p))
 }
 
+
 fn walk_many(
     paths: &[&Path], skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str)
 ) {
@@ -76,19 +77,25 @@ fn walk_many(
 }
 
 fn walk(path: &Path, skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str)) {
+    let mut contents = String::new();
+    walk_no_read(path, skip, &mut |entry| {
+        contents.clear();
+        if t!(File::open(entry.path()), entry.path()).read_to_string(&mut contents).is_err() {
+            contents.clear();
+        }
+        f(&entry, &contents);
+    });
+}
+
+fn walk_no_read(path: &Path, skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&DirEntry)) {
     let walker = WalkDir::new(path).into_iter()
         .filter_entry(|e| !skip(e.path()));
-    let mut contents = String::new();
     for entry in walker {
         if let Ok(entry) = entry {
             if entry.file_type().is_dir() {
                 continue;
             }
-            contents.clear();
-            if t!(File::open(entry.path()), entry.path()).read_to_string(&mut contents).is_err() {
-                contents.clear();
-            }
-            f(&entry, &contents);
+            f(&entry);
         }
     }
 }

Diff for: src/tools/tidy/src/libcoretest.rs

@@ -4,30 +4,22 @@
 //! item. All tests must be written externally in `libcore/tests`.
 
 use std::path::Path;
-use std::fs::read_to_string;
 
 pub fn check(path: &Path, bad: &mut bool) {
     let libcore_path = path.join("libcore");
     super::walk(
         &libcore_path,
         &mut |subpath| t!(subpath.strip_prefix(&libcore_path)).starts_with("tests"),
-        &mut |entry, _contents| {
+        &mut |entry, contents| {
             let subpath = entry.path();
             if let Some("rs") = subpath.extension().and_then(|e| e.to_str()) {
-                match read_to_string(subpath) {
-                    Ok(contents) => {
-                        if contents.contains("#[test]") {
-                            tidy_error!(
-                                bad,
-                                "{} contains #[test]; libcore tests must be placed inside \
-                                `src/libcore/tests/`",
-                                subpath.display()
-                            );
-                        }
-                    }
-                    Err(err) => {
-                        panic!("failed to read file {:?}: {}", subpath, err);
-                    }
+                if contents.contains("#[test]") {
+                    tidy_error!(
+                        bad,
+                        "{} contains #[test]; libcore tests must be placed inside \
+                        `src/libcore/tests/`",
+                        subpath.display()
+                    );
                 }
             }
         },

Diff for: src/tools/tidy/src/pal.rs

@@ -31,8 +31,6 @@
 //! platform-specific cfgs are allowed. Not sure yet how to deal with
 //! this in the long term.
 
-use std::fs::File;
-use std::io::Read;
 use std::path::Path;
 use std::iter::Iterator;
 
@@ -87,30 +85,26 @@ const EXCEPTION_PATHS: &[&str] = &[
 ];
 
 pub fn check(path: &Path, bad: &mut bool) {
-    let mut contents = String::new();
     // Sanity check that the complex parsing here works.
     let mut saw_target_arch = false;
     let mut saw_cfg_bang = false;
-    super::walk(path, &mut super::filter_dirs, &mut |entry, _contents| {
+    super::walk(path, &mut super::filter_dirs, &mut |entry, contents| {
         let file = entry.path();
         let filestr = file.to_string_lossy().replace("\\", "/");
         if !filestr.ends_with(".rs") { return }
 
         let is_exception_path = EXCEPTION_PATHS.iter().any(|s| filestr.contains(&**s));
         if is_exception_path { return }
 
-        check_cfgs(&mut contents, &file, bad, &mut saw_target_arch, &mut saw_cfg_bang);
+        check_cfgs(contents, &file, bad, &mut saw_target_arch, &mut saw_cfg_bang);
     });
 
     assert!(saw_target_arch);
     assert!(saw_cfg_bang);
 }
 
-fn check_cfgs(contents: &mut String, file: &Path,
+fn check_cfgs(contents: &str, file: &Path,
               bad: &mut bool, saw_target_arch: &mut bool, saw_cfg_bang: &mut bool) {
-    contents.truncate(0);
-    t!(t!(File::open(file), file).read_to_string(contents));
-
     // For now it's ok to have platform-specific code after 'mod tests'.
     let mod_tests_idx = find_test_mod(contents);
     let contents = &contents[..mod_tests_idx];

Diff for: src/tools/tidy/src/style.rs

@@ -13,8 +13,6 @@
 //! A number of these checks can be opted-out of with various directives of the form:
 //! `// ignore-tidy-CHECK-NAME`.
 
-use std::fs::File;
-use std::io::prelude::*;
 use std::path::Path;
 
 const COLS: usize = 100;
@@ -109,7 +107,7 @@ enum Directive {
     Ignore(bool),
 }
 
-fn contains_ignore_directive(contents: &String, check: &str) -> Directive {
+fn contains_ignore_directive(contents: &str, check: &str) -> Directive {
     if contents.contains(&format!("// ignore-tidy-{}", check)) ||
         contents.contains(&format!("# ignore-tidy-{}", check)) {
         Directive::Ignore(false)
@@ -129,8 +127,7 @@ macro_rules! suppressible_tidy_err {
 }
 
 pub fn check(path: &Path, bad: &mut bool) {
-    let mut contents = String::new();
-    super::walk(path, &mut super::filter_dirs, &mut |entry, _contents| {
+    super::walk(path, &mut super::filter_dirs, &mut |entry, contents| {
         let file = entry.path();
         let filename = file.file_name().unwrap().to_string_lossy();
         let extensions = [".rs", ".py", ".js", ".sh", ".c", ".cpp", ".h"];
@@ -139,9 +136,6 @@ pub fn check(path: &Path, bad: &mut bool) {
             return
         }
 
-        contents.truncate(0);
-        t!(t!(File::open(file), file).read_to_string(&mut contents));
-
         if contents.is_empty() {
             tidy_error!(bad, "{}: empty file", file.display());
         }

Diff for: src/tools/tidy/src/ui_tests.rs

@@ -4,10 +4,8 @@ use std::fs;
 use std::path::Path;
 
 pub fn check(path: &Path, bad: &mut bool) {
-    super::walk_many(
-        &[&path.join("test/ui"), &path.join("test/ui-fulldeps")],
-        &mut |_| false,
-        &mut |entry, _contents| {
+    for path in &[&path.join("test/ui"), &path.join("test/ui-fulldeps")] {
+        super::walk_no_read(path, &mut |_| false, &mut |entry| {
             let file_path = entry.path();
             if let Some(ext) = file_path.extension() {
                 if ext == "stderr" || ext == "stdout" {
@@ -46,6 +44,6 @@ pub fn check(path: &Path, bad: &mut bool) {
                     }
                 }
             }
-        },
-    );
+        });
+    }
 }