From 3306363c1a4dd30304c6818634101355c2574da8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sat, 10 Sep 2022 23:41:51 +0200 Subject: [PATCH 1/4] BREAKING CHANGE fix running path/to/cargo-clippy --fix Previously, we would assume clippy is called as "cargo clippy --arg1 --arg2" so we stripped the first two items and parsed "--arg1" and "--arg2" as cmdline args. The problem is when we run cargo-clippy binary directly as "target/debug/cargo-clippy --arg1 --arg2", we would still remove the first two args which would be "..cargo-clippy" and "--arg1" in this case which is wrong. Fix this by checking if we run clippy via "cargo clippy" or "../path/to/cargo-clippy" and for the latter, only skip the first arg instead of two. Spotted by Kraktus because lintcheck --fix was no longer working This is potentially a breaking change because the "target/debug/cargo-clippy clippy --fix" will no longer work! Use "target/debug/cargo-clippy --fix" directly instead. changelog: fix "cargo-clippy --foo" omitting the first flag, break "../cargo-clippy clippy --foo" which will now complain about the "clippy" arg. --- src/main.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 30beaae34d2f..4a51cfc6ab8b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,5 @@ #![cfg_attr(feature = "deny-warnings", deny(warnings))] +#![feature(let_chains)] // warn on lints, that are included in `rust-lang/rust`s bootstrap #![warn(rust_2018_idioms, unused_lifetimes)] @@ -43,7 +44,16 @@ pub fn main() { return; } - if let Err(code) = process(env::args().skip(2)) { + // if we run "cargo clippy" (as cargo subcommand), we have to strip "cargo clippy" (first 2 args) + // but if we are run via "..../target/debug/cargo-clippy", only ommit the first arg, the second one + // might be a normal cmdline arg already (which we don't want to ommit) + let args = if let Some(first_arg) = env::args().next() && first_arg.ends_with("cargo-clippy") { + env::args().skip(1) + } else { + env::args().skip(2) + }; + + if let Err(code) = process(args) { process::exit(code); } } @@ -216,4 +226,14 @@ mod tests { let cmd = ClippyCmd::new(args); assert_eq!("check", cmd.cargo_subcommand); } + + #[test] + fn dont_skip_arg() { + let args = "target/debug/cargo-clippy --fix" + .split_whitespace() + .map(ToString::to_string); + let cmd = ClippyCmd::new(args); + assert_eq!("fix", cmd.cargo_subcommand); + assert!(!cmd.args.iter().any(|arg| arg.ends_with("unstable-options"))); + } } From 83fa8f86d9983bc263f4201a09031ed109f7c924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sun, 11 Sep 2022 00:12:56 +0200 Subject: [PATCH 2/4] fix test fallout --- tests/dogfood.rs | 1 - tests/workspace.rs | 3 --- 2 files changed, 4 deletions(-) diff --git a/tests/dogfood.rs b/tests/dogfood.rs index 3f16c180ea78..c1c79db6f093 100644 --- a/tests/dogfood.rs +++ b/tests/dogfood.rs @@ -98,7 +98,6 @@ fn run_clippy_for_package(project: &str, args: &[&str]) -> bool { command .current_dir(root_dir.join(project)) .env("CARGO_INCREMENTAL", "0") - .arg("clippy") .arg("--all-targets") .arg("--all-features"); diff --git a/tests/workspace.rs b/tests/workspace.rs index 699ab2be199a..b20c2e06ba9b 100644 --- a/tests/workspace.rs +++ b/tests/workspace.rs @@ -71,7 +71,6 @@ fn test_no_deps_ignores_path_deps_in_workspaces() { .current_dir(&cwd) .env("CARGO_INCREMENTAL", "0") .env("CARGO_TARGET_DIR", &target_dir) - .arg("clippy") .args(["-p", "subcrate"]) .arg("--no-deps") .arg("--") @@ -91,7 +90,6 @@ fn test_no_deps_ignores_path_deps_in_workspaces() { .current_dir(&cwd) .env("CARGO_INCREMENTAL", "0") .env("CARGO_TARGET_DIR", &target_dir) - .arg("clippy") .args(["-p", "subcrate"]) .arg("--") .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir @@ -118,7 +116,6 @@ fn test_no_deps_ignores_path_deps_in_workspaces() { .current_dir(&cwd) .env("CARGO_INCREMENTAL", "0") .env("CARGO_TARGET_DIR", &target_dir) - .arg("clippy") .args(["-p", "subcrate"]) .arg("--") .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir From b9b12d4201516bd1649b4ba4bd75e6fb0bda6d25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sun, 11 Sep 2022 00:14:46 +0200 Subject: [PATCH 3/4] account for windows .exe --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 4a51cfc6ab8b..5268856b9318 100644 --- a/src/main.rs +++ b/src/main.rs @@ -47,7 +47,7 @@ pub fn main() { // if we run "cargo clippy" (as cargo subcommand), we have to strip "cargo clippy" (first 2 args) // but if we are run via "..../target/debug/cargo-clippy", only ommit the first arg, the second one // might be a normal cmdline arg already (which we don't want to ommit) - let args = if let Some(first_arg) = env::args().next() && first_arg.ends_with("cargo-clippy") { + let args = if let Some(first_arg) = env::args().next() && (first_arg.ends_with("cargo-clippy") || first_arg.ends_with("cargo-clippy.exe")) { env::args().skip(1) } else { env::args().skip(2) From 56d2dd9d7c72f517a5fdc999951511e73c956731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Wed, 15 May 2024 23:41:49 +0200 Subject: [PATCH 4/4] add warning that "cargo-clippy arg1 arg2" will no longer ignore arg1 after edition 2024 --- src/main.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5268856b9318..d4ff00a83c48 100644 --- a/src/main.rs +++ b/src/main.rs @@ -47,8 +47,15 @@ pub fn main() { // if we run "cargo clippy" (as cargo subcommand), we have to strip "cargo clippy" (first 2 args) // but if we are run via "..../target/debug/cargo-clippy", only ommit the first arg, the second one // might be a normal cmdline arg already (which we don't want to ommit) - let args = if let Some(first_arg) = env::args().next() && (first_arg.ends_with("cargo-clippy") || first_arg.ends_with("cargo-clippy.exe")) { - env::args().skip(1) + let args = if let Some(first_arg) = env::args().next() + && (first_arg.ends_with("cargo-clippy") || first_arg.ends_with("cargo-clippy.exe")) + { + // turn this of during the migration + // env::args().skip(1) + eprintln!( + "WARNING: potentially breaking change: 'cargo-clippy arg1 arg2...' will no longer ignore 'arg1' after edition=2024" + ); + env::args().skip(2) } else { env::args().skip(2) };