initial working version of cargo fix --clippy

src/bin/cargo/commands/clippy.rs

@@ -69,7 +69,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
         .into());
     }
 
-    let wrapper = util::process("clippy-driver");
+    let wrapper = util::process(util::config::clippy_driver());
     compile_opts.build_config.rustc_wrapper = Some(wrapper);
 
     ops::compile(&ws, &compile_opts)?;

src/bin/cargo/commands/fix.rs

@@ -72,6 +72,20 @@ 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 check")
+                .hidden(true),
+        )
+        .arg(
+            Arg::with_name("clippy-arg")
+                .long("clippy-arg")
+                .help("Args to pass through to clippy, implies --clippy")
+                .hidden(true)
+                .multiple(true)
+                .number_of_values(1),
+        )
         .after_help(
             "\
 This Cargo subcommand will automatically take rustc's suggestions from
@@ -125,6 +139,16 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     // code as we can.
     let mut opts = args.compile_options(config, mode, Some(&ws))?;
 
+    let clippy_args = args.values_of_lossy("clippy-args");
+    let use_clippy = args.is_present("clippy") || clippy_args.is_some();
+
+    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,8 @@ 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"),
+            use_clippy,
+            clippy_args,
         },
     )?;
     Ok(())

src/cargo/ops/fix.rs

@@ -63,6 +63,8 @@ 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_ENV: &str = "__CARGO_FIX_CLIPPY_PLZ";
+const CLIPPY_FIX_ARGS: &str = "__CARGO_FIX_CLIPPY_ARGS";
 
 pub struct FixOptions<'a> {
     pub edition: bool,
@@ -73,6 +75,8 @@ pub struct FixOptions<'a> {
     pub allow_no_vcs: bool,
     pub allow_staged: bool,
     pub broken_code: bool,
+    pub use_clippy: bool,
+    pub clippy_args: Option<Vec<String>>,
 }
 
 pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions<'_>) -> CargoResult<()> {
@@ -99,6 +103,18 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions<'_>) -> CargoResult<()> {
         wrapper.env(IDIOMS_ENV, "1");
     }
 
+    if opts.use_clippy {
+        if let Err(e) = util::process("clippy-driver").arg("-V").exec_with_output() {
+            eprintln!("Warning: clippy-driver not found: {:?}", e);
+        }
+        wrapper.env(CLIPPY_FIX_ENV, "1");
+    }
+
+    if let Some(clippy_args) = &opts.clippy_args {
+        let clippy_args = serde_json::to_string(&clippy_args).unwrap();
+        wrapper.env(CLIPPY_FIX_ARGS, clippy_args);
+    }
+
     *opts
         .compile_opts
         .build_config
@@ -385,6 +401,7 @@ fn rustfix_and_fix(
 
     let mut cmd = Command::new(rustc);
     cmd.arg("--error-format=json");
+
     args.apply(&mut cmd);
     let output = cmd
         .output()
@@ -565,6 +582,7 @@ struct FixArgs {
     other: Vec<OsString>,
     primary_package: bool,
     rustc: Option<PathBuf>,
+    clippy_args: Vec<String>,
 }
 
 enum PrepareFor {
@@ -582,7 +600,16 @@ 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 env::var(CLIPPY_FIX_ENV).is_ok() {
+            ret.rustc = Some(util::config::clippy_driver());
+        } else {
+            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();
+        }
+
         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,6 +635,7 @@ 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,6 +645,11 @@ 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);

src/cargo/util/config.rs

@@ -1784,3 +1784,13 @@ 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")
+        .ok()
+        .unwrap_or_else(|| "clippy_driver".into())
+        .into()
+}

tests/testsuite/cache_messages.rs

@@ -239,44 +239,6 @@ fn rustdoc() {
     assert_eq!(as_str(&rustdoc_output.stderr), rustdoc_stderr);
 }
 
-#[cargo_test]
-fn clippy() {
-    if !is_nightly() {
-        // --json-rendered is unstable
-        return;
-    }
-    if let Err(e) = process("clippy-driver").arg("-V").exec_with_output() {
-        eprintln!("clippy-driver not available, skipping clippy test");
-        eprintln!("{:?}", e);
-        return;
-    }
-
-    // Caching clippy output.
-    // This is just a random clippy lint (assertions_on_constants) that
-    // hopefully won't change much in the future.
-    let p = project()
-        .file("src/lib.rs", "pub fn f() { assert!(true); }")
-        .build();
-
-    p.cargo("clippy-preview -Zunstable-options -Zcache-messages")
-        .masquerade_as_nightly_cargo()
-        .with_stderr_contains("[..]assert!(true)[..]")
-        .run();
-
-    // Again, reading from the cache.
-    p.cargo("clippy-preview -Zunstable-options -Zcache-messages")
-        .masquerade_as_nightly_cargo()
-        .with_stderr_contains("[..]assert!(true)[..]")
-        .run();
-
-    // FIXME: Unfortunately clippy is sharing the same hash with check. This
-    // causes the cache to be reused when it shouldn't.
-    p.cargo("check -Zcache-messages")
-        .masquerade_as_nightly_cargo()
-        .with_stderr_contains("[..]assert!(true)[..]") // This should not be here.
-        .run();
-}
-
 #[cargo_test]
 fn fix() {
     if !is_nightly() {

tests/testsuite/clippy.rs

@@ -0,0 +1,79 @@
+use crate::support::{is_nightly, process, project};
+
+#[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);
+        return;
+    }
+
+    // Caching clippy output.
+    // This is just a random clippy lint (assertions_on_constants) that
+    // hopefully won't change much in the future.
+    let p = project()
+        .file("src/lib.rs", "pub fn f() { assert!(true); }")
+        .build();
+
+    p.cargo("clippy-preview -Zunstable-options -Zcache-messages")
+        .masquerade_as_nightly_cargo()
+        .with_stderr_contains("[..]assert!(true)[..]")
+        .run();
+
+    // Again, reading from the cache.
+    p.cargo("clippy-preview -Zunstable-options -Zcache-messages")
+        .masquerade_as_nightly_cargo()
+        .with_stderr_contains("[..]assert!(true)[..]")
+        .run();
+
+    // FIXME: Unfortunately clippy is sharing the same hash with check. This
+    // causes the cache to be reused when it shouldn't.
+    p.cargo("check -Zcache-messages")
+        .masquerade_as_nightly_cargo()
+        .with_stderr_contains("[..]assert!(true)[..]") // This should not be here.
+        .run();
+}
+
+#[cargo_test]
+fn fix_with_clippy() {
+    if !is_nightly() {
+        // fix --clippy 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);
+        return;
+    }
+
+    let p = project()
+        .file(
+            "src/lib.rs",
+            "
+                pub fn foo() {
+                    let mut v = Vec::<String>::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();
+}

tests/testsuite/main.rs

@@ -29,6 +29,7 @@ mod cargo_features;
 mod cfg;
 mod check;
 mod clean;
+mod clippy;
 mod collisions;
 mod concurrent;
 mod config;