hir_typeck: suggest removing redundant .iter() and iter_mut() calls - #153700
vishnupoddar12 wants to merge 1 commit into
Conversation
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
8f0debd to
7dc8033
Compare
vishnupoddar12
changed the title
There was a problem hiding this comment.
You're doing a lot of Span wrangling and dealing with code snippets to try and create a suitable Span for the suggestion, but instead you should be leveraging the hir Exprs to find the right spans. you can do things like rcvr.span.between(arg[0].span) to construct a span that points at
rcvr.method(arg0, arg1);
^^^^^^^^
This diagnostic intercepts cases where `.iter()` is called on a type that already implements `Iterator` Instead of confusing trait fallback suggestions, it explicitly guides the user to remove the `.iter()` call. It implements precise span computation to support `cargo fix` while carefully preserving all inline comments and horizontal whitespace during the AST string replacement.
7dc8033 to
f67a0cf
Compare
|
Thanks @estebank, I was able to reduce lot of unnecessary span wrangling reducing code change by 30-40 lines. It should be cleaner now. |
|
Hi @estebank, Friendly ping! I updated the PR to use |
|
@estebank I suppose this is very low priority bug causing the delay. If not, I just want to understand how to proceed on this PR. |
|
Hi @estebank, it looks like this PR is ready for another round of review when you get a chance. Thanks! |
| // Between receiver and method name (the part containing the dot) | ||
| let dot_part_span = rcvr_expr.span.between(span); | ||
| let dot_snippet = self.tcx.sess.source_map().span_to_snippet(dot_part_span).ok()?; | ||
| let dot_offset = Self::find_last_real_dot(&dot_snippet)?; | ||
|
|
||
| let replacement_span = sugg_span | ||
| .with_lo(dot_part_span.lo() + rustc_span::BytePos(u32::try_from(dot_offset).ok()?)); | ||
|
|
||
| // A comment in the section being removed risks deleting user documentation, | ||
| // so we downgrade applicability to signal that human review is warranted. | ||
| let deleted_snippet = self.tcx.sess.source_map().span_to_snippet(replacement_span).ok()?; | ||
| let applicability = if deleted_snippet.contains("//") || deleted_snippet.contains("/*") { | ||
| Applicability::MaybeIncorrect | ||
| } else { | ||
| Applicability::MachineApplicable | ||
| }; | ||
|
|
||
| Some((replacement_span, applicability)) |
There was a problem hiding this comment.
Like @estebank suggested, this can be implemented as a one-liner using rcvr_expr.span and sugg_span. span_to_snippet is almost never a good solution.
cjgillot
added
S-waiting-on-author
vishnupoddar12
changed the title
|
☔ The latest upstream changes (presumably #163439) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
This branch has not been deployed
6 participants
This diagnostic intercepts cases where
.iter()or.iter_mut()is called on a type that already implementsIteratorInstead of confusing trait fallback suggestions, it explicitly guides the user to remove the
.iter()call. It implements precise span computation to supportcargo fixwhile carefully preserving all inline comments and horizontal whitespace during the AST string replacement.Fixes #153667