Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change cargo fix --edition to only fix edition lints. #9846

Merged
merged 1 commit into from
Aug 30, 2021
Merged

Change cargo fix --edition to only fix edition lints. #9846

merged 1 commit into from
Aug 30, 2021

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Aug 27, 2021

This changes it so that cargo fix --edition will only fix edition lints. The reason for this is that sometimes non-edition lints get in the way, and make suggestions that can cause failures. An example is a user that only ever runs cargo test or cargo check --profile=test locally, and doesn't realize there are problems with running without cfg(test) such as unused warnings.

This works by using --cap-lints=allow along with --force-warn which takes precedence over cap-lints.

This only works on nightly since --force-warn is still unstable. I will update this as part of #9800.

Closes #5738

@rust-highfive
Copy link

r? @alexcrichton

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 27, 2021
@ehuss
Copy link
Contributor Author

ehuss commented Aug 27, 2021

I'm not super comfortable with this change, as I feel like it could be possible that other lint fixes may be needed to successfully compile. But I can't think of any examples, so it is probably not an issue.

I'm also a bit uncertain about whether or not this is important enough to change. It caused a number of problems with the crater run, primarily with people not using #[cfg(test)] correctly. I assume most of those projects were just toy projects or unmaintained, otherwise I think they would notice the unused warnings and fix them.

It also caused some problems with non-edition lints that are known to give bad suggestions, but those were rare. And again, I would assume a crate that is being maintained would remove the warnings before trying to migrate.

So...I'm not sure if this is a good idea.

@alexcrichton
Copy link
Member

Eh this seems isolated enough that it's not really all that bad. I don't get the impression that cargo fix is regularly run so I'm not surprised that some fixes fail to apply. I do think that the best fix would be to fix those offenders in rustc itself, but there's not a whole lot we can do about that (unless I'm misunderstanding why fixes fail to apply).

Given that this is just flipping an argument though and it kinda makes sense that you at least specifically want edition fixes with --edition and probably not many others, I wouldn't mind landing this.

@ehuss
Copy link
Contributor Author

ehuss commented Aug 27, 2021

Yea, it would be nice to fix the bad suggestions, but there are a lot that hit weird edge cases. And there are some that are fundamentally not possible to avoid, like the #[test] example in this patch.

If you think this is fine, I'm ok with it.

For the next language, can there be no macros and no conditional compilation? They cause no end of trouble. 😝

@alexcrichton
Copy link
Member

Oh I see what you mean about #[test]. Yeah if it's more fundamental in that there's no bugs here I think this is the way to go 👍

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Aug 30, 2021

📌 Commit 46450d6 has been approved by alexcrichton

@bors 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 Aug 30, 2021
@bors
Copy link
Collaborator

bors commented Aug 30, 2021

⌛ Testing commit 46450d6 with merge c227565...

@bors
Copy link
Collaborator

bors commented Aug 30, 2021

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing c227565 to master...

@bors bors merged commit c227565 into rust-lang:master Aug 30, 2021
@ehuss ehuss mentioned this pull request Sep 1, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 1, 2021
@bors
Auto merge of rust-lang#88563 - ehuss:update-cargo-books, r=ehuss
50171c3 
Update cargo, books

## nomicon

2 commits in 0c7e5bd1428e7838252bb57b7f0fbfda4ec82f02..fe6227eb3c8533200c52dffa42ef1b6f2f02c40e
2021-08-04 10:18:22 -0700 to 2021-08-31 05:42:38 +0900
- update lifetime-elision to show what elided code under `rust_2018_idi… (rust-lang/nomicon#306)
- Change code for `into_iter` on the `RawVec` section for consistency/soundness (rust-lang/nomicon#302)

## cargo

8 commits in f559c109cc79fe413a8535fb620a5a58b3823d94..18751dd3f238d94d384a7fe967abfac06cbfe0b9
2021-08-26 22:54:55 +0000 to 2021-09-01 14:26:00 +0000
- print the full destination path when no track duplicates (rust-lang/cargo#9850)
- Stabilize 2021 edition (rust-lang/cargo#9800)
- Stabilize patch-in-config (and prefer config over manifest) (rust-lang/cargo#9839)
- Adding the cargo doc --examples subcommand (rust-lang/cargo#9808)
- Make library created with `cargo new` clippy happy (rust-lang/cargo#9796)
- Swap out some outdated repo urls in documentation (rust-lang/cargo#9862)
- Change `cargo fix --edition` to only fix edition lints. (rust-lang/cargo#9846)
- Show desc of well known subcommands (fmt, clippy) in cargo --list (rust-lang/cargo#9848)

## reference

1 commits in da6ea9b03f74cae0a292f40315723d7a3a973637..0e5ed7a4bec065f0cc18c35d1c904639e095314d
2021-08-19 21:28:10 -0700 to 2021-08-29 17:33:21 +0900
- expressions.md: Attempt fixing broken grammar in Mutability paragraph (rust-lang/reference#1084)

## book

1 commits in 687e21bde2ea10c261f79fa14797c5137425098d..fcb5e0ea68112d85a1d29a7a7335978ef2a02181
2021-08-18 20:48:38 -0400 to 2021-08-31 21:26:19 -0400
- Improve the reading of the code (rust-lang/book#2845)

## rustc-dev-guide

7 commits in cf0e151b7925a40f13fbc6573c6f97d5f94c7c17..95f1acf9a39d6f402f654e917e2c1dfdb779c5fc
2021-08-22 11:47:02 -0300 to 2021-08-31 12:38:30 -0500
- Add link to `Span`
- Add rustc-source to suggested rust-analyzer config (rust-lang/rustc-dev-guide#1189)
- Fix typo, clarify backtick wording, and use inline code
- Trailing date comments in a line inside of a paragraph caused beginning of a new paragraph. (rust-lang/rustc-dev-guide#1196)
- Fix warning "Renderer command uses a path relative to the renderer output directory ..." (rust-lang/rustc-dev-guide#1194)
- Fix a code block containing ```rust
- date-check: Recognize capitalized 'Date' as well

## edition-guide

1 commits in 3710b0cae783d0bcd2b42452a63b081473f5970a..2d9b1b9da706de24650fdc5c3b0182f55c82115d
2021-07-26 11:34:46 -0700 to 2021-08-31 10:44:09 +0200
- Update for 2021 stabilization (rust-lang/edition-guide#266)

## embedded-book

1 commits in 4f9fcaa30d11ba52b641e6fd5206536d65838af9..c3a51e23859554369e6bbb5128dcef0e4f159fb5
2021-08-06 17:43:12 +0000 to 2021-08-26 07:04:58 +0000
- Make glossary more linkable and add more detail  (rust-embedded/book#299)
@ehuss ehuss added this to the 1.56.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.56.0
Development

Successfully merging this pull request may close these issues.

Cargo fix --prepare-for should only apply edition fixes
4 participants
@ehuss @rust-highfive @alexcrichton @bors