clippy_lints: support iter_mut in ITER_NEXT_SLICE fixes #17115#17122
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
| ITER_NEXT_SLICE, | ||
| expr.span, | ||
| "using `.iter().next()` on a Slice without end index", | ||
| "using `.{method}.next()` on a Slice without end index", |
There was a problem hiding this comment.
True, but it wasnt there to begin with so i didnt wanna add that, but ill add it
There was a problem hiding this comment.
What do you mean? Of course there was no format!(), as iter() was hardcoded, but this is no longer the case after your change.
There was a problem hiding this comment.
oh yeah ur talking since we try to load method dynamically we need to use format! ? sorry its my first time contributing to clippy
| "try calling", | ||
| format!( | ||
| "{}.first()", | ||
| "{}.{method}()", |
There was a problem hiding this comment.
Are you suggesting calling {method} instead of {method}.next()?
There was a problem hiding this comment.
nope ur right should be .next() i will change it. expect a commit in the coming 15 mins addressing everything hopefully
| (sym::iter, []) => iter_next_slice::check(cx, expr, recv2, false), | ||
| (sym::iter_mut, []) => iter_next_slice::check(cx, expr, recv2, true), |
There was a problem hiding this comment.
Pass the method name instead and let the function distinguish between both cases.
| (sym::iter, []) => iter_next_slice::check(cx, expr, recv2, false), | |
| (sym::iter_mut, []) => iter_next_slice::check(cx, expr, recv2, true), | |
| (sym::iter | sym::iter_mut, []) => iter_next_slice::check(cx, expr, recv2, name), |
| let v = vec![1, 2, 3]; | ||
|
|
||
| let _ = s.first(); | ||
| let _ = s.iter(); |
There was a problem hiding this comment.
This makes no sense, you are suggesting using .iter() instead of .iter().next().
There was a problem hiding this comment.
yeah thats my bad that was some old i forgot to change
|
Reminder, once the PR becomes ready for a review, use |
|
@rustbot ready i pushed the changes u wanted @samueltardieu |
| let _: Option<&mut i32> = s_mut.iter_mut().next(); | ||
| //~^ iter_next_slice | ||
|
|
||
| let _ = v_mut.iter_mut().next(); |
There was a problem hiding this comment.
Please add a type here as well.
|
When you're done, could you please squash the commits together? |
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready @samueltardieu i think its good now. Please have a look |
I am a bit surprised, tests fail, which means that you haven't run them locally. |
|
@rustbot ready the tests pass now |
|
But commits are not squashed anymore. |
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready squashed commit |
| parent_expr_opt = get_parent_expr(cx, parent_expr); | ||
| } | ||
|
|
||
| let method = method_name.as_str(); |
There was a problem hiding this comment.
Why do you need to transform it into a &str? This is inefficient as it requires a lock from the interner.
| ITER_NEXT_SLICE, | ||
| expr.span, | ||
| "using `.iter().next()` on a Slice without end index", | ||
| format!("using `.{method}.next()` on a Slice without end index"), |
There was a problem hiding this comment.
| format!("using `.{method}.next()` on a Slice without end index"), | |
| format!("using `.{method_name}.next()` on a Slice without end index"), |
| ITER_NEXT_SLICE, | ||
| expr.span, | ||
| "using `.iter().next()` on an array", | ||
| format!("using `.{method}.next()` on an array"), |
There was a problem hiding this comment.
| format!("using `.{method}.next()` on an array"), | |
| format!("using `.{method_name}.next()` on an array"), |
|
@rustbot ready |
You don't seem to have modified the lint description. @rustbot author |
|
updated the lint description @rustbot ready |
|
thanks alot for ur patience. Certainly learned alot |
View all comments
fixes #17115
.stderrfile)cargo testpasses locallycargo dev update_lintscargo dev fmtchangelog: This PR extends the iter_next_slice lint to also detect and suggest replacements for .iter_mut().next() calls on slices and arrays, providing consistent behavior for mutable iterators.