Skip to content

Commit d8a9068

Browse files
borsbors
committed
Auto merge of #12449 - Jacherr:issue-6439, r=llogiq
Add new lint `zero_repeat_side_effects` Fixes #6439 Adds a new `suspicious` lint zero_repeat_side_effects. This lint warns the user when initializing an array or `Vec` using the `Repeat` syntax, i.e., `[x; y]`. If `x` is an `Expr::Call/MethodCall` or contains an `Expr::Call/MethodCall` and `y` is zero, then there is a chance that the internal call can produce side effects, such as printing to console, which is not very obvious. This lint warns against this and instead suggests to separate out the function call and the array/Vec initialization. changelog: Add new lint `zero_repeat_side_effects`
2 parents c173ea6 + 0c82fd0 commit d8a9068

7 files changed

+355
-0
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5814,6 +5814,7 @@ Released 2018-09-13
58145814
[`zero_divided_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_divided_by_zero
58155815
[`zero_prefixed_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_prefixed_literal
58165816
[`zero_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_ptr
5817+
[`zero_repeat_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_repeat_side_effects
58175818
[`zero_sized_map_values`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_sized_map_values
58185819
[`zero_width_space`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_width_space
58195820
[`zst_offset`]: https://rust-lang.github.io/rust-clippy/master/index.html#zst_offset

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -751,5 +751,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
751751
crate::write::WRITE_LITERAL_INFO,
752752
crate::write::WRITE_WITH_NEWLINE_INFO,
753753
crate::zero_div_zero::ZERO_DIVIDED_BY_ZERO_INFO,
754+
crate::zero_repeat_side_effects::ZERO_REPEAT_SIDE_EFFECTS_INFO,
754755
crate::zero_sized_map_values::ZERO_SIZED_MAP_VALUES_INFO,
755756
];

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ mod visibility;
373373
mod wildcard_imports;
374374
mod write;
375375
mod zero_div_zero;
376+
mod zero_repeat_side_effects;
376377
mod zero_sized_map_values;
377378
// end lints modules, do not remove this comment, it’s used in `update_lints`
378379

@@ -1120,6 +1121,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11201121
store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl));
11211122
store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations));
11221123
store.register_late_pass(|_| Box::new(assigning_clones::AssigningClones));
1124+
store.register_late_pass(|_| Box::new(zero_repeat_side_effects::ZeroRepeatSideEffects));
11231125
// add lints here, do not remove this comment, it's used in `new_lint`
11241126
}
11251127

‎clippy_lints/src/zero_repeat_side_effects.rs

+154
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::higher::VecArgs;
3+
use clippy_utils::source::snippet;
4+
use clippy_utils::visitors::for_each_expr;
5+
use rustc_ast::LitKind;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{ExprKind, Node};
8+
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_middle::ty::{self, ConstKind, Ty};
10+
use rustc_session::declare_lint_pass;
11+
use rustc_span::Span;
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Checks for array or vec initializations which call a function or method,
16+
/// but which have a repeat count of zero.
17+
///
18+
/// ### Why is this bad?
19+
/// Such an initialization, despite having a repeat length of 0, will still call the inner function.
20+
/// This may not be obvious and as such there may be unintended side effects in code.
21+
///
22+
/// ### Example
23+
/// ```no_run
24+
/// fn side_effect() -> i32 {
25+
/// println!("side effect");
26+
/// 10
27+
/// }
28+
/// let a = [side_effect(); 0];
29+
/// ```
30+
/// Use instead:
31+
/// ```no_run
32+
/// fn side_effect() -> i32 {
33+
/// println!("side effect");
34+
/// 10
35+
/// }
36+
/// side_effect();
37+
/// let a: [i32; 0] = [];
38+
/// ```
39+
#[clippy::version = "1.75.0"]
40+
pub ZERO_REPEAT_SIDE_EFFECTS,
41+
suspicious,
42+
"usage of zero-sized initializations of arrays or vecs causing side effects"
43+
}
44+
45+
declare_lint_pass!(ZeroRepeatSideEffects => [ZERO_REPEAT_SIDE_EFFECTS]);
46+
47+
impl LateLintPass<'_> for ZeroRepeatSideEffects {
48+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>) {
49+
if let Some(args) = VecArgs::hir(cx, expr)
50+
&& let VecArgs::Repeat(inner_expr, len) = args
51+
&& let ExprKind::Lit(l) = len.kind
52+
&& let LitKind::Int(i, _) = l.node
53+
&& i.0 == 0
54+
{
55+
inner_check(cx, expr, inner_expr, true);
56+
} else if let ExprKind::Repeat(inner_expr, _) = expr.kind
57+
&& let ty::Array(_, cst) = cx.typeck_results().expr_ty(expr).kind()
58+
&& let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind()
59+
&& let Ok(element_count) = element_count.try_to_target_usize(cx.tcx)
60+
&& element_count == 0
61+
{
62+
inner_check(cx, expr, inner_expr, false);
63+
}
64+
}
65+
}
66+
67+
fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: &'_ rustc_hir::Expr<'_>, is_vec: bool) {
68+
// check if expr is a call or has a call inside it
69+
if for_each_expr(inner_expr, |x| {
70+
if let ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _, _) = x.kind {
71+
std::ops::ControlFlow::Break(())
72+
} else {
73+
std::ops::ControlFlow::Continue(())
74+
}
75+
})
76+
.is_some()
77+
{
78+
let parent_hir_node = cx.tcx.parent_hir_node(expr.hir_id);
79+
let return_type = cx.typeck_results().expr_ty(expr);
80+
81+
if let Node::Local(l) = parent_hir_node {
82+
array_span_lint(
83+
cx,
84+
l.span,
85+
inner_expr.span,
86+
l.pat.span,
87+
Some(return_type),
88+
is_vec,
89+
false,
90+
);
91+
} else if let Node::Expr(x) = parent_hir_node
92+
&& let ExprKind::Assign(l, _, _) = x.kind
93+
{
94+
array_span_lint(cx, x.span, inner_expr.span, l.span, Some(return_type), is_vec, true);
95+
} else {
96+
span_lint_and_sugg(
97+
cx,
98+
ZERO_REPEAT_SIDE_EFFECTS,
99+
expr.span.source_callsite(),
100+
"function or method calls as the initial value in zero-sized array initializers may cause side effects",
101+
"consider using",
102+
format!(
103+
"{{ {}; {}[] as {return_type} }}",
104+
snippet(cx, inner_expr.span.source_callsite(), ".."),
105+
if is_vec { "vec!" } else { "" },
106+
),
107+
Applicability::Unspecified,
108+
);
109+
}
110+
}
111+
}
112+
113+
fn array_span_lint(
114+
cx: &LateContext<'_>,
115+
expr_span: Span,
116+
func_call_span: Span,
117+
variable_name_span: Span,
118+
expr_ty: Option<Ty<'_>>,
119+
is_vec: bool,
120+
is_assign: bool,
121+
) {
122+
let has_ty = expr_ty.is_some();
123+
124+
span_lint_and_sugg(
125+
cx,
126+
ZERO_REPEAT_SIDE_EFFECTS,
127+
expr_span.source_callsite(),
128+
"function or method calls as the initial value in zero-sized array initializers may cause side effects",
129+
"consider using",
130+
format!(
131+
"{}; {}{}{} = {}[]{}{}",
132+
snippet(cx, func_call_span.source_callsite(), ".."),
133+
if has_ty && !is_assign { "let " } else { "" },
134+
snippet(cx, variable_name_span.source_callsite(), ".."),
135+
if let Some(ty) = expr_ty
136+
&& !is_assign
137+
{
138+
format!(": {ty}")
139+
} else {
140+
String::new()
141+
},
142+
if is_vec { "vec!" } else { "" },
143+
if let Some(ty) = expr_ty
144+
&& is_assign
145+
{
146+
format!(" as {ty}")
147+
} else {
148+
String::new()
149+
},
150+
if is_assign { "" } else { ";" }
151+
),
152+
Applicability::Unspecified,
153+
);
154+
}

‎tests/ui/zero_repeat_side_effects.fixed

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#![warn(clippy::zero_repeat_side_effects)]
2+
#![allow(clippy::unnecessary_operation)]
3+
#![allow(clippy::useless_vec)]
4+
#![allow(clippy::needless_late_init)]
5+
6+
fn f() -> i32 {
7+
println!("side effect");
8+
10
9+
}
10+
11+
fn main() {
12+
const N: usize = 0;
13+
const M: usize = 1;
14+
15+
// should trigger
16+
17+
// on arrays
18+
f(); let a: [i32; 0] = [];
19+
f(); let a: [i32; 0] = [];
20+
let mut b;
21+
f(); b = [] as [i32; 0];
22+
f(); b = [] as [i32; 0];
23+
24+
// on vecs
25+
// vecs dont support infering value of consts
26+
f(); let c: std::vec::Vec<i32> = vec![];
27+
let d;
28+
f(); d = vec![] as std::vec::Vec<i32>;
29+
30+
// for macros
31+
println!("side effect"); let e: [(); 0] = [];
32+
33+
// for nested calls
34+
{ f() }; let g: [i32; 0] = [];
35+
36+
// as function param
37+
drop({ f(); vec![] as std::vec::Vec<i32> });
38+
39+
// when singled out/not part of assignment/local
40+
{ f(); vec![] as std::vec::Vec<i32> };
41+
{ f(); [] as [i32; 0] };
42+
{ f(); [] as [i32; 0] };
43+
44+
// should not trigger
45+
46+
// on arrays with > 0 repeat
47+
let a = [f(); 1];
48+
let a = [f(); M];
49+
let mut b;
50+
b = [f(); 1];
51+
b = [f(); M];
52+
53+
// on vecs with > 0 repeat
54+
let c = vec![f(); 1];
55+
let d;
56+
d = vec![f(); 1];
57+
58+
// as function param
59+
drop(vec![f(); 1]);
60+
}

‎tests/ui/zero_repeat_side_effects.rs

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#![warn(clippy::zero_repeat_side_effects)]
2+
#![allow(clippy::unnecessary_operation)]
3+
#![allow(clippy::useless_vec)]
4+
#![allow(clippy::needless_late_init)]
5+
6+
fn f() -> i32 {
7+
println!("side effect");
8+
10
9+
}
10+
11+
fn main() {
12+
const N: usize = 0;
13+
const M: usize = 1;
14+
15+
// should trigger
16+
17+
// on arrays
18+
let a = [f(); 0];
19+
let a = [f(); N];
20+
let mut b;
21+
b = [f(); 0];
22+
b = [f(); N];
23+
24+
// on vecs
25+
// vecs dont support infering value of consts
26+
let c = vec![f(); 0];
27+
let d;
28+
d = vec![f(); 0];
29+
30+
// for macros
31+
let e = [println!("side effect"); 0];
32+
33+
// for nested calls
34+
let g = [{ f() }; 0];
35+
36+
// as function param
37+
drop(vec![f(); 0]);
38+
39+
// when singled out/not part of assignment/local
40+
vec![f(); 0];
41+
[f(); 0];
42+
[f(); N];
43+
44+
// should not trigger
45+
46+
// on arrays with > 0 repeat
47+
let a = [f(); 1];
48+
let a = [f(); M];
49+
let mut b;
50+
b = [f(); 1];
51+
b = [f(); M];
52+
53+
// on vecs with > 0 repeat
54+
let c = vec![f(); 1];
55+
let d;
56+
d = vec![f(); 1];
57+
58+
// as function param
59+
drop(vec![f(); 1]);
60+
}

‎tests/ui/zero_repeat_side_effects.stderr

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
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
3+
|
4+
LL | let a = [f(); 0];
5+
| ^^^^^^^^^^^^^^^^^ help: consider using: `f(); let a: [i32; 0] = [];`
6+
|
7+
= note: `-D clippy::zero-repeat-side-effects` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::zero_repeat_side_effects)]`
9+
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:19:5
12+
|
13+
LL | let a = [f(); N];
14+
| ^^^^^^^^^^^^^^^^^ help: consider using: `f(); let a: [i32; 0] = [];`
15+
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:21:5
18+
|
19+
LL | b = [f(); 0];
20+
| ^^^^^^^^^^^^ help: consider using: `f(); b = [] as [i32; 0]`
21+
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:22:5
24+
|
25+
LL | b = [f(); N];
26+
| ^^^^^^^^^^^^ help: consider using: `f(); b = [] as [i32; 0]`
27+
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:26:5
30+
|
31+
LL | let c = vec![f(); 0];
32+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `f(); let c: std::vec::Vec<i32> = vec![];`
33+
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:28:5
36+
|
37+
LL | d = vec![f(); 0];
38+
| ^^^^^^^^^^^^^^^^ help: consider using: `f(); d = vec![] as std::vec::Vec<i32>`
39+
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:31:5
42+
|
43+
LL | let e = [println!("side effect"); 0];
44+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `println!("side effect"); let e: [(); 0] = [];`
45+
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:34:5
48+
|
49+
LL | let g = [{ f() }; 0];
50+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `{ f() }; let g: [i32; 0] = [];`
51+
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:37:10
54+
|
55+
LL | drop(vec![f(); 0]);
56+
| ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec<i32> }`
57+
58+
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
59+
--> tests/ui/zero_repeat_side_effects.rs:40:5
60+
|
61+
LL | vec![f(); 0];
62+
| ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec<i32> }`
63+
64+
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
65+
--> tests/ui/zero_repeat_side_effects.rs:41:5
66+
|
67+
LL | [f(); 0];
68+
| ^^^^^^^^ help: consider using: `{ f(); [] as [i32; 0] }`
69+
70+
error: function or method calls as the initial value in zero-sized array initializers may cause side effects
71+
--> tests/ui/zero_repeat_side_effects.rs:42:5
72+
|
73+
LL | [f(); N];
74+
| ^^^^^^^^ help: consider using: `{ f(); [] as [i32; 0] }`
75+
76+
error: aborting due to 12 previous errors
77+

0 commit comments

Comments
 (0)