tidy: increase performance of auto extra checks feature

Author: lolbinarycat

src/tools/tidy/src/extra_checks/mod.rs

@@ -86,7 +86,7 @@ fn check_impl(
         std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("true") || v == "1");
 
     // Split comma-separated args up
-    let lint_args = match extra_checks {
+    let mut lint_args = match extra_checks {
         Some(s) => s
             .strip_prefix("--extra-checks=")
             .unwrap()
@@ -99,11 +99,7 @@ fn check_impl(
             })
             .filter_map(|(res, src)| match res {
                 Ok(arg) => {
-                    if arg.is_inactive_auto(ci_info) {
-                        None
-                    } else {
-                        Some(arg)
-                    }
+                    Some(arg)
                 }
                 Err(err) => {
                     // only warn because before bad extra checks would be silently ignored.
@@ -114,6 +110,11 @@ fn check_impl(
             .collect(),
         None => vec![],
     };
+    if lint_args.iter().any(|ck| ck.auto) {
+        crate::files_modified_batch_filter(ci_info, &mut lint_args, |ck, path| {
+            ck.is_non_auto_or_matches(path)
+        });
+    }
 
     macro_rules! extra_check {
         ($lang:ident, $kind:ident) => {
@@ -721,23 +722,26 @@ impl ExtraCheckArg {
         self.lang == lang && self.kind.map(|k| k == kind).unwrap_or(true)
     }
 
-    /// Returns `true` if this is an auto arg and the relevant files are not modified.
-    fn is_inactive_auto(&self, ci_info: &CiInfo) -> bool {
+    /// Returns `false` if this is an auto arg and the passed filename does not trigger the auto rule
+    fn is_non_auto_or_matches(&self, filepath: &str) -> bool {
         if !self.auto {
-            return false;
+            return true;
         }
         let ext = match self.lang {
             ExtraCheckLang::Py => ".py",
             ExtraCheckLang::Cpp => ".cpp",
             ExtraCheckLang::Shell => ".sh",
             ExtraCheckLang::Js => ".js",
             ExtraCheckLang::Spellcheck => {
-                return !crate::files_modified(ci_info, |s| {
-                    SPELLCHECK_DIRS.iter().any(|dir| Path::new(s).starts_with(dir))
-                });
+                for dir in SPELLCHECK_DIRS {
+                    if Path::new(filepath).starts_with(dir) {
+                        return true;
+                    }
+                }
+                return false;
             }
         };
-        !crate::files_modified(ci_info, |s| s.ends_with(ext))
+        filepath.ends_with(ext)
     }
 
     fn has_supported_kind(&self) -> bool {

src/tools/tidy/src/lib.rs

@@ -125,40 +125,62 @@ pub fn git_diff<S: AsRef<OsStr>>(base_commit: &str, extra_arg: S) -> Option<Stri
     Some(String::from_utf8_lossy(&output.stdout).into())
 }
 
-/// Returns true if any modified file matches the predicate, if we are in CI, or if unable to list modified files.
-pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool {
+/// Similar to `files_modified`, but only involves a single call to `git`.
+///
+/// removes all elements from `items` that do not cause any match when `pred` is called with the list of modifed files.
+///
+/// if in CI, no elements will be removed.
+pub fn files_modified_batch_filter<T>(
+    ci_info: &CiInfo,
+    items: &mut Vec<T>,
+    pred: impl Fn(&T, &str) -> bool,
+) {
     if CiEnv::is_ci() {
         // assume everything is modified on CI because we really don't want false positives there.
-        return true;
+        return;
     }
     let Some(base_commit) = &ci_info.base_commit else {
         eprintln!("No base commit, assuming all files are modified");
-        return true;
+        return;
     };
     match crate::git_diff(base_commit, "--name-status") {
         Some(output) => {
-            let modified_files = output.lines().filter_map(|ln| {
-                let (status, name) = ln
-                    .trim_end()
-                    .split_once('\t')
-                    .expect("bad format from `git diff --name-status`");
-                if status == "M" { Some(name) } else { None }
-            });
-            for modified_file in modified_files {
-                if pred(modified_file) {
-                    return true;
+            let modified_files: Vec<_> = output
+                .lines()
+                .filter_map(|ln| {
+                    let (status, name) = ln
+                        .trim_end()
+                        .split_once('\t')
+                        .expect("bad format from `git diff --name-status`");
+                    if status == "M" { Some(name) } else { None }
+                })
+                .collect();
+            items.retain(|item| {
+                for modified_file in &modified_files {
+                    if pred(item, modified_file) {
+                        // at least one predicate matches, keep this item.
+                        return true;
+                    }
                 }
-            }
-            false
+                // no predicates matched, remove this item.
+                false
+            });
         }
         None => {
             eprintln!("warning: failed to run `git diff` to check for changes");
             eprintln!("warning: assuming all files are modified");
-            true
+            return;
         }
     }
 }
 
+/// Returns true if any modified file matches the predicate, if we are in CI, or if unable to list modified files.
+pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool {
+    let mut v = vec![()];
+    files_modified_batch_filter(ci_info, &mut v, |_, p| pred(p));
+    v.is_empty()
+}
+
 pub mod alphabetical;
 pub mod bins;
 pub mod debug_artifacts;