Auto merge of #6188 - ebroto:primary_package, r=Manishearth

Add --no-deps option to avoid running on path dependencies in workspaces

Since rust-lang/cargo#8758 has hit nightly, this allows us to address the second bullet point and [the concern related to `--fix`](rust-lang/cargo#8143 (comment)) in the [RUSTC_WORKSPACE_WRAPPER tracking issue](rust-lang/cargo#8143).

As a reminder stabilizing that env var will solve #4612 (Clippy not running after `cargo check` in stable) and would allow to stabilize the `--fix` option in Clippy.

changelog: Add `--no-deps` option to avoid running on path dependencies in workspaces

Fixes #3025

Author: bors

README.md

@@ -88,6 +88,22 @@ Note that this is still experimental and only supported on the nightly channel:
 cargo clippy --fix -Z unstable-options
 ```
 
+#### Workspaces
+
+All the usual workspace options should work with Clippy. For example the following command
+will run Clippy on the `example` crate:
+
+```terminal
+cargo clippy -p example
+```
+
+As with `cargo check`, this includes dependencies that are members of the workspace, like path dependencies.
+If you want to run Clippy **only** on the given crate, use the `--no-deps` option like this:
+
+```terminal
+cargo clippy -p example -- --no-deps 
+```
+
 ### Running Clippy from the command line without installing it
 
 To have cargo compile your crate with Clippy without Clippy installation

clippy_workspace_tests/path_dep/Cargo.toml

@@ -0,0 +1,3 @@
+[package]
+name = "path_dep"
+version = "0.1.0"

clippy_workspace_tests/path_dep/src/lib.rs

@@ -0,0 +1,6 @@
+#![deny(clippy::empty_loop)]
+
+#[cfg(feature = "primary_package_test")]
+pub fn lint_me() {
+    loop {}
+}

clippy_workspace_tests/subcrate/Cargo.toml

@@ -1,3 +1,6 @@
 [package]
 name = "subcrate"
 version = "0.1.0"
+
+[dependencies]
+path_dep = { path = "../path_dep" }

src/driver.rs

@@ -398,27 +398,34 @@ pub fn main() {
             args.extend(vec!["--sysroot".into(), sys_root]);
         };
 
+        let mut no_deps = false;
+        let clippy_args = env::var("CLIPPY_ARGS")
+            .unwrap_or_default()
+            .split("__CLIPPY_HACKERY__")
+            .filter_map(|s| match s {
+                "" => None,
+                "--no-deps" => {
+                    no_deps = true;
+                    None
+                },
+                _ => Some(s.to_string()),
+            })
+            .chain(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()])
+            .collect::<Vec<String>>();
+
         // 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();
-
-        if clippy_enabled {
-            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())
-                    }
-                }));
-            }
+        let clippy_disabled = env::var("CLIPPY_TESTS").map_or(false, |val| val != "true")
+            || arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_some()
+            || no_deps && env::var("CARGO_PRIMARY_PACKAGE").is_err();
+
+        if !clippy_disabled {
+            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 };
+            if clippy_disabled { &mut default } else { &mut clippy };
         rustc_driver::RunCompiler::new(&args, callbacks).run()
     }))
 }

tests/dogfood.rs

@@ -3,7 +3,7 @@
 #![feature(once_cell)]
 
 use std::lazy::SyncLazy;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::process::Command;
 
 mod cargo;
@@ -41,12 +41,77 @@ fn dogfood_clippy() {
 
 #[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();
+        }
+
+        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
+        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));
+
+        assert!(output.status.success());
+
+        // Make sure we start with a clean state
+        clean(&cwd, &target_dir);
+
+        // Test that without the `--no-deps` argument, `path_dep` is linted.
+        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
+            .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());
+    }
+
     // run clippy on remaining subprojects and fail the test if lint warnings are reported
     if cargo::is_rustc_test_suite() {
         return;
     }
     let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
 
+    // NOTE: `path_dep` crate is omitted on purpose here
     for d in &[
         "clippy_workspace_tests",
         "clippy_workspace_tests/src",
@@ -72,4 +137,8 @@ fn dogfood_subprojects() {
 
         assert!(output.status.success());
     }
+
+    // NOTE: Since tests run in parallel we can't run cargo commands on the same workspace at the
+    // same time, so we test this immediately after the dogfood for workspaces.
+    test_no_deps_ignores_path_deps_in_workspaces();
 }