-
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
new lint: iter_out_of_bounds
#11396
new lint: iter_out_of_bounds
#11396
Conversation
r? @Jarcho (rustbot has picked a reviewer for you, use r? to override) |
&& let Some(skipped) = expr_as_u128(cx, arg) | ||
&& skipped > len | ||
{ | ||
span_lint_and_note(cx, ITER_OUT_OF_BOUNDS, expr.span, message, None, note); |
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.
This really should have a MaybeIncorrect
suggestion added to it.
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.
I'm not sure what the suggestion should be or if it makes sense to have one. At least for something like [1,2,3].iter().take(4)
it could probably suggest removing the .take()
call as that'll still yield the same items with or without it, but I don't know what the user could have meant with [1,2,3].iter().skip(4)
. Should it suggest and empty iterator? 🤔
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.
I was only considering take
here. Given the suggestion isn't going to have a lot of value (decent chance of being the wrong fix), I guess it's fine to not have one.
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.
This should have some more negative tests added. e.g. [1, 2, 3].iter().take(2)
Thank you. @bors r+ |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Closes #11345
The original idea in the linked issue seemed to be just about arrays afaict, but I extended this to catch some other iterator sources such as
iter::once
oriter::empty
.I'm not entirely sure if this name makes a lot of sense now that it's not just about arrays anymore (specifically, not sure if you can call
.take(1)
on aniter::Empty
to be "out of bounds"?).changelog: [
iter_out_of_bounds
]: new lint