diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 04d6ce9f8..7003dd06a 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/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 diff --git a/src/cargo/core/compiler/job_queue/mod.rs b/src/cargo/core/compiler/job_queue/mod.rs index 38ab0fe49..abae9bf51 100644 --- a/src/cargo/core/compiler/job_queue/mod.rs +++ b/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(), }; diff --git a/src/cargo/util/diagnostic_server.rs b/src/cargo/util/diagnostic_server.rs index cc5314260..36215735b 100644 --- a/src/cargo/util/diagnostic_server.rs +++ b/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, + // A set of messages that have already been printed. dedupe: HashSet, } impl<'a> DiagnosticPrinter<'a> { - pub fn new(config: &'a Config) -> DiagnosticPrinter<'a> { + pub fn new( + config: &'a Config, + workspace_wrapper: &'a Option, + ) -> 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) -> &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, diff --git a/tests/testsuite/check.rs b/tests/testsuite/check.rs index bbcf750fb..7bc9a38a3 100644 --- a/tests/testsuite/check.rs +++ b/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(); } diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 54a021c03..33de721cd 100644 --- a/tests/testsuite/fix.rs +++ b/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,7 +140,13 @@ 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") @@ -160,7 +164,50 @@ 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/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\ + \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-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\