From f2af204d17aad5264372218c58dd7d87a6769307 Mon Sep 17 00:00:00 2001 From: yanglsh Date: Fri, 10 Oct 2025 00:16:12 +0800 Subject: [PATCH] fix: `zero_repeat_side_effects` misses curlies --- clippy_lints/src/zero_repeat_side_effects.rs | 72 ++++++++++++++++---- tests/ui/zero_repeat_side_effects.fixed | 32 ++++++++- tests/ui/zero_repeat_side_effects.rs | 26 ++++++- tests/ui/zero_repeat_side_effects.stderr | 52 ++++++++++---- 4 files changed, 151 insertions(+), 31 deletions(-) diff --git a/clippy_lints/src/zero_repeat_side_effects.rs b/clippy_lints/src/zero_repeat_side_effects.rs index a8351690068d..95085161c09c 100644 --- a/clippy_lints/src/zero_repeat_side_effects.rs +++ b/clippy_lints/src/zero_repeat_side_effects.rs @@ -4,10 +4,11 @@ use clippy_utils::source::{snippet, snippet_indent}; use rustc_ast::LitKind; use rustc_data_structures::packed::Pu128; use rustc_errors::Applicability; -use rustc_hir::{ConstArgKind, ExprKind, Node}; +use rustc_hir::{ConstArgKind, Expr, ExprKind, LetStmt, LocalSource, Node}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::IsSuggestable; +use rustc_middle::ty::{IsSuggestable, Ty}; use rustc_session::declare_lint_pass; +use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -44,7 +45,7 @@ declare_clippy_lint! { declare_lint_pass!(ZeroRepeatSideEffects => [ZERO_REPEAT_SIDE_EFFECTS]); impl LateLintPass<'_> for ZeroRepeatSideEffects { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &rustc_hir::Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { if let Some(args) = VecArgs::hir(cx, expr) && let VecArgs::Repeat(inner_expr, len) = args && let ExprKind::Lit(l) = len.kind @@ -69,7 +70,7 @@ impl LateLintPass<'_> for ZeroRepeatSideEffects { } } -fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: &'_ rustc_hir::Expr<'_>, is_vec: bool) { +fn inner_check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, inner_expr: &'_ Expr<'_>, is_vec: bool) { // check if expr is a call or has a call inside it if inner_expr.can_have_side_effects() { 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: let vec = if is_vec { "vec!" } else { "" }; let (span, sugg) = match parent_hir_node { - Node::LetStmt(l) => ( - l.span, - format!( - "{inner_expr};\n{indent}let {var_name}: {return_type} = {vec}[];", - var_name = snippet(cx, l.pat.span.source_callsite(), "..") - ), - ), + Node::LetStmt(l) + if matches!(l.source, LocalSource::AssignDesugar) + && let mut parent_iter = cx.tcx.hir_parent_iter(l.hir_id) + && let Some((_, Node::Stmt(_))) = parent_iter.next() + && let Some((_, Node::Block(_))) = parent_iter.next() + && let Some((_, Node::Expr(x))) = parent_iter.next() => + { + ( + x.span, + assign_expr_suggestion(cx, x, l.pat.span, &inner_expr, return_type, vec), + ) + }, + Node::LetStmt(l) => (l.span, let_stmt_suggestion(cx, l, &inner_expr, return_type, vec)), Node::Expr(x) if let ExprKind::Assign(l, _, _) = x.kind => ( x.span, - format!( - "{inner_expr};\n{indent}{var_name} = {vec}[] as {return_type}", - var_name = snippet(cx, l.span.source_callsite(), "..") - ), + assign_expr_suggestion(cx, x, l.span, &inner_expr, return_type, vec), ), // NOTE: don't use the stmt span to avoid touching the trailing semicolon 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: ); } } + +fn let_stmt_suggestion( + cx: &LateContext<'_>, + let_stmt: &LetStmt<'_>, + inner_expr: &str, + return_type: Ty<'_>, + vec_str: &str, +) -> String { + let indent = snippet_indent(cx, let_stmt.span).unwrap_or_default(); + format!( + "{inner_expr};\n{}let {var_name}: {return_type} = {vec_str}[];", + indent, + var_name = snippet(cx, let_stmt.pat.span.source_callsite(), "..") + ) +} + +fn assign_expr_suggestion( + cx: &LateContext<'_>, + outer_expr: &Expr<'_>, + assign_expr_span: Span, + inner_expr: &str, + return_type: Ty<'_>, + vec_str: &str, +) -> String { + let mut parent_hir_node = cx.tcx.parent_hir_node(outer_expr.hir_id); + if let Node::Stmt(stmt) = parent_hir_node { + parent_hir_node = cx.tcx.parent_hir_node(stmt.hir_id); + } + let needs_curly = !matches!(parent_hir_node, Node::Block(_)); + + let indent = snippet_indent(cx, outer_expr.span).unwrap_or_default(); + let var_name = snippet(cx, assign_expr_span.source_callsite(), ".."); + if needs_curly { + format!("{{\n {indent}{inner_expr};\n {indent}{var_name} = {vec_str}[] as {return_type}\n{indent}}}",) + } else { + format!("{inner_expr};\n{indent}{var_name} = {vec_str}[] as {return_type}") + } +} diff --git a/tests/ui/zero_repeat_side_effects.fixed b/tests/ui/zero_repeat_side_effects.fixed index b5fca36f3f08..4bdff6fd0f6c 100644 --- a/tests/ui/zero_repeat_side_effects.fixed +++ b/tests/ui/zero_repeat_side_effects.fixed @@ -1,6 +1,11 @@ #![warn(clippy::zero_repeat_side_effects)] -#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)] -#![allow(clippy::no_effect)] // only fires _after_ the fix +#![allow( + clippy::unnecessary_operation, + clippy::useless_vec, + clippy::needless_late_init, + clippy::single_match, + clippy::no_effect // only fires _after_ the fix +)] fn f() -> i32 { println!("side effect"); @@ -119,3 +124,26 @@ fn issue_14681() { }); //~^ zero_repeat_side_effects } + +fn issue_15824() { + fn f() {} + + match 0 { + 0 => { + f(); + _ = [] as [(); 0] + }, + //~^ zero_repeat_side_effects + _ => {}, + } + + let mut a = [(); 0]; + match 0 { + 0 => { + f(); + a = [] as [(); 0] + }, + //~^ zero_repeat_side_effects + _ => {}, + } +} diff --git a/tests/ui/zero_repeat_side_effects.rs b/tests/ui/zero_repeat_side_effects.rs index ea043d21638c..a1454d724c87 100644 --- a/tests/ui/zero_repeat_side_effects.rs +++ b/tests/ui/zero_repeat_side_effects.rs @@ -1,6 +1,11 @@ #![warn(clippy::zero_repeat_side_effects)] -#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)] -#![allow(clippy::no_effect)] // only fires _after_ the fix +#![allow( + clippy::unnecessary_operation, + clippy::useless_vec, + clippy::needless_late_init, + clippy::single_match, + clippy::no_effect // only fires _after_ the fix +)] fn f() -> i32 { println!("side effect"); @@ -102,3 +107,20 @@ fn issue_14681() { foo(&[Some(Some(S::new())); 0]); //~^ zero_repeat_side_effects } + +fn issue_15824() { + fn f() {} + + match 0 { + 0 => _ = [f(); 0], + //~^ zero_repeat_side_effects + _ => {}, + } + + let mut a = [(); 0]; + match 0 { + 0 => a = [f(); 0], + //~^ zero_repeat_side_effects + _ => {}, + } +} diff --git a/tests/ui/zero_repeat_side_effects.stderr b/tests/ui/zero_repeat_side_effects.stderr index 49e850d03534..f376a1501b00 100644 --- a/tests/ui/zero_repeat_side_effects.stderr +++ b/tests/ui/zero_repeat_side_effects.stderr @@ -1,5 +1,5 @@ error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:17:5 + --> tests/ui/zero_repeat_side_effects.rs:22:5 | LL | let a = [f(); 0]; | ^^^^^^^^^^^^^^^^^ @@ -13,7 +13,7 @@ LL + let a: [i32; 0] = []; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:20:5 + --> tests/ui/zero_repeat_side_effects.rs:25:5 | LL | b = [f(); 0]; | ^^^^^^^^^^^^ @@ -25,7 +25,7 @@ LL ~ b = [] as [i32; 0]; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:25:5 + --> tests/ui/zero_repeat_side_effects.rs:30:5 | LL | let c = vec![f(); 0]; | ^^^^^^^^^^^^^^^^^^^^^ @@ -37,7 +37,7 @@ LL + let c: std::vec::Vec = vec![]; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:28:5 + --> tests/ui/zero_repeat_side_effects.rs:33:5 | LL | d = vec![f(); 0]; | ^^^^^^^^^^^^^^^^ @@ -49,7 +49,7 @@ LL ~ d = vec![] as std::vec::Vec; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:32:5 + --> tests/ui/zero_repeat_side_effects.rs:37:5 | LL | let e = [println!("side effect"); 0]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -61,7 +61,7 @@ LL + let e: [(); 0] = []; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:36:5 + --> tests/ui/zero_repeat_side_effects.rs:41:5 | LL | let g = [{ f() }; 0]; | ^^^^^^^^^^^^^^^^^^^^^ @@ -73,7 +73,7 @@ LL + let g: [i32; 0] = []; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:40:10 + --> tests/ui/zero_repeat_side_effects.rs:45:10 | LL | drop(vec![f(); 0]); | ^^^^^^^^^^^^ @@ -87,7 +87,7 @@ LL ~ }); | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:44:5 + --> tests/ui/zero_repeat_side_effects.rs:49:5 | LL | vec![f(); 0]; | ^^^^^^^^^^^^ @@ -99,7 +99,7 @@ LL ~ vec![] as std::vec::Vec; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:46:5 + --> tests/ui/zero_repeat_side_effects.rs:51:5 | LL | [f(); 0]; | ^^^^^^^^ @@ -111,7 +111,7 @@ LL ~ [] as [i32; 0]; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:100:10 + --> tests/ui/zero_repeat_side_effects.rs:105:10 | LL | foo(&[Some(f()); 0]); | ^^^^^^^^^^^^^^ @@ -125,7 +125,7 @@ LL ~ }); | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:102:10 + --> tests/ui/zero_repeat_side_effects.rs:107:10 | LL | foo(&[Some(Some(S::new())); 0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -138,5 +138,33 @@ LL + [] as [std::option::Option>; 0] LL ~ }); | -error: aborting due to 11 previous errors +error: expression with side effects as the initial value in a zero-sized array initializer + --> tests/ui/zero_repeat_side_effects.rs:115:14 + | +LL | 0 => _ = [f(); 0], + | ^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL ~ 0 => { +LL + f(); +LL + _ = [] as [(); 0] +LL ~ }, + | + +error: expression with side effects as the initial value in a zero-sized array initializer + --> tests/ui/zero_repeat_side_effects.rs:122:14 + | +LL | 0 => a = [f(); 0], + | ^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL ~ 0 => { +LL + f(); +LL + a = [] as [(); 0] +LL ~ }, + | + +error: aborting due to 13 previous errors