diff --git a/src/bin/cargo/commands/clippy.rs b/src/bin/cargo/commands/clippy.rs index 44054ae9c37..8e838389b0f 100644 --- a/src/bin/cargo/commands/clippy.rs +++ b/src/bin/cargo/commands/clippy.rs @@ -69,8 +69,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { .into()); } - let wrapper = util::process("clippy-driver"); - compile_opts.build_config.rustc_wrapper = Some(wrapper); + let wrapper = util::process(util::config::clippy_driver()); + compile_opts.build_config.primary_unit_rustc = Some(wrapper); ops::compile(&ws, &compile_opts)?; Ok(()) diff --git a/src/bin/cargo/commands/fix.rs b/src/bin/cargo/commands/fix.rs index 4ae59e580e3..471d05a4c95 100644 --- a/src/bin/cargo/commands/fix.rs +++ b/src/bin/cargo/commands/fix.rs @@ -72,6 +72,15 @@ pub fn cli() -> App { .long("allow-staged") .help("Fix code even if the working directory has staged changes"), ) + .arg( + Arg::with_name("clippy") + .long("clippy") + .help("Get fix suggestions from clippy instead of rustc") + .hidden(true) + .multiple(true) + .min_values(0) + .number_of_values(1), + ) .after_help( "\ This Cargo subcommand will automatically take rustc's suggestions from @@ -125,6 +134,21 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { // code as we can. let mut opts = args.compile_options(config, mode, Some(&ws))?; + let use_clippy = args.is_present("clippy"); + + let clippy_args = args + .value_of("clippy") + .map(|s| s.split(' ').map(|s| s.to_string()).collect()) + .or_else(|| Some(vec![])) + .filter(|_| use_clippy); + + if use_clippy && !config.cli_unstable().unstable_options { + return Err(failure::format_err!( + "`cargo fix --clippy` is unstable, pass `-Z unstable-options` to enable it" + ) + .into()); + } + if let CompileFilter::Default { .. } = opts.filter { opts.filter = CompileFilter::Only { all_targets: true, @@ -135,6 +159,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { tests: FilterRule::All, } } + ops::fix( &ws, &mut ops::FixOptions { @@ -146,6 +171,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { allow_no_vcs: args.is_present("allow-no-vcs"), allow_staged: args.is_present("allow-staged"), broken_code: args.is_present("broken-code"), + clippy_args, }, )?; Ok(()) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index f5fbe119354..7f795c442cd 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -24,8 +24,8 @@ pub struct BuildConfig { pub force_rebuild: bool, /// Output a build plan to stdout instead of actually compiling. pub build_plan: bool, - /// An optional wrapper, if any, used to wrap rustc invocations - pub rustc_wrapper: Option, + /// An optional override of the rustc path for primary units only + pub primary_unit_rustc: Option, pub rustfix_diagnostic_server: RefCell>, /// Whether or not Cargo should cache compiler output on disk. cache_messages: bool, @@ -98,7 +98,7 @@ impl BuildConfig { message_format: MessageFormat::Human, force_rebuild: false, build_plan: false, - rustc_wrapper: None, + primary_unit_rustc: None, rustfix_diagnostic_server: RefCell::new(None), cache_messages: config.cli_unstable().cache_messages, }) diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 46c6b4b2cc1..324c052ad24 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -51,10 +51,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { units: &'a UnitInterner<'a>, extra_compiler_args: HashMap, Vec>, ) -> CargoResult> { - let mut rustc = config.load_global_rustc(Some(ws))?; - if let Some(wrapper) = &build_config.rustc_wrapper { - rustc.set_wrapper(wrapper.clone()); - } + let rustc = config.load_global_rustc(Some(ws))?; let host_config = TargetConfig::new(config, &rustc.host)?; let target_config = match build_config.requested_target.as_ref() { diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 99fab990ae8..5f54818b6d9 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -69,6 +69,7 @@ pub struct Compilation<'cfg> { config: &'cfg Config, rustc_process: ProcessBuilder, + primary_unit_rustc_process: Option, target_runner: Option<(PathBuf, Vec)>, } @@ -77,13 +78,20 @@ impl<'cfg> Compilation<'cfg> { pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> CargoResult> { let mut rustc = bcx.rustc.process(); + let mut primary_unit_rustc_process = + bcx.build_config.primary_unit_rustc.clone().map(|mut r| { + r.arg(&bcx.rustc.path); + r + }); + if bcx.config.extra_verbose() { rustc.display_env_vars(); + + if let Some(rustc) = primary_unit_rustc_process.as_mut() { + rustc.display_env_vars(); + } } - let srv = bcx.build_config.rustfix_diagnostic_server.borrow(); - if let Some(server) = &*srv { - server.configure(&mut rustc); - } + Ok(Compilation { // TODO: deprecated; remove. native_dirs: BTreeSet::new(), @@ -100,6 +108,7 @@ impl<'cfg> Compilation<'cfg> { rustdocflags: HashMap::new(), config: bcx.config, rustc_process: rustc, + primary_unit_rustc_process, host: bcx.host_triple().to_string(), target: bcx.target_triple().to_string(), target_runner: target_runner(bcx)?, @@ -107,8 +116,21 @@ impl<'cfg> Compilation<'cfg> { } /// See `process`. - pub fn rustc_process(&self, pkg: &Package, target: &Target) -> CargoResult { - let mut p = self.fill_env(self.rustc_process.clone(), pkg, true)?; + pub fn rustc_process( + &self, + pkg: &Package, + target: &Target, + is_primary: bool, + ) -> CargoResult { + let rustc = if is_primary { + self.primary_unit_rustc_process + .clone() + .unwrap_or_else(|| self.rustc_process.clone()) + } else { + self.rustc_process.clone() + }; + + let mut p = self.fill_env(rustc, pkg, true)?; if target.edition() != Edition::Edition2015 { p.arg(format!("--edition={}", target.edition())); } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 628ac8bbff8..eb55e0659af 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -180,9 +180,6 @@ fn rustc<'a, 'cfg>( exec: &Arc, ) -> CargoResult { let mut rustc = prepare_rustc(cx, &unit.target.rustc_crate_types(), unit)?; - if cx.is_primary_package(unit) { - rustc.env("CARGO_PRIMARY_PACKAGE", "1"); - } let build_plan = cx.bcx.build_config.build_plan; let name = unit.pkg.name().to_string(); @@ -593,7 +590,11 @@ fn prepare_rustc<'a, 'cfg>( crate_types: &[&str], unit: &Unit<'a>, ) -> CargoResult { - let mut base = cx.compilation.rustc_process(unit.pkg, unit.target)?; + let is_primary = cx.is_primary_package(unit); + + let mut base = cx + .compilation + .rustc_process(unit.pkg, unit.target, is_primary)?; base.inherit_jobserver(&cx.jobserver); build_base_args(cx, &mut base, unit, crate_types)?; build_deps_args(&mut base, cx, unit)?; diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index e4c1a6fabe4..1ac31974b72 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -63,6 +63,7 @@ const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE"; const PREPARE_FOR_ENV: &str = "__CARGO_FIX_PREPARE_FOR"; const EDITION_ENV: &str = "__CARGO_FIX_EDITION"; const IDIOMS_ENV: &str = "__CARGO_FIX_IDIOMS"; +const CLIPPY_FIX_ARGS: &str = "__CARGO_FIX_CLIPPY_ARGS"; pub struct FixOptions<'a> { pub edition: bool, @@ -73,6 +74,7 @@ pub struct FixOptions<'a> { pub allow_no_vcs: bool, pub allow_staged: bool, pub broken_code: bool, + pub clippy_args: Option>, } pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions<'_>) -> CargoResult<()> { @@ -99,12 +101,38 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions<'_>) -> CargoResult<()> { wrapper.env(IDIOMS_ENV, "1"); } + if opts.clippy_args.is_some() { + if let Err(e) = util::process("clippy-driver").arg("-V").exec_with_output() { + eprintln!("Warning: clippy-driver not found: {:?}", e); + } + + let clippy_args = opts + .clippy_args + .as_ref() + .map_or_else(String::new, |args| serde_json::to_string(&args).unwrap()); + + wrapper.env(CLIPPY_FIX_ARGS, clippy_args); + } + *opts .compile_opts .build_config .rustfix_diagnostic_server .borrow_mut() = Some(RustfixDiagnosticServer::new()?); - opts.compile_opts.build_config.rustc_wrapper = Some(wrapper); + + if let Some(server) = opts + .compile_opts + .build_config + .rustfix_diagnostic_server + .borrow() + .as_ref() + { + server.configure(&mut wrapper); + } + + // primary crates are compiled using a cargo subprocess to do extra work of applying fixes and + // repeating build until there are no more changes to be applied + opts.compile_opts.build_config.primary_unit_rustc = Some(wrapper); ops::compile(ws, &opts.compile_opts)?; Ok(()) @@ -193,18 +221,10 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { trace!("cargo-fix as rustc got file {:?}", args.file); let rustc = args.rustc.as_ref().expect("fix wrapper rustc was not set"); - // Our goal is to fix only the crates that the end user is interested in. - // That's very likely to only mean the crates in the workspace the user is - // working on, not random crates.io crates. - // - // The master cargo process tells us whether or not this is a "primary" - // crate via the CARGO_PRIMARY_PACKAGE environment variable. let mut fixes = FixedCrate::default(); if let Some(path) = &args.file { - if args.primary_package { - trace!("start rustfixing {:?}", path); - fixes = rustfix_crate(&lock_addr, rustc.as_ref(), path, &args)?; - } + trace!("start rustfixing {:?}", path); + fixes = rustfix_crate(&lock_addr, rustc.as_ref(), path, &args)?; } // Ok now we have our final goal of testing out the changes that we applied. @@ -253,7 +273,6 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { } // This final fall-through handles multiple cases; - // - Non-primary crates, which need to be built. // - If the fix failed, show the original warnings and suggestions. // - If `--broken-code`, show the error messages. // - If the fix succeeded, show any remaining warnings. @@ -563,8 +582,8 @@ struct FixArgs { idioms: bool, enabled_edition: Option, other: Vec, - primary_package: bool, rustc: Option, + clippy_args: Vec, } enum PrepareFor { @@ -582,7 +601,14 @@ impl Default for PrepareFor { impl FixArgs { fn get() -> FixArgs { let mut ret = FixArgs::default(); - ret.rustc = env::args_os().nth(1).map(PathBuf::from); + + if let Ok(clippy_args) = env::var(CLIPPY_FIX_ARGS) { + ret.clippy_args = serde_json::from_str(&clippy_args).unwrap(); + ret.rustc = Some(util::config::clippy_driver()); + } else { + ret.rustc = env::args_os().nth(1).map(PathBuf::from); + } + for arg in env::args_os().skip(2) { let path = PathBuf::from(arg); if path.extension().and_then(|s| s.to_str()) == Some("rs") && path.exists() { @@ -608,8 +634,8 @@ impl FixArgs { } else if env::var(EDITION_ENV).is_ok() { ret.prepare_for_edition = PrepareFor::Next; } + ret.idioms = env::var(IDIOMS_ENV).is_ok(); - ret.primary_package = env::var("CARGO_PRIMARY_PACKAGE").is_ok(); ret } @@ -617,17 +643,21 @@ impl FixArgs { if let Some(path) = &self.file { cmd.arg(path); } + + if !self.clippy_args.is_empty() { + cmd.args(&self.clippy_args); + } + cmd.args(&self.other).arg("--cap-lints=warn"); if let Some(edition) = &self.enabled_edition { cmd.arg("--edition").arg(edition); - if self.idioms && self.primary_package && edition == "2018" { + if self.idioms && edition == "2018" { cmd.arg("-Wrust-2018-idioms"); } } - if self.primary_package { - if let Some(edition) = self.prepare_for_edition_resolve() { - cmd.arg("-W").arg(format!("rust-{}-compatibility", edition)); - } + + if let Some(edition) = self.prepare_for_edition_resolve() { + cmd.arg("-W").arg(format!("rust-{}-compatibility", edition)); } } diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 54ca624c805..e31a2caae27 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -1784,3 +1784,12 @@ impl Drop for PackageCacheLock<'_> { } } } + +/// returns path to clippy-driver binary +/// +/// Allows override of the path via `CARGO_CLIPPY_DRIVER` env variable +pub fn clippy_driver() -> PathBuf { + env::var("CARGO_CLIPPY_DRIVER") + .unwrap_or_else(|_| "clippy-driver".into()) + .into() +} diff --git a/src/cargo/util/rustc.rs b/src/cargo/util/rustc.rs index db5abeed415..060517f2c16 100644 --- a/src/cargo/util/rustc.rs +++ b/src/cargo/util/rustc.rs @@ -67,17 +67,22 @@ impl Rustc { } /// Gets a process builder set up to use the found rustc version, with a wrapper if `Some`. - pub fn process(&self) -> ProcessBuilder { + pub fn process_with(&self, path: impl AsRef) -> ProcessBuilder { match self.wrapper { Some(ref wrapper) if !wrapper.get_program().is_empty() => { let mut cmd = wrapper.clone(); - cmd.arg(&self.path); + cmd.arg(path.as_ref()); cmd } - _ => self.process_no_wrapper(), + _ => util::process(path.as_ref()), } } + /// Gets a process builder set up to use the found rustc version, with a wrapper if `Some`. + pub fn process(&self) -> ProcessBuilder { + self.process_with(&self.path) + } + pub fn process_no_wrapper(&self) -> ProcessBuilder { util::process(&self.path) } diff --git a/tests/testsuite/cache_messages.rs b/tests/testsuite/cache_messages.rs index 79959ed1e80..b283a285f4e 100644 --- a/tests/testsuite/cache_messages.rs +++ b/tests/testsuite/cache_messages.rs @@ -1,4 +1,4 @@ -use crate::support::{is_nightly, process, project, registry::Package}; +use crate::support::{clippy_is_available, is_nightly, process, project, registry::Package}; use std::path::Path; fn as_str(bytes: &[u8]) -> &str { @@ -239,15 +239,31 @@ fn rustdoc() { assert_eq!(as_str(&rustdoc_output.stderr), rustdoc_stderr); } +#[cargo_test] +fn fix() { + if !is_nightly() { + // --json-rendered is unstable + return; + } + // Make sure `fix` is not broken by caching. + let p = project().file("src/lib.rs", "pub fn try() {}").build(); + + p.cargo("fix --edition --allow-no-vcs -Zcache-messages") + .masquerade_as_nightly_cargo() + .run(); + + assert_eq!(p.read_file("src/lib.rs"), "pub fn r#try() {}"); +} + #[cargo_test] fn clippy() { if !is_nightly() { // --json-rendered is unstable + eprintln!("skipping test: requires nightly"); return; } - if let Err(e) = process("clippy-driver").arg("-V").exec_with_output() { - eprintln!("clippy-driver not available, skipping clippy test"); - eprintln!("{:?}", e); + + if !clippy_is_available() { return; } @@ -277,22 +293,6 @@ fn clippy() { .run(); } -#[cargo_test] -fn fix() { - if !is_nightly() { - // --json-rendered is unstable - return; - } - // Make sure `fix` is not broken by caching. - let p = project().file("src/lib.rs", "pub fn try() {}").build(); - - p.cargo("fix --edition --allow-no-vcs -Zcache-messages") - .masquerade_as_nightly_cargo() - .run(); - - assert_eq!(p.read_file("src/lib.rs"), "pub fn r#try() {}"); -} - #[cargo_test] fn very_verbose() { if !is_nightly() { diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 748d1df9b8d..9ecca5e4526 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -3,7 +3,7 @@ use std::fs::File; use git2; use crate::support::git; -use crate::support::{basic_manifest, project}; +use crate::support::{basic_manifest, clippy_is_available, is_nightly, project}; use std::io::Write; @@ -1285,3 +1285,50 @@ fn fix_in_existing_repo_weird_ignore() { .run(); p.cargo("fix").cwd("src").run(); } + +#[cargo_test] +fn fix_with_clippy() { + if !is_nightly() { + // fix --clippy is unstable + eprintln!("skipping test: requires nightly"); + return; + } + + if !clippy_is_available() { + return; + } + + let p = project() + .file( + "src/lib.rs", + " + pub fn foo() { + let mut v = Vec::::new(); + let _ = v.iter_mut().filter(|&ref a| a.is_empty()); + } + ", + ) + .build(); + + let stderr = "\ +[CHECKING] foo v0.0.1 ([..]) +[FIXING] src/lib.rs (1 fix) +[FINISHED] [..] +"; + + p.cargo("fix -Zunstable-options --clippy --allow-no-vcs") + .masquerade_as_nightly_cargo() + .with_stderr(stderr) + .with_stdout("") + .run(); + + assert_eq!( + p.read_file("src/lib.rs"), + " + pub fn foo() { + let mut v = Vec::::new(); + let _ = v.iter_mut().filter(|a| a.is_empty()); + } + " + ); +} diff --git a/tests/testsuite/support/mod.rs b/tests/testsuite/support/mod.rs index cd09ea240ad..b695bc06650 100644 --- a/tests/testsuite/support/mod.rs +++ b/tests/testsuite/support/mod.rs @@ -1753,3 +1753,13 @@ pub fn slow_cpu_multiplier(main: u64) -> Duration { } Duration::from_secs(*SLOW_CPU_MULTIPLIER * main) } + +pub fn clippy_is_available() -> bool { + if let Err(e) = process("clippy-driver").arg("-V").exec_with_output() { + eprintln!("clippy-driver not available, skipping clippy test"); + eprintln!("{:?}", e); + false + } else { + true + } +}