diff options
| -rw-r--r-- | src/tools/tidy/src/extra_checks/mod.rs | 30 | ||||
| -rw-r--r-- | src/tools/tidy/src/lib.rs | 55 |
2 files changed, 55 insertions, 30 deletions
diff --git a/src/tools/tidy/src/extra_checks/mod.rs b/src/tools/tidy/src/extra_checks/mod.rs index 8121eb057db..f90f716cd95 100644 --- a/src/tools/tidy/src/extra_checks/mod.rs +++ b/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,10 +722,10 @@ 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", @@ -732,12 +733,15 @@ impl ExtraCheckArg { 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 { diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 4f9fb308a86..4ea9d051ddb 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -125,40 +125,61 @@ 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 } } } +/// 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; |