From 45424c7e757fc15c4dfe5b0ba281863173d785f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= <matthias.krueger@famsik.de>
Date: Fri, 5 Mar 2021 09:30:12 +0100
Subject: [PATCH] lintcheck: add --fix mode which tries to apply lint
 suggestions to the 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
---
 clippy_dev/src/lintcheck.rs | 49 ++++++++++++++++++++++++++++++++++---
 clippy_dev/src/main.rs      |  3 ++-
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/clippy_dev/src/lintcheck.rs b/clippy_dev/src/lintcheck.rs
index f01f14eb458f..765d3349ec01 100644
--- a/clippy_dev/src/lintcheck.rs
+++ b/clippy_dev/src/lintcheck.rs
@@ -229,6 +229,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);
@@ -252,7 +253,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 {
@@ -282,6 +294,23 @@ 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()
@@ -289,6 +318,7 @@ impl Crate {
             .filter(|line| filter_clippy_warnings(&line))
             .map(|json_msg| parse_json_message(json_msg, &self))
             .collect();
+
         warnings
     }
 }
@@ -301,6 +331,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 {
@@ -342,11 +374,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,
         }
     }
 }
@@ -598,7 +632,7 @@ pub fn run(clap_config: &ArgMatches) {
             .into_iter()
             .map(|krate| krate.download_and_extract())
             .filter(|krate| krate.name == only_one_crate)
-            .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &AtomicUsize::new(0), 1, 1))
+            .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &AtomicUsize::new(0), 1, 1, config.fix))
             .collect()
     } else {
         if config.max_jobs > 1 {
@@ -621,7 +655,9 @@ pub fn run(clap_config: &ArgMatches) {
             crates
                 .into_par_iter()
                 .map(|krate| krate.download_and_extract())
-                .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, num_cpus, num_crates))
+                .flat_map(|krate| {
+                    krate.run_clippy_lints(&cargo_clippy_path, &counter, num_cpus, num_crates, config.fix)
+                })
                 .collect()
         } else {
             // run sequential
@@ -629,11 +665,16 @@ pub fn run(clap_config: &ArgMatches) {
             crates
                 .into_iter()
                 .map(|krate| krate.download_and_extract())
-                .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, 1, num_crates))
+                .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, 1, num_crates, config.fix))
                 .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);
 
diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs
index 505d465760c5..33fef18d553a 100644
--- a/clippy_dev/src/main.rs
+++ b/clippy_dev/src/main.rs
@@ -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(