tidy: increase performance of auto extra checks feature
Co-authored-by: Jakub Beránek <berykubik@gmail.com>
This commit is contained in:
parent
b96f238308
commit
5185a665a9
2 changed files with 55 additions and 30 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue