Skip to content

Commit e577327

Browse files
committed
fix: needless_for_each suggests wrongly when closure has no braces
1 parent 50e0bf1 commit e577327

File tree

4 files changed

+46
-4
lines changed

4 files changed

+46
-4
lines changed

clippy_lints/src/needless_for_each.rs

+27-3
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,21 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach {
9999
)
100100
};
101101

102+
let body_param_sugg = snippet_with_applicability(cx, body.params[0].pat.span, "..", &mut applicability);
103+
let for_each_rev_sugg = snippet_with_applicability(cx, for_each_recv.span, "..", &mut applicability);
104+
let body_value_sugg = snippet_with_applicability(cx, body.value.span, "..", &mut applicability);
105+
102106
let sugg = format!(
103107
"for {} in {} {}",
104-
snippet_with_applicability(cx, body.params[0].pat.span, "..", &mut applicability),
105-
snippet_with_applicability(cx, for_each_recv.span, "..", &mut applicability),
106-
snippet_with_applicability(cx, body.value.span, "..", &mut applicability),
108+
body_param_sugg,
109+
for_each_rev_sugg,
110+
if let ExprKind::Block(block, _) = body.value.kind
111+
&& is_let_desugar(block)
112+
{
113+
format!("{{ {body_value_sugg} }}")
114+
} else {
115+
body_value_sugg.to_string()
116+
}
107117
);
108118

109119
span_lint_and_then(cx, NEEDLESS_FOR_EACH, stmt.span, "needless use of `for_each`", |diag| {
@@ -116,6 +126,20 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach {
116126
}
117127
}
118128

129+
/// Check if the block is a desugared `_ = expr` statement.
130+
fn is_let_desugar(block: &Block<'_>) -> bool {
131+
matches!(
132+
block,
133+
Block {
134+
stmts: [Stmt {
135+
kind: StmtKind::Let(_),
136+
..
137+
},],
138+
..
139+
}
140+
)
141+
}
142+
119143
/// This type plays two roles.
120144
/// 1. Collect spans of `return` in the closure body.
121145
/// 2. Detect use of `return` in `Loop` in the closure body.

tests/ui/needless_for_each_fixable.fixed

+6
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,9 @@ fn should_not_lint() {
128128
}
129129

130130
fn main() {}
131+
132+
fn issue14734(rows: &[u8]) {
133+
let mut v = vec![];
134+
for x in rows.iter() { _ = v.push(x) }
135+
//~^ needless_for_each
136+
}

tests/ui/needless_for_each_fixable.rs

+6
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,9 @@ fn should_not_lint() {
128128
}
129129

130130
fn main() {}
131+
132+
fn issue14734(rows: &[u8]) {
133+
let mut v = vec![];
134+
rows.iter().for_each(|x| _ = v.push(x));
135+
//~^ needless_for_each
136+
}

tests/ui/needless_for_each_fixable.stderr

+7-1
Original file line numberDiff line numberDiff line change
@@ -136,5 +136,11 @@ LL + acc += elem;
136136
LL + }
137137
|
138138

139-
error: aborting due to 8 previous errors
139+
error: needless use of `for_each`
140+
--> tests/ui/needless_for_each_fixable.rs:134:5
141+
|
142+
LL | rows.iter().for_each(|x| _ = v.push(x));
143+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in rows.iter() { _ = v.push(x) }`
144+
145+
error: aborting due to 9 previous errors
140146

0 commit comments

Comments
 (0)