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.

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

Consider lint check attributes on match arms #111757

Merged
merged 2 commits into from
May 26, 2023

Conversation

lowr
Copy link
Contributor

@lowr lowr commented May 19, 2023

Currently, lint check attributes on match arms have no effect for some lints. This PR makes some lint passes to take those attributes into account.

  • LateContextAndPass for late lint doesn't update last_node_with_lint_attrs when it visits match arms. This leads to lint check attributes on match arms taking no effects on late lints that operate on the arms' pattern:

    match value {
        #[deny(non_snake_case)]
        PAT => {} // `non_snake_case` only warned due to default lint level
    }

    To be honest, I'm not sure whether this is intentional or just an oversight. I've dug the implementation history and searched up issues/PRs but couldn't find any discussion on this.

  • MatchVisitor doesn't update its lint level when it visits match arms. This leads to check lint attributes on match arms taking no effect on some lints handled by this visitor, namely: bindings_with_variant_name and irrefutable_let_patterns.

    This seems to be a fallout from Check pattern refutability on THIR #108504. Before 05082f5, when the visitor operated on HIR rather than THIR, check lint attributes for the said lints were effective. This playground compiles successfully on current stable (1.69) but fails on current beta and nightly.

    I wasn't sure where best to place the test for this. Let me know if there's a better place.

@rustbot
Copy link
Collaborator

rustbot commented May 19, 2023

r? @eholk

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 19, 2023
@rustbot
Copy link
Collaborator

rustbot commented May 19, 2023

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

@eholk
Copy link
Contributor

eholk commented May 25, 2023

Looks good to me. Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented May 25, 2023

📌 Commit 3a03587 has been approved by eholk

It is now in the queue for this repository.

@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 May 25, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 25, 2023
…=eholk

Consider lint check attributes on match arms

Currently, lint check attributes on match arms have no effect for some lints. This PR makes some lint passes to take those attributes into account.

- `LateContextAndPass` for late lint doesn't update `last_node_with_lint_attrs` when it visits match arms. This leads to lint check attributes on match arms taking no effects on late lints that operate on the arms' pattern:

  ```rust
  match value {
      #[deny(non_snake_case)]
      PAT => {} // `non_snake_case` only warned due to default lint level
  }
  ```

  To be honest, I'm not sure whether this is intentional or just an oversight. I've dug the implementation history and searched up issues/PRs but couldn't find any discussion on this.

- `MatchVisitor` doesn't update its lint level when it visits match arms. This leads to check lint attributes on match arms taking no effect on some lints handled by this visitor, namely: `bindings_with_variant_name` and `irrefutable_let_patterns`.

  This seems to be a fallout from rust-lang#108504. Before 05082f5, when the visitor operated on HIR rather than THIR, check lint attributes for the said lints were effective. [This playground][play] compiles successfully on current stable (1.69) but fails on current beta and nightly.

  I wasn't sure where best to place the test for this. Let me know if there's a better place.

[play]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=38432b79e535cb175f8f7d6d236d29c3
[play-match]: https://play.rust-lang.org/?version=beta&mode=debug&edition=2021&gist=629aa71b7c84b269beadeba664e2221d
bors added a commit to rust-lang-ci/rust that referenced this pull request May 25, 2023
…mpiler-errors

Rollup of 7 pull requests

Successful merges:

 - rust-lang#107522 (Add Median of Medians fallback to introselect)
 - rust-lang#111152 (update `pulldown-cmark` to `0.9.3`)
 - rust-lang#111757 (Consider lint check attributes on match arms)
 - rust-lang#111831 (Always capture slice when pattern requires checking the length)
 - rust-lang#111929 (Don't print newlines in APITs)
 - rust-lang#111945 (Migrate GUI colors test to original CSS color format)
 - rust-lang#111950 (Remove ExpnKind::Inlined.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9d4527b into rust-lang:master May 26, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants