Auto merge of rust-lang#5363 - yaahc:clippy-fix, r=phansch,flip1995

add --fix support to `cargo-clippy`

Prior to this we had started work on integrating clippy as a subcommand directly into cargo in the form of `cargo clippy-preview` and `cargo fix --clippy`. In the course of that work it was decided that the best approach would be to strictly add the features clippy needed to cargo in order to insert `clippy-driver` only for workspace crates. This was accomplished by adding a `RUSTC_WORKSPACE_WRAPPER` env variable to cargo that will override the normal `RUSTC_WRAPPER` when both are present and the current crate is a workspace crate.

This change adds support to clippy to use this by setting the `RUSTC_WORKSPACE_WRAPPER` env variable instead `RUSTC_WRAPPER` and by detecting `--fix` as an arg and swapping out the `check` cargo command for `fix` when it is present.

WIP, here are the current issues that I still need to resolve

- [x] Detect if we're running on nightly rust
  - [x] Set `RUSTC_WORKSPACE_WRAPPER` on nightly, and `RUSTC_WRAPPER` on stable
  - [x] Error out on stable when `--fix` is specified, because stable currently hasn't landed the PR for `RUSTC_WORKSPACE_WRAPPER` so if we set this it just runs check and silently fails
- [ ] Update the help text
  - [ ] The current plan is to shell out to `cargo check --help` and then postprocess the output to mention clippy instead of check where appropriate and to add the extra info about `--fix` and the `-- -A lint` options.
- [x] tests?

changelog: add `--fix` arg to `cargo-clippy`

Author: bors

src/main.rs

@@ -1,6 +1,10 @@
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 
 use rustc_tools_util::VersionInfo;
+use std::env;
+use std::ffi::OsString;
+use std::path::PathBuf;
+use std::process::{self, Command};
 
 const CARGO_CLIPPY_HELP: &str = r#"Checks a package to catch common mistakes and improve your Rust code.
 
@@ -37,68 +41,130 @@ fn show_version() {
 
 pub fn main() {
     // Check for version and help flags even when invoked as 'cargo-clippy'
-    if std::env::args().any(|a| a == "--help" || a == "-h") {
+    if env::args().any(|a| a == "--help" || a == "-h") {
         show_help();
         return;
     }
 
-    if std::env::args().any(|a| a == "--version" || a == "-V") {
+    if env::args().any(|a| a == "--version" || a == "-V") {
         show_version();
         return;
     }
 
-    if let Err(code) = process(std::env::args().skip(2)) {
-        std::process::exit(code);
+    if let Err(code) = process(env::args().skip(2)) {
+        process::exit(code);
     }
 }
 
-fn process<I>(mut old_args: I) -> Result<(), i32>
-where
-    I: Iterator<Item = String>,
-{
-    let mut args = vec!["check".to_owned()];
+struct ClippyCmd {
+    unstable_options: bool,
+    cargo_subcommand: &'static str,
+    args: Vec<String>,
+    clippy_args: String,
+}
+
+impl ClippyCmd {
+    fn new<I>(mut old_args: I) -> Self
+    where
+        I: Iterator<Item = String>,
+    {
+        let mut cargo_subcommand = "check";
+        let mut unstable_options = false;
+        let mut args = vec![];
+
+        for arg in old_args.by_ref() {
+            match arg.as_str() {
+                "--fix" => {
+                    cargo_subcommand = "fix";
+                    continue;
+                },
+                "--" => break,
+                // Cover -Zunstable-options and -Z unstable-options
+                s if s.ends_with("unstable-options") => unstable_options = true,
+                _ => {},
+            }
+
+            args.push(arg);
+        }
+
+        if cargo_subcommand == "fix" && !unstable_options {
+            panic!("Usage of `--fix` requires `-Z unstable-options`");
+        }
 
-    for arg in old_args.by_ref() {
-        if arg == "--" {
-            break;
+        // Run the dogfood tests directly on nightly cargo. This is required due
+        // to a bug in rustup.rs when running cargo on custom toolchains. See issue #3118.
+        if env::var_os("CLIPPY_DOGFOOD").is_some() && cfg!(windows) {
+            args.insert(0, "+nightly".to_string());
+        }
+
+        let clippy_args: String = old_args.map(|arg| format!("{}__CLIPPY_HACKERY__", arg)).collect();
+
+        ClippyCmd {
+            unstable_options,
+            cargo_subcommand,
+            args,
+            clippy_args,
+        }
+    }
+
+    fn path_env(&self) -> &'static str {
+        if self.unstable_options {
+            "RUSTC_WORKSPACE_WRAPPER"
+        } else {
+            "RUSTC_WRAPPER"
         }
-        args.push(arg);
     }
 
-    let clippy_args: String = old_args.map(|arg| format!("{}__CLIPPY_HACKERY__", arg)).collect();
+    fn path() -> PathBuf {
+        let mut path = env::current_exe()
+            .expect("current executable path invalid")
+            .with_file_name("clippy-driver");
+
+        if cfg!(windows) {
+            path.set_extension("exe");
+        }
 
-    let mut path = std::env::current_exe()
-        .expect("current executable path invalid")
-        .with_file_name("clippy-driver");
-    if cfg!(windows) {
-        path.set_extension("exe");
+        path
     }
 
-    let target_dir = std::env::var_os("CLIPPY_DOGFOOD")
-        .map(|_| {
-            std::env::var_os("CARGO_MANIFEST_DIR").map_or_else(
-                || std::ffi::OsString::from("clippy_dogfood"),
-                |d| {
-                    std::path::PathBuf::from(d)
-                        .join("target")
-                        .join("dogfood")
-                        .into_os_string()
-                },
-            )
-        })
-        .map(|p| ("CARGO_TARGET_DIR", p));
-
-    // Run the dogfood tests directly on nightly cargo. This is required due
-    // to a bug in rustup.rs when running cargo on custom toolchains. See issue #3118.
-    if std::env::var_os("CLIPPY_DOGFOOD").is_some() && cfg!(windows) {
-        args.insert(0, "+nightly".to_string());
+    fn target_dir() -> Option<(&'static str, OsString)> {
+        env::var_os("CLIPPY_DOGFOOD")
+            .map(|_| {
+                env::var_os("CARGO_MANIFEST_DIR").map_or_else(
+                    || std::ffi::OsString::from("clippy_dogfood"),
+                    |d| {
+                        std::path::PathBuf::from(d)
+                            .join("target")
+                            .join("dogfood")
+                            .into_os_string()
+                    },
+                )
+            })
+            .map(|p| ("CARGO_TARGET_DIR", p))
     }
 
-    let exit_status = std::process::Command::new("cargo")
-        .args(&args)
-        .env("RUSTC_WRAPPER", path)
-        .env("CLIPPY_ARGS", clippy_args)
-        .envs(target_dir)
+    fn into_std_cmd(self) -> Command {
+        let mut cmd = Command::new("cargo");
+
+        cmd.env(self.path_env(), Self::path())
+            .envs(ClippyCmd::target_dir())
+            .env("CLIPPY_ARGS", self.clippy_args)
+            .arg(self.cargo_subcommand)
+            .args(&self.args);
+
+        cmd
+    }
+}
+
+fn process<I>(old_args: I) -> Result<(), i32>
+where
+    I: Iterator<Item = String>,
+{
+    let cmd = ClippyCmd::new(old_args);
+
+    let mut cmd = cmd.into_std_cmd();
+
+    let exit_status = cmd
         .spawn()
         .expect("could not run cargo")
         .wait()
@@ -110,3 +176,44 @@ where
         Err(exit_status.code().unwrap_or(-1))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::ClippyCmd;
+
+    #[test]
+    #[should_panic]
+    fn fix_without_unstable() {
+        let args = "cargo clippy --fix".split_whitespace().map(ToString::to_string);
+        let _ = ClippyCmd::new(args);
+    }
+
+    #[test]
+    fn fix_unstable() {
+        let args = "cargo clippy --fix -Zunstable-options"
+            .split_whitespace()
+            .map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+        assert_eq!("fix", cmd.cargo_subcommand);
+        assert_eq!("RUSTC_WORKSPACE_WRAPPER", cmd.path_env());
+        assert!(cmd.args.iter().any(|arg| arg.ends_with("unstable-options")));
+    }
+
+    #[test]
+    fn check() {
+        let args = "cargo clippy".split_whitespace().map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+        assert_eq!("check", cmd.cargo_subcommand);
+        assert_eq!("RUSTC_WRAPPER", cmd.path_env());
+    }
+
+    #[test]
+    fn check_unstable() {
+        let args = "cargo clippy -Zunstable-options"
+            .split_whitespace()
+            .map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+        assert_eq!("check", cmd.cargo_subcommand);
+        assert_eq!("RUSTC_WORKSPACE_WRAPPER", cmd.path_env());
+    }
+}