Skip to content

Commit 4180830

Browse files
blyxyasblyxyas
authored
Fix: zero_repeat_side_effects misses curlies (rust-lang#15853)
Closes rust-lang#15824 Closes rust-lang#15825 changelog: [`zero_repeat_side_effects`] fix missing curlies
2 parents a10cafe + f2af204 commit 4180830

File tree

4 files changed

+151
-31
lines changed

4 files changed

+151
-31
lines changed

‎clippy_lints/src/zero_repeat_side_effects.rs‎

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ use clippy_utils::source::{snippet, snippet_indent};
44
use rustc_ast::LitKind;
55
use rustc_data_structures::packed::Pu128;
66
use rustc_errors::Applicability;
7-
use rustc_hir::{ConstArgKind, ExprKind, Node};
7+
use rustc_hir::{ConstArgKind, Expr, ExprKind, LetStmt, LocalSource, Node};
88
use rustc_lint::{LateContext, LateLintPass};
9-
use rustc_middle::ty::IsSuggestable;
9+
use rustc_middle::ty::{IsSuggestable, Ty};
1010
use rustc_session::declare_lint_pass;
11+
use rustc_span::Span;
1112

1213
declare_clippy_lint! {
1314
/// ### What it does
@@ -44,7 +45,7 @@ declare_clippy_lint! {
4445
declare_lint_pass!(ZeroRepeatSideEffects => [ZERO_REPEAT_SIDE_EFFECTS]);
4546

4647
impl LateLintPass<'_> for ZeroRepeatSideEffects {
47-
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &rustc_hir::Expr<'_>) {
48+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
4849
if let Some(args) = VecArgs::hir(cx, expr)
4950
&& let VecArgs::Repeat(inner_expr, len) = args
5051
&& let ExprKind::Lit(l) = len.kind
@@ -69,7 +70,7 @@ impl LateLintPass<'_> for ZeroRepeatSideEffects {
6970
}
7071
}
7172

72-
fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: &'_ rustc_hir::Expr<'_>, is_vec: bool) {
73+
fn inner_check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, inner_expr: &'_ Expr<'_>, is_vec: bool) {
7374
// check if expr is a call or has a call inside it
7475
if inner_expr.can_have_side_effects() {
7576
let parent_hir_node = cx.tcx.parent_hir_node(expr.hir_id);
@@ -81,19 +82,22 @@ fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr:
8182
let vec = if is_vec { "vec!" } else { "" };
8283

8384
let (span, sugg) = match parent_hir_node {
84-
Node::LetStmt(l) => (
85-
l.span,
86-
format!(
87-
"{inner_expr};\n{indent}let {var_name}: {return_type} = {vec}[];",
88-
var_name = snippet(cx, l.pat.span.source_callsite(), "..")
89-
),
90-
),
85+
Node::LetStmt(l)
86+
if matches!(l.source, LocalSource::AssignDesugar)
87+
&& let mut parent_iter = cx.tcx.hir_parent_iter(l.hir_id)
88+
&& let Some((_, Node::Stmt(_))) = parent_iter.next()
89+
&& let Some((_, Node::Block(_))) = parent_iter.next()
90+
&& let Some((_, Node::Expr(x))) = parent_iter.next() =>
91+
{
92+
(
93+
x.span,
94+
assign_expr_suggestion(cx, x, l.pat.span, &inner_expr, return_type, vec),
95+
)
96+
},
97+
Node::LetStmt(l) => (l.span, let_stmt_suggestion(cx, l, &inner_expr, return_type, vec)),
9198
Node::Expr(x) if let ExprKind::Assign(l, _, _) = x.kind => (
9299
x.span,
93-
format!(
94-
"{inner_expr};\n{indent}{var_name} = {vec}[] as {return_type}",
95-
var_name = snippet(cx, l.span.source_callsite(), "..")
96-
),
100+
assign_expr_suggestion(cx, x, l.span, &inner_expr, return_type, vec),
97101
),
98102
// NOTE: don't use the stmt span to avoid touching the trailing semicolon
99103
Node::Stmt(_) => (expr.span, format!("{inner_expr};\n{indent}{vec}[] as {return_type}")),
@@ -131,3 +135,41 @@ fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr:
131135
);
132136
}
133137
}
138+
139+
fn let_stmt_suggestion(
140+
cx: &LateContext<'_>,
141+
let_stmt: &LetStmt<'_>,
142+
inner_expr: &str,
143+
return_type: Ty<'_>,
144+
vec_str: &str,
145+
) -> String {
146+
let indent = snippet_indent(cx, let_stmt.span).unwrap_or_default();
147+
format!(
148+
"{inner_expr};\n{}let {var_name}: {return_type} = {vec_str}[];",
149+
indent,
150+
var_name = snippet(cx, let_stmt.pat.span.source_callsite(), "..")
151+
)
152+
}
153+
154+
fn assign_expr_suggestion(
155+
cx: &LateContext<'_>,
156+
outer_expr: &Expr<'_>,
157+
assign_expr_span: Span,
158+
inner_expr: &str,
159+
return_type: Ty<'_>,
160+
vec_str: &str,
161+
) -> String {
162+
let mut parent_hir_node = cx.tcx.parent_hir_node(outer_expr.hir_id);
163+
if let Node::Stmt(stmt) = parent_hir_node {
164+
parent_hir_node = cx.tcx.parent_hir_node(stmt.hir_id);
165+
}
166+
let needs_curly = !matches!(parent_hir_node, Node::Block(_));
167+
168+
let indent = snippet_indent(cx, outer_expr.span).unwrap_or_default();
169+
let var_name = snippet(cx, assign_expr_span.source_callsite(), "..");
170+
if needs_curly {
171+
format!("{{\n {indent}{inner_expr};\n {indent}{var_name} = {vec_str}[] as {return_type}\n{indent}}}",)
172+
} else {
173+
format!("{inner_expr};\n{indent}{var_name} = {vec_str}[] as {return_type}")
174+
}
175+
}

‎tests/ui/zero_repeat_side_effects.fixed‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
#![warn(clippy::zero_repeat_side_effects)]
2-
#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)]
3-
#![allow(clippy::no_effect)] // only fires _after_ the fix
2+
#![allow(
3+
clippy::unnecessary_operation,
4+
clippy::useless_vec,
5+
clippy::needless_late_init,
6+
clippy::single_match,
7+
clippy::no_effect // only fires _after_ the fix
8+
)]
49

510
fn f() -> i32 {
611
println!("side effect");
@@ -119,3 +124,26 @@ fn issue_14681() {
119124
});
120125
//~^ zero_repeat_side_effects
121126
}
127+
128+
fn issue_15824() {
129+
fn f() {}
130+
131+
match 0 {
132+
0 => {
133+
f();
134+
_ = [] as [(); 0]
135+
},
136+
//~^ zero_repeat_side_effects
137+
_ => {},
138+
}
139+
140+
let mut a = [(); 0];
141+
match 0 {
142+
0 => {
143+
f();
144+
a = [] as [(); 0]
145+
},
146+
//~^ zero_repeat_side_effects
147+
_ => {},
148+
}
149+
}

‎tests/ui/zero_repeat_side_effects.rs‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
#![warn(clippy::zero_repeat_side_effects)]
2-
#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)]
3-
#![allow(clippy::no_effect)] // only fires _after_ the fix
2+
#![allow(
3+
clippy::unnecessary_operation,
4+
clippy::useless_vec,
5+
clippy::needless_late_init,
6+
clippy::single_match,
7+
clippy::no_effect // only fires _after_ the fix
8+
)]
49

510
fn f() -> i32 {
611
println!("side effect");
@@ -102,3 +107,20 @@ fn issue_14681() {
102107
foo(&[Some(Some(S::new())); 0]);
103108
//~^ zero_repeat_side_effects
104109
}
110+
111+
fn issue_15824() {
112+
fn f() {}
113+
114+
match 0 {
115+
0 => _ = [f(); 0],
116+
//~^ zero_repeat_side_effects
117+
_ => {},
118+
}
119+
120+
let mut a = [(); 0];
121+
match 0 {
122+
0 => a = [f(); 0],
123+
//~^ zero_repeat_side_effects
124+
_ => {},
125+
}
126+
}

‎tests/ui/zero_repeat_side_effects.stderr‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: expression with side effects as the initial value in a zero-sized array initializer
2-
--> tests/ui/zero_repeat_side_effects.rs:17:5
2+
--> tests/ui/zero_repeat_side_effects.rs:22:5
33
|
44
LL | let a = [f(); 0];
55
| ^^^^^^^^^^^^^^^^^
@@ -13,7 +13,7 @@ LL + let a: [i32; 0] = [];
1313
|
1414

1515
error: expression with side effects as the initial value in a zero-sized array initializer
16-
--> tests/ui/zero_repeat_side_effects.rs:20:5
16+
--> tests/ui/zero_repeat_side_effects.rs:25:5
1717
|
1818
LL | b = [f(); 0];
1919
| ^^^^^^^^^^^^
@@ -25,7 +25,7 @@ LL ~ b = [] as [i32; 0];
2525
|
2626

2727
error: expression with side effects as the initial value in a zero-sized array initializer
28-
--> tests/ui/zero_repeat_side_effects.rs:25:5
28+
--> tests/ui/zero_repeat_side_effects.rs:30:5
2929
|
3030
LL | let c = vec![f(); 0];
3131
| ^^^^^^^^^^^^^^^^^^^^^
@@ -37,7 +37,7 @@ LL + let c: std::vec::Vec<i32> = vec![];
3737
|
3838

3939
error: expression with side effects as the initial value in a zero-sized array initializer
40-
--> tests/ui/zero_repeat_side_effects.rs:28:5
40+
--> tests/ui/zero_repeat_side_effects.rs:33:5
4141
|
4242
LL | d = vec![f(); 0];
4343
| ^^^^^^^^^^^^^^^^
@@ -49,7 +49,7 @@ LL ~ d = vec![] as std::vec::Vec<i32>;
4949
|
5050

5151
error: expression with side effects as the initial value in a zero-sized array initializer
52-
--> tests/ui/zero_repeat_side_effects.rs:32:5
52+
--> tests/ui/zero_repeat_side_effects.rs:37:5
5353
|
5454
LL | let e = [println!("side effect"); 0];
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -61,7 +61,7 @@ LL + let e: [(); 0] = [];
6161
|
6262

6363
error: expression with side effects as the initial value in a zero-sized array initializer
64-
--> tests/ui/zero_repeat_side_effects.rs:36:5
64+
--> tests/ui/zero_repeat_side_effects.rs:41:5
6565
|
6666
LL | let g = [{ f() }; 0];
6767
| ^^^^^^^^^^^^^^^^^^^^^
@@ -73,7 +73,7 @@ LL + let g: [i32; 0] = [];
7373
|
7474

7575
error: expression with side effects as the initial value in a zero-sized array initializer
76-
--> tests/ui/zero_repeat_side_effects.rs:40:10
76+
--> tests/ui/zero_repeat_side_effects.rs:45:10
7777
|
7878
LL | drop(vec![f(); 0]);
7979
| ^^^^^^^^^^^^
@@ -87,7 +87,7 @@ LL ~ });
8787
|
8888

8989
error: expression with side effects as the initial value in a zero-sized array initializer
90-
--> tests/ui/zero_repeat_side_effects.rs:44:5
90+
--> tests/ui/zero_repeat_side_effects.rs:49:5
9191
|
9292
LL | vec![f(); 0];
9393
| ^^^^^^^^^^^^
@@ -99,7 +99,7 @@ LL ~ vec![] as std::vec::Vec<i32>;
9999
|
100100

101101
error: expression with side effects as the initial value in a zero-sized array initializer
102-
--> tests/ui/zero_repeat_side_effects.rs:46:5
102+
--> tests/ui/zero_repeat_side_effects.rs:51:5
103103
|
104104
LL | [f(); 0];
105105
| ^^^^^^^^
@@ -111,7 +111,7 @@ LL ~ [] as [i32; 0];
111111
|
112112

113113
error: expression with side effects as the initial value in a zero-sized array initializer
114-
--> tests/ui/zero_repeat_side_effects.rs:100:10
114+
--> tests/ui/zero_repeat_side_effects.rs:105:10
115115
|
116116
LL | foo(&[Some(f()); 0]);
117117
| ^^^^^^^^^^^^^^
@@ -125,7 +125,7 @@ LL ~ });
125125
|
126126

127127
error: expression with side effects as the initial value in a zero-sized array initializer
128-
--> tests/ui/zero_repeat_side_effects.rs:102:10
128+
--> tests/ui/zero_repeat_side_effects.rs:107:10
129129
|
130130
LL | foo(&[Some(Some(S::new())); 0]);
131131
| ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -138,5 +138,33 @@ LL + [] as [std::option::Option<std::option::Option<S>>; 0]
138138
LL ~ });
139139
|
140140

141-
error: aborting due to 11 previous errors
141+
error: expression with side effects as the initial value in a zero-sized array initializer
142+
--> tests/ui/zero_repeat_side_effects.rs:115:14
143+
|
144+
LL | 0 => _ = [f(); 0],
145+
| ^^^^^^^^^^^^
146+
|
147+
help: consider performing the side effect separately
148+
|
149+
LL ~ 0 => {
150+
LL + f();
151+
LL + _ = [] as [(); 0]
152+
LL ~ },
153+
|
154+
155+
error: expression with side effects as the initial value in a zero-sized array initializer
156+
--> tests/ui/zero_repeat_side_effects.rs:122:14
157+
|
158+
LL | 0 => a = [f(); 0],
159+
| ^^^^^^^^^^^^
160+
|
161+
help: consider performing the side effect separately
162+
|
163+
LL ~ 0 => {
164+
LL + f();
165+
LL + a = [] as [(); 0]
166+
LL ~ },
167+
|
168+
169+
error: aborting due to 13 previous errors
142170

0 commit comments

Comments
 (0)