Skip to content

New lint: pats_with_wild_match_arm #4960

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
Merged
merged 7 commits into from
Jan 9, 2020
Merged

New lint: pats_with_wild_match_arm #4960

merged 7 commits into from
Jan 9, 2020

Conversation

ThibsG
Copy link
Contributor

@ThibsG ThibsG commented Dec 26, 2019 •
edited
Loading

Wildcard use with other pattern in same match arm.

The wildcard covers other(s) pattern(s) as it will match anyway.

changelog: add new lint when multiple patterns (including wildcard) are used in a match arm.

Fixes #4640.

krishna-veerareddy
krishna-veerareddy reviewed Dec 26, 2019
View reviewed changes
krishna-veerareddy
krishna-veerareddy reviewed Dec 26, 2019
View reviewed changes
krishna-veerareddy
krishna-veerareddy reviewed Dec 26, 2019
View reviewed changes
@ghost
Copy link

ghost commented Dec 27, 2019

This is great but I think the name could be better. I would call it wildcard_pattern_alternative or something like that... 🤔

@JohnTitor
Copy link
Member

JohnTitor commented Dec 27, 2019 •
edited
Loading

Could you include Fixes #4640 in the PR description so that we can close once this is merged?
And the changelog line is also needed.

@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 27, 2019
@ThibsG
Copy link
Contributor Author

ThibsG commented Dec 27, 2019

@mikerite Yes I believe we could do better for the name. I will think about it

flip1995
flip1995 requested changes Dec 28, 2019
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Lint name suggestion: WILDCARD_IN_OR_PATTERNS

@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 28, 2019
@bors
Copy link
Contributor

bors commented Jan 4, 2020

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

@ThibsG ThibsG force-pushed the patterns_with_wildcard_#4640 branch from 8bea285 to ad16718 Compare January 5, 2020 14:07
@ThibsG ThibsG requested a review from flip1995 January 5, 2020 14:16
flip1995
flip1995 reviewed Jan 6, 2020
View reviewed changes
@ThibsG ThibsG force-pushed the patterns_with_wildcard_#4640 branch from 596f208 to 4ba1bc7 Compare January 7, 2020 17:54
@ThibsG ThibsG force-pushed the patterns_with_wildcard_#4640 branch from 4ba1bc7 to 0fa0df9 Compare January 7, 2020 18:11
@flip1995
Copy link
Member

flip1995 commented Jan 7, 2020

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 7, 2020

📌 Commit 0fa0df9 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Jan 7, 2020

⌛ Testing commit 0fa0df9 with merge 0ff62a5...

bors added a commit that referenced this pull request Jan 7, 2020
@bors
Auto merge of #4960 - ThibsG:patterns_with_wildcard_#4640, r=flip1995
0ff62a5 
New lint: pats_with_wild_match_arm

Wildcard use with other pattern in same match arm.

The wildcard covers other(s) pattern(s) as it will match anyway.

Changelog: add new lint when multiple patterns (including wildcard) are used in a match arm.

Fixes #4640.
@bors
Copy link
Contributor

bors commented Jan 7, 2020

💔 Test failed - checks-travis

@ThibsG
Copy link
Contributor Author

ThibsG commented Jan 7, 2020

Sorry @bors, didn't know you prefer 'changelog' over 'Changelog' in PR description

@flip1995
Copy link
Member

flip1995 commented Jan 7, 2020

Not @bors' fault, I hacked this check into travis 😋

@bors retry

@ThibsG
Copy link
Contributor Author

ThibsG commented Jan 8, 2020

@flip1995 do I need to fix something else here ?

@flip1995
Copy link
Member

flip1995 commented Jan 9, 2020

@bors are you still there?

@bors
Copy link
Contributor

bors commented Jan 9, 2020

🍰 Activated

@flip1995
Copy link
Member

flip1995 commented Jan 9, 2020

Nope, bors just didn't trigger 🤔

@bors r+

@bors
Copy link
Contributor

bors commented Jan 9, 2020

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.

@bors
Copy link
Contributor

bors commented Jan 9, 2020

📌 Commit 0fa0df9 has been approved by flip1995

bors added a commit that referenced this pull request Jan 9, 2020
@bors
Auto merge of #4960 - ThibsG:patterns_with_wildcard_#4640, r=flip1995
ac795a6 
New lint: pats_with_wild_match_arm

Wildcard use with other pattern in same match arm.

The wildcard covers other(s) pattern(s) as it will match anyway.

changelog: add new lint when multiple patterns (including wildcard) are used in a match arm.

Fixes #4640.
@bors
Copy link
Contributor

bors commented Jan 9, 2020

⌛ Testing commit 0fa0df9 with merge ac795a6...

@bors
Copy link
Contributor

bors commented Jan 9, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing ac795a6 to master...

@bors bors merged commit 0fa0df9 into rust-lang:master Jan 9, 2020
@ThibsG ThibsG deleted the patterns_with_wildcard_#4640 branch January 9, 2020 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 approved these changes

+2 more reviewers

@krishna-veerareddy krishna-veerareddy krishna-veerareddy left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unneeded multiple pattern with wildcard
5 participants
@ThibsG @JohnTitor @bors @flip1995 @krishna-veerareddy