Improve useless_conversion .into_iter() suggestion for nested references
#16238
Conversation
rustbot
added
the
S-waiting-on-review
There was a problem hiding this comment.
Pull request overview
This PR refines the useless_conversion lint to fix over-borrowed suggestions when dealing with .into_iter() calls on nested references. The core change modifies the adjustments function to include early-break logic that stops processing adjustments once the target type is reached.
- Adds early break logic in the
adjustmentsfunction to stop at the target type - Introduces a regression test for
.into_iter()on nested slice references (&&[T])
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
clippy_lints/src/useless_conversion.rs |
Modified adjustments function to break early when reaching the target type, preventing over-application of adjustment prefixes |
tests/ui/useless_conversion_into_iter_adjustments.rs |
Added regression test for issue #14847 testing .into_iter() on &&[i32] |
tests/ui/useless_conversion_into_iter_adjustments.stderr |
Expected test output showing the corrected suggestion (*items instead of over-borrowed alternatives) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let target = adjustments.last().unwrap().target; | ||
|
|
||
| // stop once we reach the target type to avoid unnecessary adjustments. | ||
| for adj in adjustments { | ||
| match adj.kind { | ||
| Adjust::Deref(_) => prefix = format!("*{prefix}"), | ||
| Adjust::Borrow(AutoBorrow::Ref(AutoBorrowMutability::Mut { .. })) => prefix = format!("&mut {prefix}"), | ||
| Adjust::Borrow(AutoBorrow::Ref(AutoBorrowMutability::Not)) => prefix = format!("&{prefix}"), | ||
| _ => {}, | ||
| } | ||
|
|
||
| if adj.target == target { | ||
| break; | ||
| } |
There was a problem hiding this comment.
The logic and comment here may be misleading. The target variable is set to adjustments.last().unwrap().target, which means the break condition adj.target == target will only be true for the last adjustment in the iteration. This results in processing all adjustments before breaking, which appears to be the same behavior as not having the early break.
If the intent is to stop at a specific target type (to avoid over-applying adjustments as mentioned in the PR description), consider whether target should be determined differently - perhaps based on the type requirements of the conversion context, rather than simply being the last adjustment's target. Otherwise, clarify the comment to explain what specific case this break is handling.
|
| if adjustments.is_empty() { | ||
| return prefix; | ||
| } | ||
|
|
||
| let target = adjustments.last().unwrap().target; |
There was a problem hiding this comment.
This can be simplified to:
let [.., last] = adjustments else { return prefix };
let mut target = last.target;or
let [.., Adjustment { mut target, .. }] = *adjustments else { return prefix };
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
3 participants
Description
I updated the
useless_conversionlint to stop applying adjustment prefixes once it reaches the final target type.Previously, the lint would continue applying adjustments even after the type requirements were met, which often resulted in over-borrowed suggestions like
&**y. By breaking the loop early once the target type is reached, we now emit the minimal, idiomatic suggestion (e.g.,*y).fixes #14847
Test Plan
I added a targeted UI regression test:
tests/ui/useless_conversion_into_iter_adjustments.rs.This covers
.into_iter()usage on nested slice references (&&[T]) and verifies that Clippy now suggests*itemsinstead of the previous incorrect suggestion.Checklist
.stderrfile)cargo testpasses locallycargo dev fmtchangelog: [
useless_conversion]: refine.into_iter()suggestions to stop at the final target type (fixing over-borrowed suggestions like&**y)