Skip to content

Commit 19343bf

Browse files
samueltardieusamueltardieu
authored
fix(zero_repeat_side_effects): better identify exprs with side effects (#15814)
Fixes rust-lang/rust-clippy#14681 changelog: [`zero_repeat_side_effects`]: better identify exprs with side effects
2 parents 0a7b328 + adff9ba commit 19343bf

File tree

4 files changed

+189
-117
lines changed

4 files changed

+189
-117
lines changed

‎clippy_lints/src/zero_repeat_side_effects.rs‎

Lines changed: 34 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,16 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
1+
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::higher::VecArgs;
33
use clippy_utils::source::snippet;
4-
use clippy_utils::visitors::for_each_expr_without_closures;
54
use rustc_ast::LitKind;
65
use rustc_data_structures::packed::Pu128;
76
use rustc_errors::Applicability;
87
use rustc_hir::{ConstArgKind, ExprKind, Node};
98
use rustc_lint::{LateContext, LateLintPass};
10-
use rustc_middle::ty::Ty;
119
use rustc_session::declare_lint_pass;
12-
use rustc_span::Span;
1310

1411
declare_clippy_lint! {
1512
/// ### What it does
16-
/// Checks for array or vec initializations which call a function or method,
13+
/// Checks for array or vec initializations which contain an expression with side effects,
1714
/// but which have a repeat count of zero.
1815
///
1916
/// ### Why is this bad?
@@ -73,89 +70,43 @@ impl LateLintPass<'_> for ZeroRepeatSideEffects {
7370

7471
fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: &'_ rustc_hir::Expr<'_>, is_vec: bool) {
7572
// check if expr is a call or has a call inside it
76-
if for_each_expr_without_closures(inner_expr, |x| {
77-
if let ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _, _) = x.kind {
78-
std::ops::ControlFlow::Break(())
79-
} else {
80-
std::ops::ControlFlow::Continue(())
81-
}
82-
})
83-
.is_some()
84-
{
73+
if inner_expr.can_have_side_effects() {
8574
let parent_hir_node = cx.tcx.parent_hir_node(expr.hir_id);
8675
let return_type = cx.typeck_results().expr_ty(expr);
8776

88-
if let Node::LetStmt(l) = parent_hir_node {
89-
array_span_lint(
90-
cx,
77+
let inner_expr = snippet(cx, inner_expr.span.source_callsite(), "..");
78+
let vec = if is_vec { "vec!" } else { "" };
79+
80+
let (span, sugg) = match parent_hir_node {
81+
Node::LetStmt(l) => (
9182
l.span,
92-
inner_expr.span,
93-
l.pat.span,
94-
Some(return_type),
95-
is_vec,
96-
false,
97-
);
98-
} else if let Node::Expr(x) = parent_hir_node
99-
&& let ExprKind::Assign(l, _, _) = x.kind
100-
{
101-
array_span_lint(cx, x.span, inner_expr.span, l.span, Some(return_type), is_vec, true);
102-
} else {
103-
span_lint_and_sugg(
104-
cx,
105-
ZERO_REPEAT_SIDE_EFFECTS,
106-
expr.span.source_callsite(),
107-
"function or method calls as the initial value in zero-sized array initializers may cause side effects",
108-
"consider using",
10983
format!(
110-
"{{ {}; {}[] as {return_type} }}",
111-
snippet(cx, inner_expr.span.source_callsite(), ".."),
112-
if is_vec { "vec!" } else { "" },
84+
"{inner_expr}; let {var_name}: {return_type} = {vec}[];",
85+
var_name = snippet(cx, l.pat.span.source_callsite(), "..")
11386
),
114-
Applicability::Unspecified,
115-
);
116-
}
117-
}
118-
}
119-
120-
fn array_span_lint(
121-
cx: &LateContext<'_>,
122-
expr_span: Span,
123-
func_call_span: Span,
124-
variable_name_span: Span,
125-
expr_ty: Option<Ty<'_>>,
126-
is_vec: bool,
127-
is_assign: bool,
128-
) {
129-
let has_ty = expr_ty.is_some();
130-
131-
span_lint_and_sugg(
132-
cx,
133-
ZERO_REPEAT_SIDE_EFFECTS,
134-
expr_span.source_callsite(),
135-
"function or method calls as the initial value in zero-sized array initializers may cause side effects",
136-
"consider using",
137-
format!(
138-
"{}; {}{}{} = {}[]{}{}",
139-
snippet(cx, func_call_span.source_callsite(), ".."),
140-
if has_ty && !is_assign { "let " } else { "" },
141-
snippet(cx, variable_name_span.source_callsite(), ".."),
142-
if let Some(ty) = expr_ty
143-
&& !is_assign
144-
{
145-
format!(": {ty}")
146-
} else {
147-
String::new()
148-
},
149-
if is_vec { "vec!" } else { "" },
150-
if let Some(ty) = expr_ty
151-
&& is_assign
152-
{
153-
format!(" as {ty}")
154-
} else {
155-
String::new()
87+
),
88+
Node::Expr(x) if let ExprKind::Assign(l, _, _) = x.kind => (
89+
x.span,
90+
format!(
91+
"{inner_expr}; {var_name} = {vec}[] as {return_type}",
92+
var_name = snippet(cx, l.span.source_callsite(), "..")
93+
),
94+
),
95+
_ => (expr.span, format!("{{ {inner_expr}; {vec}[] as {return_type} }}")),
96+
};
97+
span_lint_and_then(
98+
cx,
99+
ZERO_REPEAT_SIDE_EFFECTS,
100+
span.source_callsite(),
101+
"expression with side effects as the initial value in a zero-sized array initializer",
102+
|diag| {
103+
diag.span_suggestion_verbose(
104+
span.source_callsite(),
105+
"consider performing the side effect separately",
106+
sugg,
107+
Applicability::Unspecified,
108+
);
156109
},
157-
if is_assign { "" } else { ";" }
158-
),
159-
Applicability::Unspecified,
160-
);
110+
);
111+
}
161112
}

‎tests/ui/zero_repeat_side_effects.fixed‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
#![warn(clippy::zero_repeat_side_effects)]
2-
#![allow(clippy::unnecessary_operation)]
3-
#![allow(clippy::useless_vec)]
4-
#![allow(clippy::needless_late_init)]
2+
#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)]
53

64
fn f() -> i32 {
75
println!("side effect");
@@ -79,3 +77,27 @@ fn issue_13110() {
7977
const LENGTH: usize = LEN!();
8078
let _data = [f(); LENGTH];
8179
}
80+
81+
// TODO: consider moving the defintion+impl inside `issue_14681`
82+
// once https://github.com/rust-lang/rust/issues/146786 is fixed
83+
#[derive(Clone, Copy)]
84+
struct S;
85+
86+
impl S {
87+
fn new() -> Self {
88+
println!("This is a side effect");
89+
S
90+
}
91+
}
92+
93+
// should not trigger on non-function calls
94+
fn issue_14681() {
95+
fn foo<T>(_s: &[Option<T>]) {}
96+
97+
foo(&[Some(0i64); 0]);
98+
foo(&[Some(Some(0i64)); 0]);
99+
foo(&{ Some(f()); [] as [std::option::Option<i32>; 0] });
100+
//~^ zero_repeat_side_effects
101+
foo(&{ Some(Some(S::new())); [] as [std::option::Option<std::option::Option<S>>; 0] });
102+
//~^ zero_repeat_side_effects
103+
}

‎tests/ui/zero_repeat_side_effects.rs‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
#![warn(clippy::zero_repeat_side_effects)]
2-
#![allow(clippy::unnecessary_operation)]
3-
#![allow(clippy::useless_vec)]
4-
#![allow(clippy::needless_late_init)]
2+
#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)]
53

64
fn f() -> i32 {
75
println!("side effect");
@@ -79,3 +77,27 @@ fn issue_13110() {
7977
const LENGTH: usize = LEN!();
8078
let _data = [f(); LENGTH];
8179
}
80+
81+
// TODO: consider moving the defintion+impl inside `issue_14681`
82+
// once https://github.com/rust-lang/rust/issues/146786 is fixed
83+
#[derive(Clone, Copy)]
84+
struct S;
85+
86+
impl S {
87+
fn new() -> Self {
88+
println!("This is a side effect");
89+
S
90+
}
91+
}
92+
93+
// should not trigger on non-function calls
94+
fn issue_14681() {
95+
fn foo<T>(_s: &[Option<T>]) {}
96+
97+
foo(&[Some(0i64); 0]);
98+
foo(&[Some(Some(0i64)); 0]);
99+
foo(&[Some(f()); 0]);
100+
//~^ zero_repeat_side_effects
101+
foo(&[Some(Some(S::new())); 0]);
102+
//~^ zero_repeat_side_effects
103+
}

‎tests/ui/zero_repeat_side_effects.stderr‎

Lines changed: 105 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,136 @@
1-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
2-
--> tests/ui/zero_repeat_side_effects.rs:18:5
1+
error: expression with side effects as the initial value in a zero-sized array initializer
2+
--> tests/ui/zero_repeat_side_effects.rs:16:5
33
|
44
LL | let a = [f(); 0];
5-
| ^^^^^^^^^^^^^^^^^ help: consider using: `f(); let a: [i32; 0] = [];`
5+
| ^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::zero-repeat-side-effects` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::zero_repeat_side_effects)]`
9+
help: consider performing the side effect separately
10+
|
11+
LL - let a = [f(); 0];
12+
LL + f(); let a: [i32; 0] = [];
13+
|
914

10-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
11-
--> tests/ui/zero_repeat_side_effects.rs:21:5
15+
error: expression with side effects as the initial value in a zero-sized array initializer
16+
--> tests/ui/zero_repeat_side_effects.rs:19:5
1217
|
1318
LL | b = [f(); 0];
14-
| ^^^^^^^^^^^^ help: consider using: `f(); b = [] as [i32; 0]`
19+
| ^^^^^^^^^^^^
20+
|
21+
help: consider performing the side effect separately
22+
|
23+
LL - b = [f(); 0];
24+
LL + f(); b = [] as [i32; 0];
25+
|
1526

16-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
17-
--> tests/ui/zero_repeat_side_effects.rs:26:5
27+
error: expression with side effects as the initial value in a zero-sized array initializer
28+
--> tests/ui/zero_repeat_side_effects.rs:24:5
1829
|
1930
LL | let c = vec![f(); 0];
20-
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `f(); let c: std::vec::Vec<i32> = vec![];`
31+
| ^^^^^^^^^^^^^^^^^^^^^
32+
|
33+
help: consider performing the side effect separately
34+
|
35+
LL - let c = vec![f(); 0];
36+
LL + f(); let c: std::vec::Vec<i32> = vec![];
37+
|
2138

22-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
23-
--> tests/ui/zero_repeat_side_effects.rs:29:5
39+
error: expression with side effects as the initial value in a zero-sized array initializer
40+
--> tests/ui/zero_repeat_side_effects.rs:27:5
2441
|
2542
LL | d = vec![f(); 0];
26-
| ^^^^^^^^^^^^^^^^ help: consider using: `f(); d = vec![] as std::vec::Vec<i32>`
43+
| ^^^^^^^^^^^^^^^^
44+
|
45+
help: consider performing the side effect separately
46+
|
47+
LL - d = vec![f(); 0];
48+
LL + f(); d = vec![] as std::vec::Vec<i32>;
49+
|
2750

28-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
29-
--> tests/ui/zero_repeat_side_effects.rs:33:5
51+
error: expression with side effects as the initial value in a zero-sized array initializer
52+
--> tests/ui/zero_repeat_side_effects.rs:31:5
3053
|
3154
LL | let e = [println!("side effect"); 0];
32-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `println!("side effect"); let e: [(); 0] = [];`
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
56+
|
57+
help: consider performing the side effect separately
58+
|
59+
LL - let e = [println!("side effect"); 0];
60+
LL + println!("side effect"); let e: [(); 0] = [];
61+
|
3362

34-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
35-
--> tests/ui/zero_repeat_side_effects.rs:37:5
63+
error: expression with side effects as the initial value in a zero-sized array initializer
64+
--> tests/ui/zero_repeat_side_effects.rs:35:5
3665
|
3766
LL | let g = [{ f() }; 0];
38-
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `{ f() }; let g: [i32; 0] = [];`
67+
| ^^^^^^^^^^^^^^^^^^^^^
68+
|
69+
help: consider performing the side effect separately
70+
|
71+
LL - let g = [{ f() }; 0];
72+
LL + { f() }; let g: [i32; 0] = [];
73+
|
3974

40-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
41-
--> tests/ui/zero_repeat_side_effects.rs:41:10
75+
error: expression with side effects as the initial value in a zero-sized array initializer
76+
--> tests/ui/zero_repeat_side_effects.rs:39:10
4277
|
4378
LL | drop(vec![f(); 0]);
44-
| ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec<i32> }`
79+
| ^^^^^^^^^^^^
80+
|
81+
help: consider performing the side effect separately
82+
|
83+
LL - drop(vec![f(); 0]);
84+
LL + drop({ f(); vec![] as std::vec::Vec<i32> });
85+
|
4586

46-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
47-
--> tests/ui/zero_repeat_side_effects.rs:45:5
87+
error: expression with side effects as the initial value in a zero-sized array initializer
88+
--> tests/ui/zero_repeat_side_effects.rs:43:5
4889
|
4990
LL | vec![f(); 0];
50-
| ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec<i32> }`
91+
| ^^^^^^^^^^^^
92+
|
93+
help: consider performing the side effect separately
94+
|
95+
LL - vec![f(); 0];
96+
LL + { f(); vec![] as std::vec::Vec<i32> };
97+
|
5198

52-
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
53-
--> tests/ui/zero_repeat_side_effects.rs:47:5
99+
error: expression with side effects as the initial value in a zero-sized array initializer
100+
--> tests/ui/zero_repeat_side_effects.rs:45:5
54101
|
55102
LL | [f(); 0];
56-
| ^^^^^^^^ help: consider using: `{ f(); [] as [i32; 0] }`
103+
| ^^^^^^^^
104+
|
105+
help: consider performing the side effect separately
106+
|
107+
LL - [f(); 0];
108+
LL + { f(); [] as [i32; 0] };
109+
|
110+
111+
error: expression with side effects as the initial value in a zero-sized array initializer
112+
--> tests/ui/zero_repeat_side_effects.rs:99:10
113+
|
114+
LL | foo(&[Some(f()); 0]);
115+
| ^^^^^^^^^^^^^^
116+
|
117+
help: consider performing the side effect separately
118+
|
119+
LL - foo(&[Some(f()); 0]);
120+
LL + foo(&{ Some(f()); [] as [std::option::Option<i32>; 0] });
121+
|
122+
123+
error: expression with side effects as the initial value in a zero-sized array initializer
124+
--> tests/ui/zero_repeat_side_effects.rs:101:10
125+
|
126+
LL | foo(&[Some(Some(S::new())); 0]);
127+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
128+
|
129+
help: consider performing the side effect separately
130+
|
131+
LL - foo(&[Some(Some(S::new())); 0]);
132+
LL + foo(&{ Some(Some(S::new())); [] as [std::option::Option<std::option::Option<S>>; 0] });
133+
|
57134

58-
error: aborting due to 9 previous errors
135+
error: aborting due to 11 previous errors
59136

0 commit comments

Comments
 (0)