Skip to content

Commit

Permalink
lintcheck: add --fix mode which tries to apply lint suggestions to th…
Browse files Browse the repository at this point in the history
…e sources and prints a warning if that fails

Great for spotting false positives/broken suggestions of applicable lints.

There are false positives though becasue I'm not sure yet how to silence rustc warnings while keeping clippy warnings.
Sometimes rustc makes a suggestion that fails to apply and the implementation does not differenciate between clippy and rustc warnings when applying lint suggestions.

changelog: none
  • Loading branch information
matthiaskrgr committed Mar 5, 2021
1 parent f0e6ce8 commit 3033888
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 5 deletions.
47 changes: 43 additions & 4 deletions clippy_dev/src/lintcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ impl Crate {
target_dir_index: &AtomicUsize,
thread_limit: usize,
total_crates_to_lint: usize,
fix: bool,
) -> Vec<ClippyWarning> {
// advance the atomic index by one
let index = target_dir_index.fetch_add(1, Ordering::SeqCst);
Expand All @@ -255,7 +256,18 @@ impl Crate {

let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir");

let mut args = vec!["--", "--message-format=json", "--", "--cap-lints=warn"];
let mut args = if fix {
vec![
"-Zunstable-options",
"--fix",
"-Zunstable-options",
"--allow-no-vcs",
"--",
"--cap-lints=warn",
]
} else {
vec!["--", "--message-format=json", "--", "--cap-lints=warn"]
};

if let Some(options) = &self.options {
for opt in options {
Expand Down Expand Up @@ -285,13 +297,31 @@ impl Crate {
);
});
let stdout = String::from_utf8_lossy(&all_output.stdout);
let stderr = String::from_utf8_lossy(&all_output.stderr);

if fix {
if let Some(stderr) = stderr
.lines()
.find(|line| line.contains("failed to automatically apply fixes suggested by rustc to crate"))
{
let subcrate = &stderr[63..];
println!(
"ERROR: failed to apply some suggetion to {} / to (sub)crate {}",
self.name, subcrate
);
}
// fast path, we don't need the warnings anyway
return Vec::new();
}

let output_lines = stdout.lines();
let warnings: Vec<ClippyWarning> = output_lines
.into_iter()
// get all clippy warnings and ICEs
.filter(|line| filter_clippy_warnings(&line))
.map(|json_msg| parse_json_message(json_msg, &self))
.collect();

warnings
}
}
Expand All @@ -304,6 +334,8 @@ struct LintcheckConfig {
sources_toml_path: PathBuf,
// we save the clippy lint results here
lintcheck_results_path: PathBuf,
// whether to just run --fix and not collect all the warnings
fix: bool,
}

impl LintcheckConfig {
Expand Down Expand Up @@ -345,11 +377,13 @@ impl LintcheckConfig {
// no -j passed, use a single thread
None => 1,
};
let fix: bool = clap_config.is_present("fix");

LintcheckConfig {
max_jobs,
sources_toml_path,
lintcheck_results_path,
fix,
}
}
}
Expand Down Expand Up @@ -597,7 +631,7 @@ pub fn run(clap_config: &ArgMatches) {
.into_iter()
.map(|krate| krate.download_and_extract())
.filter(|krate| krate.name == only_one_crate)
.map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &AtomicUsize::new(0), 1, 1))
.map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &AtomicUsize::new(0), 1, 1, config.fix))
.flatten()
.collect()
} else {
Expand All @@ -621,7 +655,7 @@ pub fn run(clap_config: &ArgMatches) {
crates
.into_par_iter()
.map(|krate| krate.download_and_extract())
.map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, num_cpus, num_crates))
.map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, num_cpus, num_crates, config.fix))
.flatten()
.collect()
} else {
Expand All @@ -630,12 +664,17 @@ pub fn run(clap_config: &ArgMatches) {
crates
.into_iter()
.map(|krate| krate.download_and_extract())
.map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, 1, num_crates))
.map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, 1, num_crates, config.fix))
.flatten()
.collect()
}
};

// if we are in --fix mode, don't change the log files, terminate here
if config.fix {
return;
}

// generate some stats
let (stats_formatted, new_stats) = gather_stats(&clippy_warnings);

Expand Down
3 changes: 2 additions & 1 deletion clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ fn get_clap_config<'a>() -> ArgMatches<'a> {
.short("j")
.long("jobs")
.help("number of threads to use, 0 automatic choice"),
);
)
.arg(Arg::with_name("fix").help("runs cargo clippy --fix and checks if all suggestions apply"));

let app = App::new("Clippy developer tooling")
.subcommand(
Expand Down

0 comments on commit 3033888

Please sign in to comment.