diff --git a/README.md b/README.md index 3cc03cf36033..63057609bb6f 100644 --- a/README.md +++ b/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 diff --git a/src/driver.rs b/src/driver.rs index d5143e1438ee..82582db0b5ef 100644 --- a/src/driver.rs +++ b/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) { + 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, +} + +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, +} + 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() + } })) } diff --git a/tests/dogfood.rs b/tests/dogfood.rs index 89526648d2e0..d92530f073f5 100644 --- a/tests/dogfood.rs +++ b/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,32 +45,46 @@ 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(); - } - - 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"); +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"); + + // 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(); + + // `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)); - // Make sure we start with a clean state - clean(&cwd, &target_dir); + assert!(output.status.success()); - // `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 lint_path_dep = || { + // Test that without the `--no-deps` argument, `path_dep` is linted. let output = Command::new(&*CLIPPY_PATH) .current_dir(&cwd) .env("CLIPPY_DOGFOOD", "1") @@ -78,7 +92,6 @@ fn dogfood_subprojects() { .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() @@ -87,12 +100,18 @@ fn dogfood_subprojects() { println!("stdout: {}", String::from_utf8_lossy(&output.stdout)); println!("stderr: {}", String::from_utf8_lossy(&output.stderr)); - assert!(output.status.success()); + assert!(!output.status.success()); + assert!( + String::from_utf8(output.stderr) + .unwrap() + .contains("error: empty `loop {}` wastes CPU cycles") + ); + }; - // Make sure we start with a clean state - clean(&cwd, &target_dir); + // Make sure Cargo is aware of the removal of `--no-deps`. + lint_path_dep(); - // Test that without the `--no-deps` argument, `path_dep` is linted. + let successful_build = || { let output = Command::new(&*CLIPPY_PATH) .current_dir(&cwd) .env("CLIPPY_DOGFOOD", "1") @@ -101,16 +120,32 @@ fn dogfood_subprojects() { .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()); - } + 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;