Auto merge of #11882 - hi-rustin:rustin-patch-clippy-fix, r=weihanglo

Correct the bug report for `cargo clippy --fix`

Author: bors

crates/cargo-test-support/src/lib.rs

@@ -517,6 +517,29 @@ pub fn cargo_exe() -> PathBuf {
     snapbox::cmd::cargo_bin("cargo")
 }
 
+/// A wrapper around `rustc` instead of calling `clippy`.
+pub fn wrapped_clippy_driver() -> PathBuf {
+    let clippy_driver = project()
+        .at(paths::global_root().join("clippy-driver"))
+        .file("Cargo.toml", &basic_manifest("clippy-driver", "0.0.1"))
+        .file(
+            "src/main.rs",
+            r#"
+            fn main() {
+                let mut args = std::env::args_os();
+                let _me = args.next().unwrap();
+                let rustc = args.next().unwrap();
+                let status = std::process::Command::new(rustc).args(args).status().unwrap();
+                std::process::exit(status.code().unwrap_or(1));
+            }
+            "#,
+        )
+        .build();
+    clippy_driver.cargo("build").run();
+
+    clippy_driver.bin("clippy-driver")
+}
+
 /// This is the raw output from the process.
 ///
 /// This is similar to `std::process::Output`, however the `status` is

src/cargo/core/compiler/job_queue/mod.rs

@@ -487,7 +487,7 @@ impl<'cfg> JobQueue<'cfg> {
             timings: self.timings,
             tokens: Vec::new(),
             pending_queue: Vec::new(),
-            print: DiagnosticPrinter::new(cx.bcx.config),
+            print: DiagnosticPrinter::new(cx.bcx.config, &cx.bcx.rustc().workspace_wrapper),
             finished: 0,
             per_package_future_incompat_reports: Vec::new(),
         };

src/cargo/util/diagnostic_server.rs

@@ -4,6 +4,7 @@
 use std::collections::HashSet;
 use std::io::{BufReader, Read, Write};
 use std::net::{Shutdown, SocketAddr, TcpListener, TcpStream};
+use std::path::PathBuf;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 use std::thread::{self, JoinHandle};
@@ -18,16 +19,6 @@ use crate::util::errors::CargoResult;
 use crate::util::Config;
 
 const DIAGNOSTICS_SERVER_VAR: &str = "__CARGO_FIX_DIAGNOSTICS_SERVER";
-const PLEASE_REPORT_THIS_BUG: &str =
-    "This likely indicates a bug in either rustc or cargo itself,\n\
-     and we would appreciate a bug report! You're likely to see \n\
-     a number of compiler warnings after this message which cargo\n\
-     attempted to fix but failed. If you could open an issue at\n\
-     https://github.com/rust-lang/rust/issues\n\
-     quoting the full output of this command we'd be very appreciative!\n\
-     Note that you may be able to make some more progress in the near-term\n\
-     fixing code with the `--broken-code` flag\n\n\
-     ";
 
 #[derive(Deserialize, Serialize, Hash, Eq, PartialEq, Clone)]
 pub enum Message {
@@ -83,15 +74,27 @@ impl Message {
     }
 }
 
+/// A printer that will print diagnostics messages to the shell.
 pub struct DiagnosticPrinter<'a> {
+    /// The config to get the shell to print to.
     config: &'a Config,
+    /// An optional wrapper to be used in addition to `rustc.wrapper` for workspace crates.
+    /// This is used to get the correct bug report URL. For instance,
+    /// if `clippy-driver` is set as the value for the wrapper,
+    /// then the correct bug report URL for `clippy` can be obtained.
+    workspace_wrapper: &'a Option<PathBuf>,
+    // A set of messages that have already been printed.
     dedupe: HashSet<Message>,
 }
 
 impl<'a> DiagnosticPrinter<'a> {
-    pub fn new(config: &'a Config) -> DiagnosticPrinter<'a> {
+    pub fn new(
+        config: &'a Config,
+        workspace_wrapper: &'a Option<PathBuf>,
+    ) -> DiagnosticPrinter<'a> {
         DiagnosticPrinter {
             config,
+            workspace_wrapper,
             dedupe: HashSet::new(),
         }
     }
@@ -128,7 +131,12 @@ impl<'a> DiagnosticPrinter<'a> {
                     "The full error message was:\n\n> {}\n\n",
                     message,
                 )?;
-                write!(self.config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?;
+                let issue_link = get_bug_report_url(self.workspace_wrapper);
+                write!(
+                    self.config.shell().err(),
+                    "{}",
+                    gen_please_report_this_bug_text(issue_link)
+                )?;
                 Ok(())
             }
             Message::FixFailed {
@@ -159,7 +167,12 @@ impl<'a> DiagnosticPrinter<'a> {
                     }
                     writeln!(self.config.shell().err())?;
                 }
-                write!(self.config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?;
+                let issue_link = get_bug_report_url(self.workspace_wrapper);
+                write!(
+                    self.config.shell().err(),
+                    "{}",
+                    gen_please_report_this_bug_text(issue_link)
+                )?;
                 if !errors.is_empty() {
                     writeln!(
                         self.config.shell().err(),
@@ -218,6 +231,31 @@ https://doc.rust-lang.org/edition-guide/editions/transitioning-an-existing-proje
     }
 }
 
+fn gen_please_report_this_bug_text(url: &str) -> String {
+    format!(
+        "This likely indicates a bug in either rustc or cargo itself,\n\
+     and we would appreciate a bug report! You're likely to see \n\
+     a number of compiler warnings after this message which cargo\n\
+     attempted to fix but failed. If you could open an issue at\n\
+     {}\n\
+     quoting the full output of this command we'd be very appreciative!\n\
+     Note that you may be able to make some more progress in the near-term\n\
+     fixing code with the `--broken-code` flag\n\n\
+     ",
+        url
+    )
+}
+
+fn get_bug_report_url(rustc_workspace_wrapper: &Option<PathBuf>) -> &str {
+    let clippy = std::ffi::OsStr::new("clippy-driver");
+    let issue_link = match rustc_workspace_wrapper.as_ref().and_then(|x| x.file_stem()) {
+        Some(wrapper) if wrapper == clippy => "https://github.com/rust-lang/rust-clippy/issues",
+        _ => "https://github.com/rust-lang/rust/issues",
+    };
+
+    issue_link
+}
+
 #[derive(Debug)]
 pub struct RustfixDiagnosticServer {
     listener: TcpListener,

tests/testsuite/check.rs

@@ -6,8 +6,8 @@ use crate::messages::raw_rustc_output;
 use cargo_test_support::install::exe;
 use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::Package;
-use cargo_test_support::tools;
 use cargo_test_support::{basic_bin_manifest, basic_manifest, git, project};
+use cargo_test_support::{tools, wrapped_clippy_driver};
 
 #[cargo_test]
 fn check_success() {
@@ -1416,25 +1416,6 @@ fn check_fixable_mixed() {
 
 #[cargo_test]
 fn check_fixable_warning_for_clippy() {
-    // A wrapper around `rustc` instead of calling `clippy`
-    let clippy_driver = project()
-        .at(cargo_test_support::paths::global_root().join("clippy-driver"))
-        .file("Cargo.toml", &basic_manifest("clippy-driver", "0.0.1"))
-        .file(
-            "src/main.rs",
-            r#"
-            fn main() {
-                let mut args = std::env::args_os();
-                let _me = args.next().unwrap();
-                let rustc = args.next().unwrap();
-                let status = std::process::Command::new(rustc).args(args).status().unwrap();
-                std::process::exit(status.code().unwrap_or(1));
-            }
-            "#,
-        )
-        .build();
-    clippy_driver.cargo("build").run();
-
     let foo = project()
         .file(
             "Cargo.toml",
@@ -1452,10 +1433,7 @@ fn check_fixable_warning_for_clippy() {
 
     foo.cargo("check")
         // We can't use `clippy` so we use a `rustc` workspace wrapper instead
-        .env(
-            "RUSTC_WORKSPACE_WRAPPER",
-            clippy_driver.bin("clippy-driver"),
-        )
+        .env("RUSTC_WORKSPACE_WRAPPER", wrapped_clippy_driver())
         .with_stderr_contains("[..] (run `cargo clippy --fix --lib -p foo` to apply 1 suggestion)")
         .run();
 }

tests/testsuite/fix.rs

@@ -5,8 +5,8 @@ use cargo_test_support::compare::assert_match_exact;
 use cargo_test_support::git::{self, init};
 use cargo_test_support::paths::{self, CargoPathExt};
 use cargo_test_support::registry::{Dependency, Package};
-use cargo_test_support::tools;
-use cargo_test_support::{basic_manifest, is_nightly, project};
+use cargo_test_support::{basic_manifest, is_nightly, project, Project};
+use cargo_test_support::{tools, wrapped_clippy_driver};
 
 #[cargo_test]
 fn do_not_fix_broken_builds() {
@@ -53,8 +53,7 @@ fn fix_broken_if_requested() {
         .run();
 }
 
-#[cargo_test]
-fn broken_fixes_backed_out() {
+fn rustc_shim_for_cargo_fix() -> Project {
     // This works as follows:
     // - Create a `rustc` shim (the "foo" project) which will pretend that the
     //   verification step fails.
@@ -109,7 +108,6 @@ fn broken_fixes_backed_out() {
                             fs::File::create(&first).unwrap();
                         }
                     }
-
                     let status = Command::new("rustc")
                         .args(env::args().skip(1))
                         .status()
@@ -142,11 +140,60 @@ fn broken_fixes_backed_out() {
     // Build our rustc shim
     p.cargo("build").cwd("foo").run();
 
-    // Attempt to fix code, but our shim will always fail the second compile
+    p
+}
+
+#[cargo_test]
+fn broken_fixes_backed_out() {
+    let p = rustc_shim_for_cargo_fix();
+    // Attempt to fix code, but our shim will always fail the second compile.
+    p.cargo("fix --allow-no-vcs --lib")
+        .cwd("bar")
+        .env("__CARGO_FIX_YOLO", "1")
+        .env("RUSTC", p.root().join("foo/target/debug/foo"))
+        .with_stderr_contains(
+            "warning: failed to automatically apply fixes suggested by rustc \
+             to crate `bar`\n\
+             \n\
+             after fixes were automatically applied the compiler reported \
+             errors within these files:\n\
+             \n  \
+             * src/lib.rs\n\
+             \n\
+             This likely indicates a bug in either rustc or cargo itself,\n\
+             and we would appreciate a bug report! You're likely to see \n\
+             a number of compiler warnings after this message which cargo\n\
+             attempted to fix but failed. If you could open an issue at\n\
+             https://github.com/rust-lang/rust/issues\n\
+             quoting the full output of this command we'd be very appreciative!\n\
+             Note that you may be able to make some more progress in the near-term\n\
+             fixing code with the `--broken-code` flag\n\
+             \n\
+             The following errors were reported:\n\
+             error: expected one of `!` or `::`, found `rust`\n\
+             ",
+        )
+        .with_stderr_contains("Original diagnostics will follow.")
+        .with_stderr_contains("[WARNING] variable does not need to be mutable")
+        .with_stderr_does_not_contain("[..][FIXED][..]")
+        .run();
+
+    // Make sure the fix which should have been applied was backed out
+    assert!(p.read_file("bar/src/lib.rs").contains("let mut x = 3;"));
+}
+
+#[cargo_test]
+fn broken_clippy_fixes_backed_out() {
+    let p = rustc_shim_for_cargo_fix();
+    // Attempt to fix code, but our shim will always fail the second compile.
+    // Also, we use `clippy` as a workspace wrapper to make sure that we properly
+    // generate the report bug text.
     p.cargo("fix --allow-no-vcs --lib")
         .cwd("bar")
         .env("__CARGO_FIX_YOLO", "1")
         .env("RUSTC", p.root().join("foo/target/debug/foo"))
+        //  We can't use `clippy` so we use a `rustc` workspace wrapper instead
+        .env("RUSTC_WORKSPACE_WRAPPER", wrapped_clippy_driver())
         .with_stderr_contains(
             "warning: failed to automatically apply fixes suggested by rustc \
              to crate `bar`\n\
@@ -160,7 +207,7 @@ fn broken_fixes_backed_out() {
              and we would appreciate a bug report! You're likely to see \n\
              a number of compiler warnings after this message which cargo\n\
              attempted to fix but failed. If you could open an issue at\n\
-             [..]\n\
+             https://github.com/rust-lang/rust-clippy/issues\n\
              quoting the full output of this command we'd be very appreciative!\n\
              Note that you may be able to make some more progress in the near-term\n\
              fixing code with the `--broken-code` flag\n\