[iter_overeager_cloned]: detect .cloned().map() and .cloned().for_each() #11319
Conversation
|
r? @llogiq (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
lengyijun
force-pushed
the
map_foreach
branch
2 times, most recently
from
e277f63
to
dd9d4fa
Compare
lengyijun
force-pushed
the
map_foreach
branch
3 times, most recently
from
3029f37
to
87da7b3
Compare
| if let rustc_hir::ExprKind::Closure(closure) = expr.kind | ||
| && let body@ Body {params: [p], .. } = cx.tcx.hir().body(closure.body) { | ||
| match p.pat.kind { | ||
| rustc_hir::PatKind::Binding(BindingAnnotation(_, Mutability::Not), _, _, _) => { |
There was a problem hiding this comment.
We should probably expand this to other parameters as well, by using pat.walk and checking for any Bindings. If they're all used, we could lint; though I'm not sure how to handle this, maybe only if one is used?
The reason why we can't use each_binding is that it gives us the HirId of the Pat, not the binding, so we technically can but it's not pretty
Is there any reason we should lint only Binding that I'm not aware of?
lengyijun
force-pushed
the
map_foreach
branch
2 times, most recently
from
4d755c6
to
4802375
Compare
|
☔ The latest upstream changes (presumably #11289) made this pull request unmergeable. Please resolve the merge conflicts. |
lengyijun
force-pushed
the
map_foreach
branch
5 times, most recently
from
e1ebe56
to
52c50a5
Compare
lengyijun
changed the title
|
r? @blyxyas |
| @@ -51,8 +59,46 @@ pub(super) fn check<'tcx>( | |||
| return; | |||
| } | |||
|
|
|||
| if let Op::NeedlessMove(_, expr) = op { | |||
| let rustc_hir::ExprKind::Closure(closure) = expr.kind else { return } ; | |||
| let body@ Body {params: [p], .. } = cx.tcx.hir().body(closure.body) else { return }; | |||
There was a problem hiding this comment.
I think this destructuring could mean this only lints when the body has only one param.
Is this intended? If so, some optimization could be done in that walk method.
Maybe I'm wrong, am not with an IDE at the moment to corroborate this pattern destructuring.
There was a problem hiding this comment.
map and for_each require a closure of only one param
…ach() key idea: for `f` in `.map(f)` and `.for_each(f)`: 1. `f` must be a closure with one parameter 2. don't lint if mutable paramter in clsure `f`: `|mut x| ...` 3. don't lint if parameter is moved
|
@bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: [
iter_overeager_cloned]key idea:
for
fin.map(f)and.for_each(f):fmust be closuref:|mut x| ...