-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Do not lint when range is completely included into another one #6603
Conversation
r? @flip1995 (rust-highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Some more testing would be nice, though.
match 42 { | ||
0..14 => println!("0 .. 14"), | ||
5..10 => println!("5 .. 10"), | ||
_ => (), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, shouldn't rustc trigger here? the second arm is unreachable. Well, at least this lint doesn't trigger here.
A quick check on Playground shows that rustc triggers here correctly. So I assume this is a late pass lint in rustc and therefore would only trigger once the clippy lint doesn't trigger anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I was a bit surprise too, I was also waiting for an unreachable warning 😅
@bors r+ Thanks! |
📌 Commit 0518911 has been approved by |
Do not lint when range is completely included into another one This fix has been developed following this [comment](#5986 (comment)). So this will be linted: ``` |----------| |-----------| ``` Now this won't be linted: ``` |---| |--------------------| ``` and this will still lint: ``` |--------| |--------------| ``` Fixes: #5986 changelog: none
@bors retry (for changelog) |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
This fix has been developed following this comment.
So this will be linted:
Now this won't be linted:
and this will still lint:
Fixes: #5986
changelog: Fix FPs in match_overlapping_arm, when first arm is completely included in second arm