Skip to content

Commit

Permalink
Auto merge of #11399 - Muscraft:autofix-for-clippy, r=weihanglo
Browse files Browse the repository at this point in the history
fix: Make auto-fix note work with `clippy`

[Someone pointed out](#10976 (comment)) that the suggestion to run `cargo --fix` did not work properly for `cargo clippy`. This makes sure the correct command to run is output when running under `cargo clippy`.

This is done by checking if the `RUSTC_WORKSPACE_WRAPPER` environment variable is set, since `clippy` [sets this every run](https://github.com/rust-lang/rust-clippy/blob/51ec465cc3dfb62a2dad7e808b399fa76a1c9170/src/main.rs#L140). Since other things can use `RUSTC_WORKSPACE_WRAPPER`, we look for a wrapper named [`clippy-driver`](https://github.com/rust-lang/rust-clippy/blob/51ec465cc3dfb62a2dad7e808b399fa76a1c9170/src/main.rs#L120).

Since `clippy` might not be available everywhere `cargo` is tested, this is tested using a `rustc` wrapper.
  • Loading branch information
bors committed Dec 15, 2022
2 parents a56fedc + 33c3208 commit 8607003
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 6 deletions.
30 changes: 24 additions & 6 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ use std::collections::{BTreeMap, HashMap, HashSet};
use std::fmt::Write as _;
use std::io;
use std::marker;
use std::path::PathBuf;
use std::sync::Arc;
use std::thread::{self, Scope};
use std::time::Duration;
Expand Down Expand Up @@ -799,7 +800,11 @@ impl<'cfg> DrainState<'cfg> {
self.tokens.extend(rustc_tokens);
}
self.to_send_clients.remove(&id);
self.report_warning_count(cx.bcx.config, id);
self.report_warning_count(
cx.bcx.config,
id,
&cx.bcx.rustc().workspace_wrapper,
);
self.active.remove(&id).unwrap()
}
// ... otherwise if it hasn't finished we leave it
Expand Down Expand Up @@ -1243,7 +1248,12 @@ impl<'cfg> DrainState<'cfg> {
}

/// Displays a final report of the warnings emitted by a particular job.
fn report_warning_count(&mut self, config: &Config, id: JobId) {
fn report_warning_count(
&mut self,
config: &Config,
id: JobId,
rustc_workspace_wrapper: &Option<PathBuf>,
) {
let count = match self.warning_count.remove(&id) {
// An error could add an entry for a `Unit`
// with 0 warnings but having fixable
Expand Down Expand Up @@ -1276,7 +1286,16 @@ impl<'cfg> DrainState<'cfg> {
if let FixableWarnings::Positive(fixable) = count.fixable {
// `cargo fix` doesnt have an option for custom builds
if !unit.target.is_custom_build() {
let mut command = {
// To make sure the correct command is shown for `clippy` we
// check if `RUSTC_WORKSPACE_WRAPPER` is set and pointing towards
// `clippy-driver`.
let clippy = std::ffi::OsStr::new("clippy-driver");
let command = match rustc_workspace_wrapper.as_ref().and_then(|x| x.file_stem())
{
Some(wrapper) if wrapper == clippy => "cargo clippy --fix",
_ => "cargo fix",
};
let mut args = {
let named = unit.target.description_named();
// if its a lib we need to add the package to fix
if unit.target.is_lib() {
Expand All @@ -1288,16 +1307,15 @@ impl<'cfg> DrainState<'cfg> {
if unit.mode.is_rustc_test()
&& !(unit.target.is_test() || unit.target.is_bench())
{
command.push_str(" --tests");
args.push_str(" --tests");
}
let mut suggestions = format!("{} suggestion", fixable);
if fixable > 1 {
suggestions.push_str("s")
}
drop(write!(
message,
" (run `cargo fix --{}` to apply {})",
command, suggestions
" (run `{command} --{args}` to apply {suggestions})"
))
}
}
Expand Down
47 changes: 47 additions & 0 deletions tests/testsuite/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1447,3 +1447,50 @@ fn check_fixable_mixed() {
.with_stderr_contains("[..] (run `cargo fix --bench \"bench\"` to apply 1 suggestion)")
.run();
}

#[cargo_test]
fn check_fixable_warning_for_clippy() {
// A wrapper around `rustc` instead of calling `clippy`
let clippy_driver = project()
.at(cargo_test_support::paths::global_root().join("clippy-driver"))
.file("Cargo.toml", &basic_manifest("clippy-driver", "0.0.1"))
.file(
"src/main.rs",
r#"
fn main() {
let mut args = std::env::args_os();
let _me = args.next().unwrap();
let rustc = args.next().unwrap();
let status = std::process::Command::new(rustc).args(args).status().unwrap();
std::process::exit(status.code().unwrap_or(1));
}
"#,
)
.build();
clippy_driver.cargo("build").run();

let foo = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
"#,
)
// We don't want to show a warning that is `clippy`
// specific since we are using a `rustc` wrapper
// inplace of `clippy`
.file("src/lib.rs", "use std::io;")
.build();

foo.cargo("check")
// We can't use `clippy` so we use a `rustc` workspace wrapper instead
.env(
"RUSTC_WORKSPACE_WRAPPER",
clippy_driver.bin("clippy-driver"),
)
.masquerade_as_nightly_cargo(&["auto-fix note"])
.with_stderr_contains("[..] (run `cargo clippy --fix --lib -p foo` to apply 1 suggestion)")
.run();
}

0 comments on commit 8607003

Please sign in to comment.