diff --git a/README.md b/README.md index fddf0614a0b8..aaa55e11c7db 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,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 diff --git a/clippy_workspace_tests/path_dep/Cargo.toml b/clippy_workspace_tests/path_dep/Cargo.toml new file mode 100644 index 000000000000..85a91cd2decd --- /dev/null +++ b/clippy_workspace_tests/path_dep/Cargo.toml @@ -0,0 +1,3 @@ +[package] +name = "path_dep" +version = "0.1.0" diff --git a/clippy_workspace_tests/path_dep/src/lib.rs b/clippy_workspace_tests/path_dep/src/lib.rs new file mode 100644 index 000000000000..35ce524f2b10 --- /dev/null +++ b/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 {} +} diff --git a/clippy_workspace_tests/subcrate/Cargo.toml b/clippy_workspace_tests/subcrate/Cargo.toml index 83ea5868160b..45362c11b856 100644 --- a/clippy_workspace_tests/subcrate/Cargo.toml +++ b/clippy_workspace_tests/subcrate/Cargo.toml @@ -1,3 +1,6 @@ [package] name = "subcrate" version = "0.1.0" + +[dependencies] +path_dep = { path = "../path_dep" } diff --git a/src/driver.rs b/src/driver.rs index ef31c72481a2..e490ee54c0be 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -182,6 +182,7 @@ fn toolchain_path(home: Option, toolchain: Option) -> Option None, + "--no-deps" => { + no_deps = true; + None + }, + _ => Some(s.to_string()), + }) + .chain(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()]) + .collect::>(); + + // 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 { - 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()) - } - })); - } + 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() })) } diff --git a/src/main.rs b/src/main.rs index 6739a4cf2245..ea06743394d1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -62,7 +62,7 @@ struct ClippyCmd { unstable_options: bool, cargo_subcommand: &'static str, args: Vec, - clippy_args: String, + clippy_args: Vec, } impl ClippyCmd { @@ -99,7 +99,10 @@ 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: Vec = old_args.collect(); + if cargo_subcommand == "fix" && !clippy_args.iter().any(|arg| arg == "--no-deps") { + clippy_args.push("--no-deps".into()); + } ClippyCmd { unstable_options, @@ -147,10 +150,15 @@ impl ClippyCmd { fn into_std_cmd(self) -> Command { let mut cmd = Command::new("cargo"); + let clippy_args: String = self + .clippy_args + .iter() + .map(|arg| format!("{}__CLIPPY_HACKERY__", arg)) + .collect(); cmd.env(self.path_env(), Self::path()) .envs(ClippyCmd::target_dir()) - .env("CLIPPY_ARGS", self.clippy_args) + .env("CLIPPY_ARGS", clippy_args) .arg(self.cargo_subcommand) .args(&self.args); @@ -201,6 +209,24 @@ mod tests { 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.iter().any(|arg| arg == "--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!(cmd.clippy_args.iter().filter(|arg| *arg == "--no-deps").count(), 1); + } + #[test] fn check() { let args = "cargo clippy".split_whitespace().map(ToString::to_string); diff --git a/tests/dogfood.rs b/tests/dogfood.rs index 48e0478f1699..b166a6b7c1ff 100644 --- a/tests/dogfood.rs +++ b/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(); }