Auto merge of #6834 - hyd-dev:clippy-args, r=phansch,flip1995,oli-obk

Let Cargo track CLIPPY_ARGS

This PR makes `clippy-driver` emit `CLIPPY_ARGS` in its `dep-info` output.

Just like #6441, this allows this workflow to work:
```shell
cargo clippy # warning: empty `loop {}` wastes CPU cycles
cargo clippy -- -A clippy::empty_loop # no warnings emitted
```
But without rebuilding all dependencies.

cc https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/CLIPPY_ARGS.20is.20not.20tracked.20by.20Cargo/near/228599088

changelog: Cargo now re-runs Clippy if arguments after `--` provided to `cargo clippy` are changed.

README.md

@@ -202,7 +202,6 @@ the lint(s) you are interested in:
 ```terminal
 cargo clippy -- -A clippy::all -W clippy::useless_format -W clippy::...
 ```
-Note that if you've run clippy before, this may only take effect after you've modified a file or ran `cargo clean`.
 
 ### Specifying the minimum supported Rust version
 

src/driver.rs

@@ -11,8 +11,12 @@
 extern crate rustc_driver;
 extern crate rustc_errors;
 extern crate rustc_interface;
+extern crate rustc_session;
+extern crate rustc_span;
 
 use rustc_interface::interface;
+use rustc_session::Session;
+use rustc_span::symbol::Symbol;
 use rustc_tools_util::VersionInfo;
 
 use std::borrow::Cow;
@@ -59,13 +63,44 @@ fn test_arg_value() {
     assert_eq!(arg_value(args, "--foo", |_| true), None);
 }
 
+fn track_clippy_args(sess: &Session, args_env_var: &Option<String>) {
+    sess.parse_sess.env_depinfo.borrow_mut().insert((
+        Symbol::intern("CLIPPY_ARGS"),
+        args_env_var.as_deref().map(Symbol::intern),
+    ));
+}
+
 struct DefaultCallbacks;
 impl rustc_driver::Callbacks for DefaultCallbacks {}
 
-struct ClippyCallbacks;
+/// This is different from `DefaultCallbacks` that it will inform Cargo to track the value of
+/// `CLIPPY_ARGS` environment variable.
+struct RustcCallbacks {
+    clippy_args_var: Option<String>,
+}
+
+impl rustc_driver::Callbacks for RustcCallbacks {
+    fn config(&mut self, config: &mut interface::Config) {
+        let previous = config.register_lints.take();
+        let clippy_args_var = self.clippy_args_var.take();
+        config.register_lints = Some(Box::new(move |sess, lint_store| {
+            if let Some(ref previous) = previous {
+                (previous)(sess, lint_store);
+            }
+
+            track_clippy_args(sess, &clippy_args_var);
+        }));
+    }
+}
+
+struct ClippyCallbacks {
+    clippy_args_var: Option<String>,
+}
+
 impl rustc_driver::Callbacks for ClippyCallbacks {
     fn config(&mut self, config: &mut interface::Config) {
         let previous = config.register_lints.take();
+        let clippy_args_var = self.clippy_args_var.take();
         config.register_lints = Some(Box::new(move |sess, mut lint_store| {
             // technically we're ~guaranteed that this is none but might as well call anything that
             // is there already. Certainly it can't hurt.
@@ -73,6 +108,8 @@ impl rustc_driver::Callbacks for ClippyCallbacks {
                 (previous)(sess, lint_store);
             }
 
+            track_clippy_args(sess, &clippy_args_var);
+
             let conf = clippy_lints::read_conf(&[], &sess);
             clippy_lints::register_plugins(&mut lint_store, &sess, &conf);
             clippy_lints::register_pre_expansion_lints(&mut lint_store);
@@ -277,7 +314,9 @@ pub fn main() {
         };
 
         let mut no_deps = false;
-        let clippy_args = env::var("CLIPPY_ARGS")
+        let clippy_args_var = env::var("CLIPPY_ARGS").ok();
+        let clippy_args = clippy_args_var
+            .as_deref()
             .unwrap_or_default()
             .split("__CLIPPY_HACKERY__")
             .filter_map(|s| match s {
@@ -305,11 +344,10 @@ pub fn main() {
             args.extend(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()
+        if clippy_enabled {
+            rustc_driver::RunCompiler::new(&args, &mut ClippyCallbacks { clippy_args_var }).run()
+        } else {
+            rustc_driver::RunCompiler::new(&args, &mut RustcCallbacks { clippy_args_var }).run()
+        }
     }))
 }

tests/dogfood.rs

@@ -3,7 +3,7 @@
 #![feature(once_cell)]
 
 use std::lazy::SyncLazy;
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 use std::process::Command;
 
 mod cargo;
@@ -45,53 +45,45 @@ fn dogfood_clippy() {
     assert!(output.status.success());
 }
 
-#[test]
-fn dogfood_subprojects() {
-    fn test_no_deps_ignores_path_deps_in_workspaces() {
-        fn clean(cwd: &Path, target_dir: &Path) {
-            Command::new("cargo")
-                .current_dir(cwd)
-                .env("CARGO_TARGET_DIR", target_dir)
-                .arg("clean")
-                .args(&["-p", "subcrate"])
-                .args(&["-p", "path_dep"])
-                .output()
-                .unwrap();
-        }
+fn test_no_deps_ignores_path_deps_in_workspaces() {
+    if cargo::is_rustc_test_suite() {
+        return;
+    }
+    let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    let target_dir = root.join("target").join("dogfood");
+    let cwd = root.join("clippy_workspace_tests");
 
-        if cargo::is_rustc_test_suite() {
-            return;
-        }
-        let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-        let target_dir = root.join("target").join("dogfood");
-        let cwd = root.join("clippy_workspace_tests");
+    // Make sure we start with a clean state
+    Command::new("cargo")
+        .current_dir(&cwd)
+        .env("CARGO_TARGET_DIR", &target_dir)
+        .arg("clean")
+        .args(&["-p", "subcrate"])
+        .args(&["-p", "path_dep"])
+        .output()
+        .unwrap();
 
-        // Make sure we start with a clean state
-        clean(&cwd, &target_dir);
+    // `path_dep` is a path dependency of `subcrate` that would trigger a denied lint.
+    // Make sure that with the `--no-deps` argument Clippy does not run on `path_dep`.
+    let output = Command::new(&*CLIPPY_PATH)
+        .current_dir(&cwd)
+        .env("CLIPPY_DOGFOOD", "1")
+        .env("CARGO_INCREMENTAL", "0")
+        .arg("clippy")
+        .args(&["-p", "subcrate"])
+        .arg("--")
+        .arg("--no-deps")
+        .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
+        .args(&["--cfg", r#"feature="primary_package_test""#])
+        .output()
+        .unwrap();
+    println!("status: {}", output.status);
+    println!("stdout: {}", String::from_utf8_lossy(&output.stdout));
+    println!("stderr: {}", String::from_utf8_lossy(&output.stderr));
 
-        // `path_dep` is a path dependency of `subcrate` that would trigger a denied lint.
-        // Make sure that with the `--no-deps` argument Clippy does not run on `path_dep`.
-        let output = Command::new(&*CLIPPY_PATH)
-            .current_dir(&cwd)
-            .env("CLIPPY_DOGFOOD", "1")
-            .env("CARGO_INCREMENTAL", "0")
-            .arg("clippy")
-            .args(&["-p", "subcrate"])
-            .arg("--")
-            .arg("--no-deps")
-            .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
-            .args(&["--cfg", r#"feature="primary_package_test""#])
-            .output()
-            .unwrap();
-        println!("status: {}", output.status);
-        println!("stdout: {}", String::from_utf8_lossy(&output.stdout));
-        println!("stderr: {}", String::from_utf8_lossy(&output.stderr));
-
-        assert!(output.status.success());
-
-        // Make sure we start with a clean state
-        clean(&cwd, &target_dir);
+    assert!(output.status.success());
 
+    let lint_path_dep = || {
         // Test that without the `--no-deps` argument, `path_dep` is linted.
         let output = Command::new(&*CLIPPY_PATH)
             .current_dir(&cwd)
@@ -109,8 +101,51 @@ fn dogfood_subprojects() {
         println!("stderr: {}", String::from_utf8_lossy(&output.stderr));
 
         assert!(!output.status.success());
-    }
+        assert!(
+            String::from_utf8(output.stderr)
+                .unwrap()
+                .contains("error: empty `loop {}` wastes CPU cycles")
+        );
+    };
 
+    // Make sure Cargo is aware of the removal of `--no-deps`.
+    lint_path_dep();
+
+    let successful_build = || {
+        let output = Command::new(&*CLIPPY_PATH)
+            .current_dir(&cwd)
+            .env("CLIPPY_DOGFOOD", "1")
+            .env("CARGO_INCREMENTAL", "0")
+            .arg("clippy")
+            .args(&["-p", "subcrate"])
+            .arg("--")
+            .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
+            .output()
+            .unwrap();
+        println!("status: {}", output.status);
+        println!("stdout: {}", String::from_utf8_lossy(&output.stdout));
+        println!("stderr: {}", String::from_utf8_lossy(&output.stderr));
+
+        assert!(output.status.success());
+
+        output
+    };
+
+    // Trigger a sucessful build, so Cargo would like to cache the build result.
+    successful_build();
+
+    // Make sure there's no spurious rebuild when nothing changes.
+    let stderr = String::from_utf8(successful_build().stderr).unwrap();
+    assert!(!stderr.contains("Compiling"));
+    assert!(!stderr.contains("Checking"));
+    assert!(stderr.contains("Finished"));
+
+    // Make sure Cargo is aware of the new `--cfg` flag.
+    lint_path_dep();
+}
+
+#[test]
+fn dogfood_subprojects() {
     // run clippy on remaining subprojects and fail the test if lint warnings are reported
     if cargo::is_rustc_test_suite() {
         return;