From 24befed7279ae8be1c52e698172c1366e41e06b1 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Fri, 3 Oct 2025 22:56:05 +0200 Subject: [PATCH 1/2] clean-up - inline `array_span_lint` - use `match` instead of `if-let` - give a more descriptive help message --- clippy_lints/src/zero_repeat_side_effects.rs | 104 ++++++------------- tests/ui/zero_repeat_side_effects.fixed | 4 +- tests/ui/zero_repeat_side_effects.rs | 4 +- tests/ui/zero_repeat_side_effects.stderr | 89 ++++++++++++---- 4 files changed, 105 insertions(+), 96 deletions(-) diff --git a/clippy_lints/src/zero_repeat_side_effects.rs b/clippy_lints/src/zero_repeat_side_effects.rs index 30fdf22fdbb0..e51c4a689816 100644 --- a/clippy_lints/src/zero_repeat_side_effects.rs +++ b/clippy_lints/src/zero_repeat_side_effects.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::VecArgs; use clippy_utils::source::snippet; use clippy_utils::visitors::for_each_expr_without_closures; @@ -7,9 +7,7 @@ use rustc_data_structures::packed::Pu128; use rustc_errors::Applicability; use rustc_hir::{ConstArgKind, ExprKind, Node}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::Ty; use rustc_session::declare_lint_pass; -use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -85,77 +83,39 @@ fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: let parent_hir_node = cx.tcx.parent_hir_node(expr.hir_id); let return_type = cx.typeck_results().expr_ty(expr); - if let Node::LetStmt(l) = parent_hir_node { - array_span_lint( - cx, + let inner_expr = snippet(cx, inner_expr.span.source_callsite(), ".."); + let vec = if is_vec { "vec!" } else { "" }; + + let (span, sugg) = match parent_hir_node { + Node::LetStmt(l) => ( l.span, - inner_expr.span, - l.pat.span, - Some(return_type), - is_vec, - false, - ); - } else if let Node::Expr(x) = parent_hir_node - && let ExprKind::Assign(l, _, _) = x.kind - { - array_span_lint(cx, x.span, inner_expr.span, l.span, Some(return_type), is_vec, true); - } else { - span_lint_and_sugg( - cx, - ZERO_REPEAT_SIDE_EFFECTS, - expr.span.source_callsite(), - "function or method calls as the initial value in zero-sized array initializers may cause side effects", - "consider using", format!( - "{{ {}; {}[] as {return_type} }}", - snippet(cx, inner_expr.span.source_callsite(), ".."), - if is_vec { "vec!" } else { "" }, + "{inner_expr}; let {var_name}: {return_type} = {vec}[];", + var_name = snippet(cx, l.pat.span.source_callsite(), "..") ), - Applicability::Unspecified, - ); - } - } -} - -fn array_span_lint( - cx: &LateContext<'_>, - expr_span: Span, - func_call_span: Span, - variable_name_span: Span, - expr_ty: Option>, - is_vec: bool, - is_assign: bool, -) { - let has_ty = expr_ty.is_some(); - - span_lint_and_sugg( - cx, - ZERO_REPEAT_SIDE_EFFECTS, - expr_span.source_callsite(), - "function or method calls as the initial value in zero-sized array initializers may cause side effects", - "consider using", - format!( - "{}; {}{}{} = {}[]{}{}", - snippet(cx, func_call_span.source_callsite(), ".."), - if has_ty && !is_assign { "let " } else { "" }, - snippet(cx, variable_name_span.source_callsite(), ".."), - if let Some(ty) = expr_ty - && !is_assign - { - format!(": {ty}") - } else { - String::new() - }, - if is_vec { "vec!" } else { "" }, - if let Some(ty) = expr_ty - && is_assign - { - format!(" as {ty}") - } else { - String::new() + ), + Node::Expr(x) if let ExprKind::Assign(l, _, _) = x.kind => ( + x.span, + format!( + "{inner_expr}; {var_name} = {vec}[] as {return_type}", + var_name = snippet(cx, l.span.source_callsite(), "..") + ), + ), + _ => (expr.span, format!("{{ {inner_expr}; {vec}[] as {return_type} }}")), + }; + span_lint_and_then( + cx, + ZERO_REPEAT_SIDE_EFFECTS, + span.source_callsite(), + "function or method calls as the initial value in zero-sized array initializers may cause side effects", + |diag| { + diag.span_suggestion_verbose( + span.source_callsite(), + "consider performing the side effect separately", + sugg, + Applicability::Unspecified, + ); }, - if is_assign { "" } else { ";" } - ), - Applicability::Unspecified, - ); + ); + } } diff --git a/tests/ui/zero_repeat_side_effects.fixed b/tests/ui/zero_repeat_side_effects.fixed index fb9d7880a4a7..5184249e942d 100644 --- a/tests/ui/zero_repeat_side_effects.fixed +++ b/tests/ui/zero_repeat_side_effects.fixed @@ -1,7 +1,5 @@ #![warn(clippy::zero_repeat_side_effects)] -#![allow(clippy::unnecessary_operation)] -#![allow(clippy::useless_vec)] -#![allow(clippy::needless_late_init)] +#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)] fn f() -> i32 { println!("side effect"); diff --git a/tests/ui/zero_repeat_side_effects.rs b/tests/ui/zero_repeat_side_effects.rs index 8b22ff840244..9c6e758f2e28 100644 --- a/tests/ui/zero_repeat_side_effects.rs +++ b/tests/ui/zero_repeat_side_effects.rs @@ -1,7 +1,5 @@ #![warn(clippy::zero_repeat_side_effects)] -#![allow(clippy::unnecessary_operation)] -#![allow(clippy::useless_vec)] -#![allow(clippy::needless_late_init)] +#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)] fn f() -> i32 { println!("side effect"); diff --git a/tests/ui/zero_repeat_side_effects.stderr b/tests/ui/zero_repeat_side_effects.stderr index 2dba52e2112e..dc99110932d8 100644 --- a/tests/ui/zero_repeat_side_effects.stderr +++ b/tests/ui/zero_repeat_side_effects.stderr @@ -1,59 +1,112 @@ error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:18:5 + --> tests/ui/zero_repeat_side_effects.rs:16:5 | LL | let a = [f(); 0]; - | ^^^^^^^^^^^^^^^^^ help: consider using: `f(); let a: [i32; 0] = [];` + | ^^^^^^^^^^^^^^^^^ | = note: `-D clippy::zero-repeat-side-effects` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::zero_repeat_side_effects)]` +help: consider performing the side effect separately + | +LL - let a = [f(); 0]; +LL + f(); let a: [i32; 0] = []; + | error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:21:5 + --> tests/ui/zero_repeat_side_effects.rs:19:5 | LL | b = [f(); 0]; - | ^^^^^^^^^^^^ help: consider using: `f(); b = [] as [i32; 0]` + | ^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - b = [f(); 0]; +LL + f(); b = [] as [i32; 0]; + | error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:26:5 + --> tests/ui/zero_repeat_side_effects.rs:24:5 | LL | let c = vec![f(); 0]; - | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `f(); let c: std::vec::Vec = vec![];` + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - let c = vec![f(); 0]; +LL + f(); let c: std::vec::Vec = vec![]; + | error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:29:5 + --> tests/ui/zero_repeat_side_effects.rs:27:5 | LL | d = vec![f(); 0]; - | ^^^^^^^^^^^^^^^^ help: consider using: `f(); d = vec![] as std::vec::Vec` + | ^^^^^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - d = vec![f(); 0]; +LL + f(); d = vec![] as std::vec::Vec; + | error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:33:5 + --> tests/ui/zero_repeat_side_effects.rs:31:5 | LL | let e = [println!("side effect"); 0]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `println!("side effect"); let e: [(); 0] = [];` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - let e = [println!("side effect"); 0]; +LL + println!("side effect"); let e: [(); 0] = []; + | error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:37:5 + --> tests/ui/zero_repeat_side_effects.rs:35:5 | LL | let g = [{ f() }; 0]; - | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `{ f() }; let g: [i32; 0] = [];` + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - let g = [{ f() }; 0]; +LL + { f() }; let g: [i32; 0] = []; + | error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:41:10 + --> tests/ui/zero_repeat_side_effects.rs:39:10 | LL | drop(vec![f(); 0]); - | ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec }` + | ^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - drop(vec![f(); 0]); +LL + drop({ f(); vec![] as std::vec::Vec }); + | error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:45:5 + --> tests/ui/zero_repeat_side_effects.rs:43:5 | LL | vec![f(); 0]; - | ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec }` + | ^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - vec![f(); 0]; +LL + { f(); vec![] as std::vec::Vec }; + | error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:47:5 + --> tests/ui/zero_repeat_side_effects.rs:45:5 | LL | [f(); 0]; - | ^^^^^^^^ help: consider using: `{ f(); [] as [i32; 0] }` + | ^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - [f(); 0]; +LL + { f(); [] as [i32; 0] }; + | error: aborting due to 9 previous errors From adff9baeb3d16c5d9de8146a6d67f3b276429762 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 4 Oct 2025 00:10:12 +0200 Subject: [PATCH 2/2] fix(zero_repeat_side_effects): better identify exprs with side effects --- clippy_lints/src/zero_repeat_side_effects.rs | 15 ++----- tests/ui/zero_repeat_side_effects.fixed | 24 +++++++++++ tests/ui/zero_repeat_side_effects.rs | 24 +++++++++++ tests/ui/zero_repeat_side_effects.stderr | 44 +++++++++++++++----- 4 files changed, 85 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/zero_repeat_side_effects.rs b/clippy_lints/src/zero_repeat_side_effects.rs index e51c4a689816..cd6c11b51274 100644 --- a/clippy_lints/src/zero_repeat_side_effects.rs +++ b/clippy_lints/src/zero_repeat_side_effects.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::VecArgs; use clippy_utils::source::snippet; -use clippy_utils::visitors::for_each_expr_without_closures; use rustc_ast::LitKind; use rustc_data_structures::packed::Pu128; use rustc_errors::Applicability; @@ -11,7 +10,7 @@ use rustc_session::declare_lint_pass; declare_clippy_lint! { /// ### What it does - /// Checks for array or vec initializations which call a function or method, + /// Checks for array or vec initializations which contain an expression with side effects, /// but which have a repeat count of zero. /// /// ### Why is this bad? @@ -71,15 +70,7 @@ impl LateLintPass<'_> for ZeroRepeatSideEffects { fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: &'_ rustc_hir::Expr<'_>, is_vec: bool) { // check if expr is a call or has a call inside it - if for_each_expr_without_closures(inner_expr, |x| { - if let ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _, _) = x.kind { - std::ops::ControlFlow::Break(()) - } else { - std::ops::ControlFlow::Continue(()) - } - }) - .is_some() - { + if inner_expr.can_have_side_effects() { let parent_hir_node = cx.tcx.parent_hir_node(expr.hir_id); let return_type = cx.typeck_results().expr_ty(expr); @@ -107,7 +98,7 @@ fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: cx, ZERO_REPEAT_SIDE_EFFECTS, span.source_callsite(), - "function or method calls as the initial value in zero-sized array initializers may cause side effects", + "expression with side effects as the initial value in a zero-sized array initializer", |diag| { diag.span_suggestion_verbose( span.source_callsite(), diff --git a/tests/ui/zero_repeat_side_effects.fixed b/tests/ui/zero_repeat_side_effects.fixed index 5184249e942d..e6c451ce7399 100644 --- a/tests/ui/zero_repeat_side_effects.fixed +++ b/tests/ui/zero_repeat_side_effects.fixed @@ -77,3 +77,27 @@ fn issue_13110() { const LENGTH: usize = LEN!(); let _data = [f(); LENGTH]; } + +// TODO: consider moving the defintion+impl inside `issue_14681` +// once https://github.com/rust-lang/rust/issues/146786 is fixed +#[derive(Clone, Copy)] +struct S; + +impl S { + fn new() -> Self { + println!("This is a side effect"); + S + } +} + +// should not trigger on non-function calls +fn issue_14681() { + fn foo(_s: &[Option]) {} + + foo(&[Some(0i64); 0]); + foo(&[Some(Some(0i64)); 0]); + foo(&{ Some(f()); [] as [std::option::Option; 0] }); + //~^ zero_repeat_side_effects + foo(&{ Some(Some(S::new())); [] as [std::option::Option>; 0] }); + //~^ zero_repeat_side_effects +} diff --git a/tests/ui/zero_repeat_side_effects.rs b/tests/ui/zero_repeat_side_effects.rs index 9c6e758f2e28..f8a497976aa4 100644 --- a/tests/ui/zero_repeat_side_effects.rs +++ b/tests/ui/zero_repeat_side_effects.rs @@ -77,3 +77,27 @@ fn issue_13110() { const LENGTH: usize = LEN!(); let _data = [f(); LENGTH]; } + +// TODO: consider moving the defintion+impl inside `issue_14681` +// once https://github.com/rust-lang/rust/issues/146786 is fixed +#[derive(Clone, Copy)] +struct S; + +impl S { + fn new() -> Self { + println!("This is a side effect"); + S + } +} + +// should not trigger on non-function calls +fn issue_14681() { + fn foo(_s: &[Option]) {} + + foo(&[Some(0i64); 0]); + foo(&[Some(Some(0i64)); 0]); + foo(&[Some(f()); 0]); + //~^ zero_repeat_side_effects + foo(&[Some(Some(S::new())); 0]); + //~^ zero_repeat_side_effects +} diff --git a/tests/ui/zero_repeat_side_effects.stderr b/tests/ui/zero_repeat_side_effects.stderr index dc99110932d8..771b71c686ae 100644 --- a/tests/ui/zero_repeat_side_effects.stderr +++ b/tests/ui/zero_repeat_side_effects.stderr @@ -1,4 +1,4 @@ -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:16:5 | LL | let a = [f(); 0]; @@ -12,7 +12,7 @@ LL - let a = [f(); 0]; LL + f(); let a: [i32; 0] = []; | -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:19:5 | LL | b = [f(); 0]; @@ -24,7 +24,7 @@ LL - b = [f(); 0]; LL + f(); b = [] as [i32; 0]; | -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:24:5 | LL | let c = vec![f(); 0]; @@ -36,7 +36,7 @@ LL - let c = vec![f(); 0]; LL + f(); let c: std::vec::Vec = vec![]; | -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:27:5 | LL | d = vec![f(); 0]; @@ -48,7 +48,7 @@ LL - d = vec![f(); 0]; LL + f(); d = vec![] as std::vec::Vec; | -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:31:5 | LL | let e = [println!("side effect"); 0]; @@ -60,7 +60,7 @@ LL - let e = [println!("side effect"); 0]; LL + println!("side effect"); let e: [(); 0] = []; | -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:35:5 | LL | let g = [{ f() }; 0]; @@ -72,7 +72,7 @@ LL - let g = [{ f() }; 0]; LL + { f() }; let g: [i32; 0] = []; | -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:39:10 | LL | drop(vec![f(); 0]); @@ -84,7 +84,7 @@ LL - drop(vec![f(); 0]); LL + drop({ f(); vec![] as std::vec::Vec }); | -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:43:5 | LL | vec![f(); 0]; @@ -96,7 +96,7 @@ LL - vec![f(); 0]; LL + { f(); vec![] as std::vec::Vec }; | -error: function or method calls as the initial value in zero-sized array initializers may cause side effects +error: expression with side effects as the initial value in a zero-sized array initializer --> tests/ui/zero_repeat_side_effects.rs:45:5 | LL | [f(); 0]; @@ -108,5 +108,29 @@ LL - [f(); 0]; LL + { f(); [] as [i32; 0] }; | -error: aborting due to 9 previous errors +error: expression with side effects as the initial value in a zero-sized array initializer + --> tests/ui/zero_repeat_side_effects.rs:99:10 + | +LL | foo(&[Some(f()); 0]); + | ^^^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - foo(&[Some(f()); 0]); +LL + foo(&{ Some(f()); [] as [std::option::Option; 0] }); + | + +error: expression with side effects as the initial value in a zero-sized array initializer + --> tests/ui/zero_repeat_side_effects.rs:101:10 + | +LL | foo(&[Some(Some(S::new())); 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL - foo(&[Some(Some(S::new())); 0]); +LL + foo(&{ Some(Some(S::new())); [] as [std::option::Option>; 0] }); + | + +error: aborting due to 11 previous errors