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

Create group containing all Rust 2021 edition lints #85894

Closed
nikomatsakis opened this issue Jun 1, 2021 · 11 comments · Fixed by #86330
Closed

Create group containing all Rust 2021 edition lints #85894

nikomatsakis opened this issue Jun 1, 2021 · 11 comments · Fixed by #86330
Assignees
@rylev
Labels
A-edition-2021 Area: The 2021 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jun 1, 2021 •
edited by rustbot
Loading

As follow up to #85512, we need to create a group containing all "Rust 2021 migration"-related lints. Let's start by logging them in the comments here.
@nikomatsakis nikomatsakis added the A-edition-2021 Area: The 2021 edition label Jun 1, 2021
@nikomatsakis nikomatsakis changed the title Group containing all Rust 2021 edition lints Create group containing all Rust 2021 edition lints Jun 1, 2021
@nikomatsakis
Copy link
Contributor Author

This isn't really blocked on #85512, it's just not as useful without it.

@nikomatsakis nikomatsakis mentioned this issue Jun 1, 2021
Closed
3 tasks
@m-ou-se
Copy link
Member

m-ou-se commented Jun 1, 2021 •
edited
Loading

  1. Prelude changes:
  2. Panic consistency:
  3. Lints -> hard errors
    • bare_trait_objects
    • ellipsis_inclusive_range_patterns
  4. Disjoint captures
  5. Cargo resolver 2
  6. IntoIterator for arrays
    • array_into_iter
  7. Or patterns
    • or_patterns_back_compat
  8. Reserving syntax

@rylev
Copy link
Member

rylev commented Jun 4, 2021

@rustbot claim

@inquisitivecrystal
Copy link
Contributor

inquisitivecrystal commented Jun 5, 2021 •
edited
Loading

Sigh. I apologize for the double message (now deleted). Messed up the command format, and then I couldn't tell why it wasn't working.

@rustbot label +A-lint +T-compiler

@rustbot rustbot added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 5, 2021
@rylev rylev mentioned this issue Jun 15, 2021
Merged
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jun 24, 2021
@m-ou-se
Rollup merge of rust-lang#86330 - rylev:update-fcw-handling, r=nikoma…
3eaf6e4 
…tsakis

Change how edition based future compatibility warnings are handled

This fixes rust-lang#85894 by updating how future compatibility lints work. This makes it more apparent that future compatibility warnings can happen for several different reasons.

For now `FutureCompatibilityReasons` are limited to three reasons, but we can easily add more.

This also updates the generated warning for FCW's that signal code that will error in a future edition. This makes the diagnostics between FCWs at edition boundaries more distinct from those not happening at an edition boundary.

r? `@m-ou-se`
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 25, 2021
@JohnTitor
Rollup merge of rust-lang#86330 - rylev:update-fcw-handling, r=nikoma…
95fe65a 
…tsakis

Change how edition based future compatibility warnings are handled

This fixes rust-lang#85894 by updating how future compatibility lints work. This makes it more apparent that future compatibility warnings can happen for several different reasons.

For now `FutureCompatibilityReasons` are limited to three reasons, but we can easily add more.

This also updates the generated warning for FCW's that signal code that will error in a future edition. This makes the diagnostics between FCWs at edition boundaries more distinct from those not happening at an edition boundary.

r? `@m-ou-se`
@bors bors closed this as completed in e01a720 Jun 25, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jun 27, 2021

Re-opening since there's still a few left to do.

@m-ou-se m-ou-se reopened this Jun 27, 2021
@m-ou-se m-ou-se mentioned this issue Jun 27, 2021
Merged
@m-ou-se
Copy link
Member

m-ou-se commented Jun 27, 2021

We might want to change the name of some of these lints, if we still can. E.g. deny(disjoint_capture_migration) doesn't sound like what it does.

@inquisitivecrystal
Copy link
Contributor

If we don't have a way to add lint aliases, we should add that. I've come up with some possible lint renamings, based on the relevant guidelines, though there are quite possibly better ones that still comply with the guidelines. As an aside, why are so many lints being added with noncompliant names?

Meaning
or_patterns_back_compat -> pipes_after_pat_metavariables
array_into_iter -> ambiguous_array_into_iter_derefs
disjoint_capture_migration -> disjoint_capture_drop_reorder (existing lint which I presume is related, preferably and an "ing" at the end to make it sound right with allow/deny/forbid), or maybe something like reliance_on_joint_captures

Plurality
future_prelude_collision -> future_prelude_collisions
non_fmt_panic -> non_fmt_panics
bare_trait_object -> bare_trait_objects
reserved_prefix -> reserved_prefixes

@m-ou-se
Copy link
Member

m-ou-se commented Jun 28, 2021

@inquisitivecrystal Thank you! That's very helpful.

We'll have to check which of these we can still change without causing too much churn.

As an aside, why are so many lints being added with noncompliant names?

It just seems that many contributors and reviewers aren't aware of the guidelines. (I wasn't aware until someone pointed it out after I added a lint with a noncompliant name.)

disjoint_capture_drop_reorder

This one also warns about changed Sync/Send implementations on closures. Might make it a bit tricky to find a proper name for it.

Plurality

Oh, heh, I completely missed that part of the guideline.

@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Jun 28, 2021 •
edited
Loading

We should rename disjoint_capture_migration, but disjoint_capture_drop_reorder doesn't sound right because it's incomplete. I think I like something like rust_2021_incompatible_closure_captures?

@nikomatsakis
Copy link
Contributor Author

This prefix (rust_2021_incompatible) makes sense for things that are "allow by default" -- that is, something you wouldn't warn about except as part of a migration.

JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 28, 2021
@JohnTitor
Rollup merge of rust-lang#86671 - m-ou-se:non-fmt-panic-future-incomp…
14f3335 
…atible, r=nikomatsakis

Turn non_fmt_panic into a future_incompatible edition lint.

This turns the `non_fmt_panic` lint into a future_incompatible edition lint, so it becomes part of the `rust_2021_compatibility` group. See rust-lang#85894.

This lint produces both warnings about semantical changes (e.g. `panic!("{{")`) and things that will become hard errors (e.g. `panic!("{")`). So I added a `explain_reason: false` that supresses the default "this will become a hard error" or "the semantics will change" message, and instead added a note depending on the situation. (cc `@rylev)`

r? `@nikomatsakis`
@rylev rylev mentioned this issue Jun 29, 2021
Merged
@ehuss ehuss mentioned this issue Jul 1, 2021
Merged
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jul 7, 2021
@JohnTitor
Rollup merge of rust-lang#86717 - rylev:rename, r=nikomatsakis
7e95290 
Rename some Rust 2021 lints to better names

Based on conversation in rust-lang#85894.

Rename a bunch of Rust 2021 related lints:

Lints that are officially renamed because they are already in beta or stable:
* `disjoint_capture_migration` => `rust_2021_incompatible_closure_captures`
* `or_patterns_back_compat` => `rust_2021_incompatible_or_patterns`
* `non_fmt_panic` => `non_fmt_panics`

Lints that are renamed but don't require any back -compat work since they aren't yet in stable:
* `future_prelude_collision` => `rust_2021_prelude_collisions`
* `reserved_prefix` => `rust_2021_token_prefixes`

Lints that have been discussed but that I did not rename:
* ~`non_fmt_panic` and `bare_trait_object`: is making this plural worth the headache we might cause users?~
* `array_into_iter`: I'm unsure of a good name and whether bothering users with a name change is worth it.

r? `@nikomatsakis`
@rylev
Copy link
Member

rylev commented Jul 7, 2021

With #86717 merged, we can close this issue.

@rylev rylev closed this as completed Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rylev rylev

Labels
A-edition-2021 Area: The 2021 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change how edition based future compatibility warnings are handled rylev/rust
5 participants
@nikomatsakis @m-ou-se @rylev @inquisitivecrystal @rustbot