fix: Make auto-fix note work with `clippy` by Muscraft · Pull Request #11399 · rust-lang/cargo

Muscraft · 2022-11-21T19:21:44Z

Someone pointed out 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. Since other things can use RUSTC_WORKSPACE_WRAPPER, we look for a wrapper named clippy-driver.

Since clippy might not be available everywhere cargo is tested, this is tested using a rustc wrapper.

rustbot · 2022-11-21T19:21:48Z

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

ehuss · 2022-11-21T19:24:26Z

src/cargo/core/compiler/job_queue.rs

+                    // check if `RUSTC_WORKSPACE_WRAPPER` is set. `clippy` sets
+                    // this when being ran so it can partially be identified
+                    // when this is set.
+                    let command = if rustc_workspace_wrapper.is_some() {


Unfortunately clippy isn't the only tool which sets RUSTC_WORKSPACE_WRAPPER. I think some other approach will be needed here.

I was worried about that, I was hoping it would be fine since it was used before. The only thing I can think of is using CLIPPY_ARGS, which appears to pass the test I wrote. I will update it to that.

After some discussion, it was decided that RUSTC_WORKSPACE_WRAPPER would be better as it is already agreed upon by clippy and cargo. Since other things can use RUSTC_WORKSPACE_WRAPPER, we look for a wrapper named clippy-driver.

0xPoe

Looks goog to me. Thanks!

weihanglo

Looks good! Just some tiny not important stuff.

weihanglo · 2022-12-13T17:14:30Z

src/cargo/core/compiler/job_queue.rs

+                    let command = match rustc_workspace_wrapper {
+                        #[cfg(windows)]
+                        Some(wrapper) if wrapper.ends_with("clippy-driver.exe") => {
+                            "cargo clippy --fix"
+                        }
+                        #[cfg(not(windows))]
+                        Some(wrapper) if wrapper.ends_with("clippy-driver") => "cargo clippy --fix",
+                        _ => "cargo fix",
+                    };


(nit)

Suggested change

let command = match rustc_workspace_wrapper {

#[cfg(windows)]

Some(wrapper) if wrapper.ends_with("clippy-driver.exe") => {

"cargo clippy --fix"

}

#[cfg(not(windows))]

Some(wrapper) if wrapper.ends_with("clippy-driver") => "cargo clippy --fix",

_ => "cargo fix",

};

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",

};

weihanglo · 2022-12-13T17:20:17Z

src/cargo/core/compiler/job_queue.rs

+                        " (run `{} --{}` to apply {})",
+                        command, args, suggestions


(nit)

Suggested change

" (run `{} --{}` to apply {})",

command, args, suggestions

" (run `{command} --{args}` to apply {suggestions})"

weihanglo · 2022-12-15T17:05:48Z

Thanks!

@bors r+

bors · 2022-12-15T17:05:50Z

📌 Commit 33c3208 has been approved by weihanglo

It is now in the queue for this repository.

bors · 2022-12-15T17:05:56Z

⌛ Testing commit 33c3208 with merge 8607003...

bors · 2022-12-15T18:06:22Z

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 8607003 to master...

11 commits in cc0a320879c17207bbfb96b5d778e28a2c62030d..c994a4a638370bc7e0ffcbb0e2865afdfa7d4415 2022-12-14 14:46:57 +0000 to 2022-12-18 21:50:58 +0000 - Fix examples of proc-macro crates being scraped for examples (rust-lang/cargo#11497) - Enable triagebot's relabel functionality (rust-lang/cargo#11498) - Revert "temporarily disable test `lto::test_profile`" (rust-lang/cargo#11495) - Bump to 0.69.0, update changelog (rust-lang/cargo#11493) - Fix typo (rust-lang/cargo#11491) - Display CPU info in CI (rust-lang/cargo#11488) - Fix collision_doc_profile test error (rust-lang/cargo#11489) - fix: Make auto-fix note work with `clippy` (rust-lang/cargo#11399) - fix(add): use the possessive in error message (rust-lang/cargo#11483) - Document home crate in contrib docs (rust-lang/cargo#11481) - Split up registry documentation into multiple sections (rust-lang/cargo#11480)

Update cargo 11 commits in cc0a320879c17207bbfb96b5d778e28a2c62030d..c994a4a638370bc7e0ffcbb0e2865afdfa7d4415 2022-12-14 14:46:57 +0000 to 2022-12-18 21:50:58 +0000 - Fix examples of proc-macro crates being scraped for examples (rust-lang/cargo#11497) - Enable triagebot's relabel functionality (rust-lang/cargo#11498) - Revert "temporarily disable test `lto::test_profile`" (rust-lang/cargo#11495) - Bump to 0.69.0, update changelog (rust-lang/cargo#11493) - Fix typo (rust-lang/cargo#11491) - Display CPU info in CI (rust-lang/cargo#11488) - Fix collision_doc_profile test error (rust-lang/cargo#11489) - fix: Make auto-fix note work with `clippy` (rust-lang/cargo#11399) - fix(add): use the possessive in error message (rust-lang/cargo#11483) - Document home crate in contrib docs (rust-lang/cargo#11481) - Split up registry documentation into multiple sections (rust-lang/cargo#11480) r? `@ghost`

feat: stabilize auto fix note A note that some warnings could be fixed by running a `cargo fix` command was added in #10989 and made to work with `clippy` in #11399. It has only been turned on for `nightly` builds so far; this PR would make it show on `stable`. The original motivation for making this note `nightly` only, was to [allow for iteration](#10976 (comment)) on the message output. There has yet to be any feedback on the message format in the time that it has been on `nightly`. This was brought up in a recent cargo team meeting and it was thought that we should move forward with showing this on `stable`. close #10976

rustbot assigned ehuss Nov 21, 2022

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 21, 2022

ehuss reviewed Nov 21, 2022

View reviewed changes

Muscraft force-pushed the autofix-for-clippy branch 2 times, most recently from e8e9b8b to 12f0c8a Compare November 21, 2022 21:36

0xPoe approved these changes Dec 7, 2022

View reviewed changes

weihanglo approved these changes Dec 13, 2022

View reviewed changes

Muscraft force-pushed the autofix-for-clippy branch from 12f0c8a to 857aa32 Compare December 15, 2022 16:48

fix: Make auto-fix note work with clippy

33c3208

Muscraft force-pushed the autofix-for-clippy branch from 857aa32 to 33c3208 Compare December 15, 2022 16:54

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2022

bors merged commit 8607003 into rust-lang:master Dec 15, 2022

Muscraft deleted the autofix-for-clippy branch December 15, 2022 18:20

weihanglo mentioned this pull request Dec 18, 2022

Update cargo rust-lang/rust#105884

Merged

ehuss added this to the 1.68.0 milestone Dec 30, 2022

weihanglo mentioned this pull request Jan 4, 2023

Add a summary of lints caught in cargo clippy #11532

Closed

Muscraft mentioned this pull request Jan 10, 2023

feat: stabilize auto fix note #11558

Merged

Alexendoo mentioned this pull request May 21, 2023

Warning refers to cargo fix instead of cargo clippy --fix rust-lang/rust-clippy#10804

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Make auto-fix note work with `clippy`#11399

fix: Make auto-fix note work with `clippy`#11399
bors merged 1 commit intorust-lang:masterfrom
Muscraft:autofix-for-clippy

Muscraft commented Nov 21, 2022 •

edited

Loading

Uh oh!

rustbot commented Nov 21, 2022

Uh oh!

ehuss Nov 21, 2022

Uh oh!

Muscraft Nov 21, 2022

Uh oh!

Muscraft Nov 21, 2022

Uh oh!

0xPoe left a comment

Uh oh!

weihanglo left a comment

Uh oh!

weihanglo Dec 13, 2022

Uh oh!

weihanglo Dec 13, 2022

Uh oh!

weihanglo commented Dec 15, 2022

Uh oh!

bors commented Dec 15, 2022

Uh oh!

bors commented Dec 15, 2022

Uh oh!

bors commented Dec 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	" (run `{} --{}` to apply {})",
	command, args, suggestions
	" (run `{command} --{args}` to apply {suggestions})"

Conversation

Muscraft commented Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Nov 21, 2022

Uh oh!

ehuss Nov 21, 2022

Choose a reason for hiding this comment

Uh oh!

Muscraft Nov 21, 2022

Choose a reason for hiding this comment

Uh oh!

Muscraft Nov 21, 2022

Choose a reason for hiding this comment

Uh oh!

0xPoe left a comment

Choose a reason for hiding this comment

Uh oh!

weihanglo left a comment

Choose a reason for hiding this comment

Uh oh!

weihanglo Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

weihanglo Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

weihanglo commented Dec 15, 2022

Uh oh!

bors commented Dec 15, 2022

Uh oh!

bors commented Dec 15, 2022

Uh oh!

bors commented Dec 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Muscraft commented Nov 21, 2022 •

edited

Loading