Skip to content

Rewrite cargo-clippy - #17573

Open
Jarcho wants to merge 1 commit into
rust-lang:masterfrom
Jarcho:cargo-clippy
Open

Rewrite cargo-clippy#17573
Jarcho wants to merge 1 commit into
rust-lang:masterfrom
Jarcho:cargo-clippy

Conversation

@Jarcho

@Jarcho Jarcho commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Not at all necessary rewrite, but it fixes a couple of weird things.

  • -h, -V and --explain are no longer parsed if they occur after the -- argument. Doesn't really make a difference in practice, but this matches how other programs with argument forwarding work.
  • --explain actually prints an error message when there's no argument.
  • --explain gives an error when it appears multiple times instead of only printing the first. Alternatively we could just print all lints.

This also doesn't walk the argument list four times. Not a big deal since the argument list is usually short and running cargo isn't exactly fast in the first place.

changelog: none

@rustbot rustbot added the S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. label Aug 16, 2026
@rustbot

rustbot commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request. A reviewer will take a look after it receives 2 community reviews.

In the meantime, we would highly appreciate if you could try to review any of PRs waiting on community reviews.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 16, 2026
* Move all argument parsing into a single loop.
* Print an error when `--explain` is missing an argument.
* Print an error when there are multiple `--explain` arguments.
* Don't parse flags after `--`

@CommanderStorm CommanderStorm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

community review: LGTM, but two questions

View changes since this review

Comment thread src/main.rs
Comment on lines +51 to +52
// Cargo exited due to a signal
ExitCode::from(u8::MAX)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not https://doc.rust-lang.org/beta/std/process/struct.ExitCode.html#associatedconstant.FAILURE ?

Both versions are fine with me, just asking if intentional.

Suggested change
// Cargo exited due to a signal
ExitCode::from(u8::MAX)
// Cargo exited due to a signal
ExitCode::FAILURE

@Jarcho Jarcho Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a unique exit code for this. Is it useful; probably not.

Comment thread src/main.rs
@@ -1,153 +1,167 @@
// We need this feature as it changes `dylib` linking behavior and allows us to link to
// `rustc_driver`.
#![feature(rustc_private)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would find the dylib explaination quite usefull as it is non-obvious.. can you re-add the comment here or in below expect?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feature doesn't change linking behaviour. Importing rustc_driver is what links to the dylib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants