diff --git a/clippy_lints/src/zero_repeat_side_effects.rs b/clippy_lints/src/zero_repeat_side_effects.rs index 6f2bfab1a..fa1cc41d1 100644 --- a/clippy_lints/src/zero_repeat_side_effects.rs +++ b/clippy_lints/src/zero_repeat_side_effects.rs @@ -3,11 +3,11 @@ 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; -use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{ArrayLen, ExprKind, ItemKind, Node, QPath}; +use rustc_hir::{ArrayLen, ExprKind, Node}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, ConstKind, Ty}; +use rustc_middle::ty::Ty; use rustc_session::declare_lint_pass; use rustc_span::Span; @@ -51,39 +51,23 @@ impl LateLintPass<'_> for ZeroRepeatSideEffects { if let Some(args) = VecArgs::hir(cx, expr) && let VecArgs::Repeat(inner_expr, len) = args && let ExprKind::Lit(l) = len.kind - && let LitKind::Int(i, _) = l.node - && i.0 == 0 + && let LitKind::Int(Pu128(0), _) = l.node { inner_check(cx, expr, inner_expr, true); - } else { - let ExprKind::Repeat(inner_expr, length) = expr.kind else { - return; - }; - // Skip if the length is from a macro. - if let ArrayLen::Body(anon_const) = length { - let length_expr = hir_map.body(anon_const.body).value; - if !length_expr.span.eq_ctxt(expr.span) { - return; - } - - // Get the initialization span of a const item and compare it with the span at use-site. - if let ExprKind::Path(QPath::Resolved(None, path)) = length_expr.kind - && let Res::Def(DefKind::Const, def_id) = path.res - && let Some(def_id) = def_id.as_local() - && let Node::Item(item) = cx.tcx.hir_node_by_def_id(def_id) - && let ItemKind::Const(_ty, _, body_id) = item.kind - && let init_span = hir_map.span_with_body(body_id.hir_id) - && !init_span.eq_ctxt(length_expr.span) - { - return; - } - } - if let ty::Array(_, cst) = cx.typeck_results().expr_ty(expr).kind() - && let ConstKind::Value(_, ty::ValTree::Leaf(element_count)) = cst.kind() - && element_count.to_target_usize(cx.tcx) == 0 - { - inner_check(cx, expr, inner_expr, false); - } + } + // Lint only if the length is a literal zero, and not a path to any constants. + // NOTE(@y21): When reading `[f(); LEN]`, I intuitively expect that the function is called and it + // doesn't seem as confusing as `[f(); 0]`. It would also have false positives when eg. + // the const item depends on `#[cfg]s` and has different values in different compilation + // sessions). + else if let ExprKind::Repeat(inner_expr, length) = expr.kind + && let ArrayLen::Body(anon_const) = length + && let length_expr = hir_map.body(anon_const.body).value + && !length_expr.span.from_expansion() + && let ExprKind::Lit(literal) = length_expr.kind + && let LitKind::Int(Pu128(0), _) = literal.node + { + inner_check(cx, expr, inner_expr, false); } } } diff --git a/tests/ui/zero_repeat_side_effects.fixed b/tests/ui/zero_repeat_side_effects.fixed index 75ebc6f35..2f71d1752 100644 --- a/tests/ui/zero_repeat_side_effects.fixed +++ b/tests/ui/zero_repeat_side_effects.fixed @@ -16,10 +16,8 @@ fn main() { // on arrays f(); let a: [i32; 0] = []; - f(); let a: [i32; 0] = []; let mut b; f(); b = [] as [i32; 0]; - f(); b = [] as [i32; 0]; // on vecs // vecs dont support infering value of consts @@ -39,9 +37,11 @@ fn main() { // when singled out/not part of assignment/local { f(); vec![] as std::vec::Vec }; { f(); [] as [i32; 0] }; - { f(); [] as [i32; 0] }; // should not trigger + let a = [f(); N]; + b = [f(); N]; + [f(); N]; // on arrays with > 0 repeat let a = [f(); 1]; diff --git a/tests/ui/zero_repeat_side_effects.rs b/tests/ui/zero_repeat_side_effects.rs index 1a1a9f93d..ed5cdd1ae 100644 --- a/tests/ui/zero_repeat_side_effects.rs +++ b/tests/ui/zero_repeat_side_effects.rs @@ -16,10 +16,8 @@ fn main() { // on arrays let a = [f(); 0]; - let a = [f(); N]; let mut b; b = [f(); 0]; - b = [f(); N]; // on vecs // vecs dont support infering value of consts @@ -39,9 +37,11 @@ fn main() { // when singled out/not part of assignment/local vec![f(); 0]; [f(); 0]; - [f(); N]; // should not trigger + let a = [f(); N]; + b = [f(); N]; + [f(); N]; // on arrays with > 0 repeat let a = [f(); 1]; diff --git a/tests/ui/zero_repeat_side_effects.stderr b/tests/ui/zero_repeat_side_effects.stderr index afdc60542..d578e22b9 100644 --- a/tests/ui/zero_repeat_side_effects.stderr +++ b/tests/ui/zero_repeat_side_effects.stderr @@ -8,70 +8,52 @@ LL | let a = [f(); 0]; = help: to override `-D warnings` add `#[allow(clippy::zero_repeat_side_effects)]` 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:19:5 - | -LL | let a = [f(); N]; - | ^^^^^^^^^^^^^^^^^ help: consider using: `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:20:5 | LL | b = [f(); 0]; | ^^^^^^^^^^^^ help: consider using: `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:22:5 - | -LL | b = [f(); N]; - | ^^^^^^^^^^^^ help: consider using: `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![];` 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:28:5 + --> tests/ui/zero_repeat_side_effects.rs:26:5 | LL | d = vec![f(); 0]; | ^^^^^^^^^^^^^^^^ help: consider using: `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:31:5 + --> tests/ui/zero_repeat_side_effects.rs:29:5 | LL | let e = [println!("side effect"); 0]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `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:34:5 + --> tests/ui/zero_repeat_side_effects.rs:32:5 | LL | let g = [{ f() }; 0]; | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `{ 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:37:10 + --> tests/ui/zero_repeat_side_effects.rs:35:10 | LL | drop(vec![f(); 0]); | ^^^^^^^^^^^^ help: consider using: `{ 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:40:5 + --> tests/ui/zero_repeat_side_effects.rs:38:5 | LL | vec![f(); 0]; | ^^^^^^^^^^^^ help: consider using: `{ 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:41:5 + --> tests/ui/zero_repeat_side_effects.rs:39:5 | LL | [f(); 0]; | ^^^^^^^^ help: consider using: `{ f(); [] 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:42:5 - | -LL | [f(); N]; - | ^^^^^^^^ help: consider using: `{ f(); [] as [i32; 0] }` - -error: aborting due to 12 previous errors +error: aborting due to 9 previous errors