Skip to content

Commit e349c66

Browse files
authored
fix(job_queue): Handle Clippy CLI arguments in fix message (#16652)
fixes #16637 ### What does this PR try to resolve? Clippy lints can be enabled via the command line via `cargo clippy -- -Wclippy::lint`, and these need to be included in the `cargo fix` command (because if a lint is not emitted, it's not fixed, and some lints are allow-by-default) --- When `cargo clippy` is executed, Clippy sets the env. var `CLIPPY_ARGS` to the arguments that were passed to Clippy, separated by the string `__CLIPPY_HACKERY__`. To avoid breaking behaviour in between runs (like in `fix`), we should maintain those arguments. This PR takes that `CLIPPY_ARGS` env var (if it exists), replaces the `__CLIPPY_HACKERY__` separator and appends it to the suggested `cargo clippy --fix` command when emitting fixable warnings. ### How to test and review this PR? Via copying a crate (in my case, `tokio-1.38.1`) and executing the following commands: ```bash rustup run nightly ~/git/cargo/target/debug/cargo clippy -- -Wclippy::pedantic -Aclippy::allow_attributes -Wclippy::dbg_macro -Wclippy::eq_op ``` In order to get some warnings. If any fixable warning is emitted, the output message should end with: ```text warning: `tokio` (lib) generated 42 warnings (run `cargo clippy --fix --lib -p tokio -- -Wclippy::pedantic -Aclippy::allow_attributes -Wclippy::dbg_macro -Wclippy::eq_op` to apply 16 suggestions) ``` Now, testing with the suggested command (in this case, also with the version pinned @1.38.1): ```bash rustup run nightly ~/git/cargo/target/debug/cargo clippy --fix --lib -p tokio@1.38.1 -- -Wclippy::pedantic -Aclippy::allow_attributes -Wclippy::dbg_macro -Wclippy::eq_op ``` All warnings (including those allow-by-default `pedantic` lints!) should be fixed now.
2 parents 3ca292b + 4e786ef commit e349c66

2 files changed

Lines changed: 33 additions & 6 deletions

File tree

src/cargo/core/compiler/job_queue/mod.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,11 @@ mod job_state;
116116
use std::cell::RefCell;
117117
use std::collections::{HashMap, HashSet};
118118
use std::fmt::Write as _;
119-
use std::io;
120119
use std::path::{Path, PathBuf};
121120
use std::sync::Arc;
122121
use std::thread::{self, Scope};
123122
use std::time::Duration;
123+
use std::{env, io};
124124

125125
use anyhow::{Context as _, format_err};
126126
use jobserver::{Acquired, HelperThread};
@@ -1089,11 +1089,15 @@ impl<'gctx> DrainState<'gctx> {
10891089
// check if `RUSTC_WORKSPACE_WRAPPER` is set and pointing towards
10901090
// `clippy-driver`.
10911091
let clippy = std::ffi::OsStr::new("clippy-driver");
1092-
let command = match rustc_workspace_wrapper.as_ref().and_then(|x| x.file_stem())
1093-
{
1094-
Some(wrapper) if wrapper == clippy => "cargo clippy --fix",
1095-
_ => "cargo fix",
1092+
let is_clippy = rustc_workspace_wrapper.as_ref().and_then(|x| x.file_stem())
1093+
== Some(clippy);
1094+
1095+
let command = if is_clippy {
1096+
"cargo clippy --fix"
1097+
} else {
1098+
"cargo fix"
10961099
};
1100+
10971101
let mut args =
10981102
format!("{} -p {}", unit.target.description_named(), unit.pkg.name());
10991103
if unit.mode.is_rustc_test()
@@ -1105,9 +1109,22 @@ impl<'gctx> DrainState<'gctx> {
11051109
if fixable > 1 {
11061110
suggestions.push_str("s")
11071111
}
1112+
1113+
#[expect(clippy::disallowed_methods, reason = "consistency with clippy")]
11081114
let _ = write!(
11091115
message,
1110-
" (run `{command} --{args}` to apply {suggestions})"
1116+
" (run `{command} --{args}{}` to apply {suggestions})",
1117+
if let Some(cli_lints_os) = env::var_os("CLIPPY_ARGS")
1118+
&& let Ok(cli_lints) = cli_lints_os.into_string()
1119+
&& is_clippy
1120+
{
1121+
// Clippy can take lints through the CLI, each lint flag is separated by "__CLIPPY_HACKERY__".
1122+
let cli_lints = cli_lints.replace("__CLIPPY_HACKERY__", " ");
1123+
let cli_lints = cli_lints.trim_ascii_end(); // Remove that last space left by __CLIPPY_HACKERY__
1124+
format!(" -- {cli_lints}")
1125+
} else {
1126+
"".to_owned()
1127+
}
11111128
);
11121129
}
11131130
}

tests/testsuite/check.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1550,6 +1550,16 @@ fn check_fixable_warning_for_clippy() {
15501550
...
15511551
[WARNING] `foo` (lib) generated 1 warning (run `cargo clippy --fix --lib -p foo` to apply 1 suggestion)
15521552
...
1553+
"#]])
1554+
.run();
1555+
1556+
foo.cargo("check")
1557+
.env("RUSTC_WORKSPACE_WRAPPER", tools::wrapped_clippy_driver())
1558+
.env("CLIPPY_ARGS", "-Wclippy::pedantic__CLIPPY_HACKERY__-Aclippy::allow_attributes__CLIPPY_HACKERY__") // Set -Wclippy::pedantic
1559+
.with_stderr_data(str![[r#"
1560+
...
1561+
[WARNING] `foo` (lib) generated 1 warning (run `cargo clippy --fix --lib -p foo -- -Wclippy::pedantic -Aclippy::allow_attributes` to apply 1 suggestion)
1562+
...
15531563
"#]])
15541564
.run();
15551565
}

0 commit comments

Comments
 (0)