Merge commit '4911ab124c481430672a3833b37075e6435ec34d' into clippyup

2 files changed, 207 insertions, 43 deletions

diff --git a/src/driver.rs b/src/driver.rs
index 87dd19c5d4d..40f1b802e60 100644
--- a/src/driver.rs
+++ b/src/driver.rs
@@ -1,5 +1,6 @@
 #![feature(rustc_private)]
 #![feature(once_cell)]
+#![feature(bool_to_option)]
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 // warn on lints, that are included in `rust-lang/rust`s bootstrap
 #![warn(rust_2018_idioms, unused_lifetimes)]
@@ -19,6 +20,7 @@ use rustc_tools_util::VersionInfo;
 
 use std::borrow::Cow;
 use std::env;
+use std::iter;
 use std::lazy::SyncLazy;
 use std::ops::Deref;
 use std::panic;
@@ -41,26 +43,12 @@ fn arg_value<'a, T: Deref<Target = str>>(
 
         match arg.next().or_else(|| args.next()) {
             Some(v) if pred(v) => return Some(v),
-            _ => {}
+            _ => {},
         }
     }
     None
 }
 
-#[test]
-fn test_arg_value() {
-    let args = &["--bar=bar", "--foobar", "123", "--foo"];
-
-    assert_eq!(arg_value(&[] as &[&str], "--foobar", |_| true), None);
-    assert_eq!(arg_value(args, "--bar", |_| false), None);
-    assert_eq!(arg_value(args, "--bar", |_| true), Some("bar"));
-    assert_eq!(arg_value(args, "--bar", |p| p == "bar"), Some("bar"));
-    assert_eq!(arg_value(args, "--bar", |p| p == "foo"), None);
-    assert_eq!(arg_value(args, "--foobar", |p| p == "foo"), None);
-    assert_eq!(arg_value(args, "--foobar", |p| p == "123"), Some("123"));
-    assert_eq!(arg_value(args, "--foo", |_| true), None);
-}
-
 struct DefaultCallbacks;
 impl rustc_driver::Callbacks for DefaultCallbacks {}
 
@@ -121,12 +109,11 @@ You can use tool lints to allow or deny lints from your code, eg.:
 
 const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/new";
 
-static ICE_HOOK: SyncLazy<Box<dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static>> =
-    SyncLazy::new(|| {
-        let hook = panic::take_hook();
-        panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL)));
-        hook
-    });
+static ICE_HOOK: SyncLazy<Box<dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static>> = SyncLazy::new(|| {
+    let hook = panic::take_hook();
+    panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL)));
+    hook
+});
 
 fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
     // Invoke our ICE handler, which prints the actual panic message and optionally a backtrace
@@ -183,6 +170,29 @@ fn toolchain_path(home: Option<String>, toolchain: Option<String>) -> Option<Pat
     })
 }
 
+fn remove_clippy_args<'a, T, U, I>(args: &mut Vec<T>, clippy_args: I)
+where
+    T: AsRef<str>,
+    U: AsRef<str> + ?Sized + 'a,
+    I: Iterator<Item = &'a U> + Clone,
+{
+    let args_iter = clippy_args.map(AsRef::as_ref);
+    let args_count = args_iter.clone().count();
+
+    if args_count > 0 {
+        if let Some(start) = args.windows(args_count).enumerate().find_map(|(current, window)| {
+            window
+                .iter()
+                .map(AsRef::as_ref)
+                .eq(args_iter.clone())
+                .then_some(current)
+        }) {
+            args.drain(start..start + args_count);
+        }
+    }
+}
+
+#[allow(clippy::too_many_lines)]
 pub fn main() {
     rustc_driver::init_rustc_env_logger();
     SyncLazy::force(&ICE_HOOK);
@@ -258,17 +268,14 @@ pub fn main() {
 
         // Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument.
         // We're invoking the compiler programmatically, so we ignore this/
-        let wrapper_mode =
-            orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref());
+        let wrapper_mode = orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref());
 
         if wrapper_mode {
             // we still want to be able to invoke it normally though
             orig_args.remove(1);
         }
 
-        if !wrapper_mode
-            && (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1)
-        {
+        if !wrapper_mode && (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1) {
             display_help();
             exit(0);
         }
@@ -281,25 +288,88 @@ pub fn main() {
             args.extend(vec!["--sysroot".into(), sys_root]);
         };
 
-        // this check ensures that dependencies are built but not linted and the final
-        // crate is linted but not built
-        let clippy_enabled = env::var("CLIPPY_TESTS").map_or(false, |val| val == "true")
-            || arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_none();
+        let clippy_args = env::var("CLIPPY_ARGS").unwrap_or_default();
+        let clippy_args = clippy_args.split_whitespace();
+        let no_deps = clippy_args.clone().any(|flag| flag == "--no-deps");
+
+        // We enable Clippy if one of the following conditions is met
+        // - IF Clippy is run on its test suite OR
+        // - IF Clippy is run on the main crate, not on deps (`!cap_lints_allow`) THEN
+        //    - IF `--no-deps` is not set (`!no_deps`) OR
+        //    - IF `--no-deps` is set and Clippy is run on the specified primary package
+        let clippy_tests_set = env::var("CLIPPY_TESTS").map_or(false, |val| val == "true");
+        let cap_lints_allow = arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_some();
+        let in_primary_package = env::var("CARGO_PRIMARY_PACKAGE").is_ok();
 
+        let clippy_enabled = clippy_tests_set || (!cap_lints_allow && (!no_deps || in_primary_package));
         if clippy_enabled {
+            remove_clippy_args(&mut args, iter::once("--no-deps"));
             args.extend(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()]);
-            if let Ok(extra_args) = env::var("CLIPPY_ARGS") {
-                args.extend(
-                    extra_args
-                        .split("__CLIPPY_HACKERY__")
-                        .filter_map(|s| if s.is_empty() { None } else { Some(s.to_string()) }),
-                );
-            }
+        } else {
+            // Remove all flags passed through RUSTFLAGS if Clippy is not enabled.
+            remove_clippy_args(&mut args, clippy_args);
         }
+
         let mut clippy = ClippyCallbacks;
         let mut default = DefaultCallbacks;
         let callbacks: &mut (dyn rustc_driver::Callbacks + Send) =
             if clippy_enabled { &mut clippy } else { &mut default };
+
         rustc_driver::RunCompiler::new(&args, callbacks).run()
     }))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_arg_value() {
+        let args = &["--bar=bar", "--foobar", "123", "--foo"];
+
+        assert_eq!(arg_value(&[] as &[&str], "--foobar", |_| true), None);
+        assert_eq!(arg_value(args, "--bar", |_| false), None);
+        assert_eq!(arg_value(args, "--bar", |_| true), Some("bar"));
+        assert_eq!(arg_value(args, "--bar", |p| p == "bar"), Some("bar"));
+        assert_eq!(arg_value(args, "--bar", |p| p == "foo"), None);
+        assert_eq!(arg_value(args, "--foobar", |p| p == "foo"), None);
+        assert_eq!(arg_value(args, "--foobar", |p| p == "123"), Some("123"));
+        assert_eq!(arg_value(args, "--foo", |_| true), None);
+    }
+
+    #[test]
+    fn removes_clippy_args_from_start() {
+        let mut args = vec!["-D", "clippy::await_holding_lock", "--cfg", r#"feature="some_feat""#];
+        let clippy_args = ["-D", "clippy::await_holding_lock"].iter();
+
+        remove_clippy_args(&mut args, clippy_args);
+        assert_eq!(args, &["--cfg", r#"feature="some_feat""#]);
+    }
+
+    #[test]
+    fn removes_clippy_args_from_end() {
+        let mut args = vec!["-Zui-testing", "-A", "clippy::empty_loop", "--no-deps"];
+        let clippy_args = ["-A", "clippy::empty_loop", "--no-deps"].iter();
+
+        remove_clippy_args(&mut args, clippy_args);
+        assert_eq!(args, &["-Zui-testing"]);
+    }
+
+    #[test]
+    fn removes_clippy_args_from_middle() {
+        let mut args = vec!["-Zui-testing", "-W", "clippy::filter_map", "-L", "serde"];
+        let clippy_args = ["-W", "clippy::filter_map"].iter();
+
+        remove_clippy_args(&mut args, clippy_args);
+        assert_eq!(args, &["-Zui-testing", "-L", "serde"]);
+    }
+
+    #[test]
+    fn no_clippy_args_to_remove() {
+        let mut args = vec!["-Zui-testing", "-L", "serde"];
+        let clippy_args: [&str; 0] = [];
+
+        remove_clippy_args(&mut args, clippy_args.iter());
+        assert_eq!(args, &["-Zui-testing", "-L", "serde"]);
+    }
+}
diff --git a/src/main.rs b/src/main.rs
index 6739a4cf224..1c0e04689a9 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,3 +1,5 @@
+#![feature(bool_to_option)]
+#![feature(command_access)]
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 // warn on lints, that are included in `rust-lang/rust`s bootstrap
 #![warn(rust_2018_idioms, unused_lifetimes)]
@@ -62,7 +64,7 @@ struct ClippyCmd {
     unstable_options: bool,
     cargo_subcommand: &'static str,
     args: Vec<String>,
-    clippy_args: String,
+    clippy_args: Option<String>,
 }
 
 impl ClippyCmd {
@@ -99,13 +101,17 @@ impl ClippyCmd {
             args.insert(0, "+nightly".to_string());
         }
 
-        let clippy_args: String = old_args.map(|arg| format!("{}__CLIPPY_HACKERY__", arg)).collect();
+        let mut clippy_args = old_args.collect::<Vec<String>>().join(" ");
+        if cargo_subcommand == "fix" && !clippy_args.contains("--no-deps") {
+            clippy_args = format!("{} --no-deps", clippy_args);
+        }
 
+        let has_args = !clippy_args.is_empty();
         ClippyCmd {
             unstable_options,
             cargo_subcommand,
             args,
-            clippy_args,
+            clippy_args: has_args.then_some(clippy_args),
         }
     }
 
@@ -145,15 +151,24 @@ impl ClippyCmd {
             .map(|p| ("CARGO_TARGET_DIR", p))
     }
 
-    fn into_std_cmd(self) -> Command {
+    fn into_std_cmd(self, rustflags: Option<String>) -> 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);
 
+        // HACK: pass Clippy args to the driver *also* through RUSTFLAGS.
+        // This guarantees that new builds will be triggered when Clippy flags change.
+        if let Some(clippy_args) = self.clippy_args {
+            cmd.env(
+                "RUSTFLAGS",
+                rustflags.map_or(clippy_args.clone(), |flags| format!("{} {}", clippy_args, flags)),
+            );
+            cmd.env("CLIPPY_ARGS", clippy_args);
+        }
+
         cmd
     }
 }
@@ -164,7 +179,7 @@ where
 {
     let cmd = ClippyCmd::new(old_args);
 
-    let mut cmd = cmd.into_std_cmd();
+    let mut cmd = cmd.into_std_cmd(env::var("RUSTFLAGS").ok());
 
     let exit_status = cmd
         .spawn()
@@ -182,6 +197,7 @@ where
 #[cfg(test)]
 mod tests {
     use super::ClippyCmd;
+    use std::ffi::OsStr;
 
     #[test]
     #[should_panic]
@@ -196,15 +212,37 @@ mod tests {
             .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 fix_implies_no_deps() {
+        let args = "cargo clippy --fix -Zunstable-options"
+            .split_whitespace()
+            .map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+
+        assert!(cmd.clippy_args.unwrap().contains("--no-deps"));
+    }
+
+    #[test]
+    fn no_deps_not_duplicated_with_fix() {
+        let args = "cargo clippy --fix -Zunstable-options -- --no-deps"
+            .split_whitespace()
+            .map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+
+        assert_eq!(1, cmd.clippy_args.unwrap().matches("--no-deps").count());
+    }
+
+    #[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());
     }
@@ -215,7 +253,63 @@ mod tests {
             .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());
     }
+
+    #[test]
+    fn clippy_args_into_rustflags() {
+        let args = "cargo clippy -- -W clippy::as_conversions"
+            .split_whitespace()
+            .map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+
+        let rustflags = None;
+        let cmd = cmd.into_std_cmd(rustflags);
+
+        assert!(cmd
+            .get_envs()
+            .any(|(key, val)| key == "RUSTFLAGS" && val == Some(OsStr::new("-W clippy::as_conversions"))));
+    }
+
+    #[test]
+    fn clippy_args_respect_existing_rustflags() {
+        let args = "cargo clippy -- -D clippy::await_holding_lock"
+            .split_whitespace()
+            .map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+
+        let rustflags = Some(r#"--cfg feature="some_feat""#.into());
+        let cmd = cmd.into_std_cmd(rustflags);
+
+        assert!(cmd.get_envs().any(|(key, val)| key == "RUSTFLAGS"
+            && val == Some(OsStr::new(r#"-D clippy::await_holding_lock --cfg feature="some_feat""#))));
+    }
+
+    #[test]
+    fn no_env_change_if_no_clippy_args() {
+        let args = "cargo clippy".split_whitespace().map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+
+        let rustflags = Some(r#"--cfg feature="some_feat""#.into());
+        let cmd = cmd.into_std_cmd(rustflags);
+
+        assert!(!cmd
+            .get_envs()
+            .any(|(key, _)| key == "RUSTFLAGS" || key == "CLIPPY_ARGS"));
+    }
+
+    #[test]
+    fn no_env_change_if_no_clippy_args_nor_rustflags() {
+        let args = "cargo clippy".split_whitespace().map(ToString::to_string);
+        let cmd = ClippyCmd::new(args);
+
+        let rustflags = None;
+        let cmd = cmd.into_std_cmd(rustflags);
+
+        assert!(!cmd
+            .get_envs()
+            .any(|(key, _)| key == "RUSTFLAGS" || key == "CLIPPY_ARGS"))
+    }
 }