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

new lint [redundant_guards] #10955

Merged
merged 1 commit into from
Jul 22, 2023
Merged

Conversation

Centri3
Copy link
Member

@Centri3 Centri3 commented Jun 14, 2023

Closes #7825, maybe somebody else can get the ranges lint in the comments?

changelog: New lint [redundant_guards]

@rustbot
Copy link
Collaborator

rustbot commented Jun 14, 2023

r? @Manishearth

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 14, 2023
@Manishearth
Copy link
Member

bouncing review to @blyxyas (if you're interested) cc @xFrednet

@blyxyas
Copy link
Member

blyxyas commented Jun 14, 2023

Yep, will review it! 🌵

@bors
Copy link
Collaborator

bors commented Jun 15, 2023

☔ The latest upstream changes (presumably #10934) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

wow, I imagine this has been pretty hard to do. Pretty cool (and useful!) lint. Thanks! ❤️ (Some veeery minor nits)

clippy_lints/src/redundant_guard.rs Outdated Show resolved Hide resolved
tests/ui/redundant_guard.rs Outdated Show resolved Hide resolved
@blyxyas
Copy link
Member

blyxyas commented Jun 15, 2023

Just noticed, we should probably put this in the matches group

@Centri3 Centri3 force-pushed the redundant_matches_guard branch 2 times, most recently from e089637 to bc3a80e Compare June 15, 2023 18:58
@xFrednet
Copy link
Member

@Alexendoo, do you maybe want to take over the after/co review with @blyxyas ? Then I would hand this PR over to you two :)

@Alexendoo
Copy link
Member

Sure thing @xFrednet

@xFrednet xFrednet assigned Alexendoo and unassigned Manishearth and xFrednet Jun 15, 2023
clippy_lints/src/matches/redundant_guard.rs Outdated Show resolved Hide resolved
clippy_lints/src/matches/redundant_guard.rs Outdated Show resolved Hide resolved
clippy_lints/src/matches/redundant_guard.rs Outdated Show resolved Hide resolved
clippy_lints/src/matches/redundant_guard.rs Outdated Show resolved Hide resolved
tests/ui/redundant_guard.rs Outdated Show resolved Hide resolved
@Centri3 Centri3 changed the title new lint [redundant_guard] new lint [redundant_guards] Jun 20, 2023
clippy_lints/src/matches/mod.rs Outdated Show resolved Hide resolved
clippy_lints/src/matches/redundant_guards.rs Outdated Show resolved Hide resolved
clippy_lints/src/matches/redundant_guards.rs Outdated Show resolved Hide resolved
tests/ui/redundant_guards.rs Show resolved Hide resolved
@Centri3
Copy link
Member Author

Centri3 commented Jun 21, 2023

I've applied your suggestions minus ignoring or patterns, as that doesn't seem to work unfortunately.

@Centri3 Centri3 force-pushed the redundant_matches_guard branch 2 times, most recently from 9d9ee0f to 8b113bf Compare June 21, 2023 21:10
Copy link
Member

@Alexendoo Alexendoo left a comment

Choose a reason for hiding this comment

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

Thanks, just one last nit and then if you could squash the commits that'd be great

tests/ui/redundant_guards.rs Outdated Show resolved Hide resolved
@Centri3 Centri3 force-pushed the redundant_matches_guard branch 2 times, most recently from 6c75a5b to 0470b78 Compare July 18, 2023 18:37
@bors
Copy link
Collaborator

bors commented Jul 22, 2023

☔ The latest upstream changes (presumably #10885) made this pull request unmergeable. Please resolve the merge conflicts.

@Alexendoo
Copy link
Member

Thanks again! @bors r=Alexendoo,blyxyas

@bors
Copy link
Collaborator

bors commented Jul 22, 2023

📌 Commit 51b5772 has been approved by Alexendoo,blyxyas

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 22, 2023

⌛ Testing commit 51b5772 with merge df3804a...

@bors
Copy link
Collaborator

bors commented Jul 22, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo,blyxyas
Pushing df3804a to master...

@bors bors merged commit df3804a into rust-lang:master Jul 22, 2023
8 checks passed
@Centri3 Centri3 deleted the redundant_matches_guard branch July 22, 2023 12:28
@jcamiel
Copy link

jcamiel commented Oct 7, 2023

Hi, just upgraded to 1.73 and notice this new lint. I've upgraded my code and this snippet:

    let input = input.parse::<i32>().unwrap();
    match input {
        i if i == -1 => println!("-1"),
        i if i == 0 => println!("0"),
        i => println!("{i}")
    }

Triggers the following clippy warning:

warning: redundant guard
  --> src/main.rs:14:14
   |
14 |         i if i == 0 => println!("0"),
   |              ^^^^^^
   |

Fixing the snippet, I've now:

    let input = &args[1];
    let input = input.parse::<i32>().unwrap();
    match input {
        i if i == -1 => println!("-1"),
        0 => println!("0"),
        i => println!("{i}")
    }

And no more clippy warning!

I find it odd that the branch i if i == 0 triggers the lint but not the branch i if i == -1?

@Alexendoo
Copy link
Member

Good catch, opened #11641 to cover that

I always forget that -1 isn't a literal, it's a unary operator applied to the literal 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

New lint: collapsible_guards
8 participants