Reversed empty ranges #5583
Reversed empty ranges #5583
Conversation
There was a problem hiding this comment.
Looks good to me.
I am worried about people who have set the old lint to deny who will find that reversed loop ranges can now sneak back into their code, but I figure that most people are not going to be denying this specific lint directly, so we don't actually have a problem in practice.
flip1995
added
the
S-waiting-on-author
|
@yaahc I don't think this will be an issue, since the message "this lint is now included in reversed_empty_ranges" will be displayed to the user. no regressions should come from this, that's why we should persist the tests. |
|
got it, okay, sounds perfect to me then. |
| } | ||
|
|
||
| for i in (10..0).map(|x| x * 2) { | ||
| // not an error, it can't be known what arbitrary methods do to a range |
There was a problem hiding this comment.
This will trigger the lint now. Although the mapped closure will never run in this case (nor other Iterator methods), it makes me think that someone could e.g. implement a trait for Range that manipulates the bounds arbitrarily (fields are pub)...
The fact that the range is empty will not change though, and seems a niche case. Thoughts?
flip1995
added
S-waiting-on-bors
|
Thanks, waiting for rustup #5587 |
|
Needs a rebase |
flip1995
added
S-waiting-on-author
ebroto
force-pushed
the
reversed_empty_ranges
branch
from
3394a6a to
064431d
Compare
|
Thanks! @bors r=yaahc,flip1995 |
|
📌 Commit 671c1e3 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
This lint checks range expressions with inverted limits which result in empty ranges. This includes also the ranges used to index slices.
The lint reverse_range_loop was covering iteration of reversed ranges in a for loop, which is a subset of what this new lint covers, so it has been removed. I'm not sure if that's the best choice. It would be doable to check in the new lint that we are not in the arguments of a for loop; I went for removing it because the logic was too similar to keep them separated.
changelog: Added reversed_empty_ranges lint that checks for ranges where the limits have been inverted, resulting in empty ranges. Removed reverse_range_loop which was covering a subset of the new lint.
Closes #4192
Closes #96