add new lint zero_repeat_side_effects

CHANGELOG.md

@@ -5814,6 +5814,7 @@ Released 2018-09-13
 [`zero_divided_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_divided_by_zero
 [`zero_prefixed_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_prefixed_literal
 [`zero_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_ptr
+[`zero_repeat_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_repeat_side_effects
 [`zero_sized_map_values`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_sized_map_values
 [`zero_width_space`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_width_space
 [`zst_offset`]: https://rust-lang.github.io/rust-clippy/master/index.html#zst_offset

clippy_lints/src/declared_lints.rs

@@ -751,5 +751,6 @@
     crate::write::WRITE_LITERAL_INFO,
     crate::write::WRITE_WITH_NEWLINE_INFO,
     crate::zero_div_zero::ZERO_DIVIDED_BY_ZERO_INFO,
+    crate::zero_repeat_side_effects::ZERO_REPEAT_SIDE_EFFECTS_INFO,
     crate::zero_sized_map_values::ZERO_SIZED_MAP_VALUES_INFO,
 ];

clippy_lints/src/lib.rs

@@ -373,6 +373,7 @@
 mod wildcard_imports;
 mod write;
 mod zero_div_zero;
+mod zero_repeat_side_effects;
 mod zero_sized_map_values;
 // end lints modules, do not remove this comment, it’s used in `update_lints`
 
@@ -1120,6 +1121,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl));
     store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations));
     store.register_late_pass(|_| Box::new(assigning_clones::AssigningClones));
+    store.register_late_pass(|_| Box::new(zero_repeat_side_effects::ZeroRepeatSideEffects));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/zero_repeat_side_effects.rs

@@ -0,0 +1,154 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::higher::VecArgs;
+use clippy_utils::source::snippet;
+use clippy_utils::visitors::for_each_expr;
+use rustc_ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{ExprKind, Node};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{self, ConstKind, Ty};
+use rustc_session::declare_lint_pass;
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for array or vec initializations which call a function or method,
+    /// but which have a repeat count of zero.
+    ///
+    /// ### Why is this bad?
+    /// Such an initialization, despite having a repeat length of 0, will still call the inner function.
+    /// This may not be obvious and as such there may be unintended side effects in code.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn side_effect(): i32 {
+    ///     println!("side effect");
+    ///     10
+    /// }
+    /// let a = [side_effect(); 0];
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn side_effect(): i32 {
+    ///     println!("side effect");
+    ///     10
+    /// }
+    /// side_effect();
+    /// let a: [i32; 0] = [];
+    /// ```
+    #[clippy::version = "1.75.0"]
+    pub ZERO_REPEAT_SIDE_EFFECTS,
+    suspicious,
+    "usage of zero-sized initializations of arrays or vecs causing side effects"
+}
+
+declare_lint_pass!(ZeroRepeatSideEffects => [ZERO_REPEAT_SIDE_EFFECTS]);
+
+impl LateLintPass<'_> for ZeroRepeatSideEffects {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>) {
+        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
+        {
+            inner_check(cx, expr, inner_expr, true);
+        } else if let ExprKind::Repeat(inner_expr, _) = expr.kind
+            && let ty::Array(_, cst) = cx.typeck_results().expr_ty(expr).kind()
+            && let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind()
+            && let Ok(element_count) = element_count.try_to_target_usize(cx.tcx)
+            && element_count == 0
+        {
+            inner_check(cx, expr, inner_expr, false);
+        }
+    }
+}
+
+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(inner_expr, |x| {
+        if let ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _, _) = x.kind {
+            std::ops::ControlFlow::Break(())
+        } else {
+            std::ops::ControlFlow::Continue(())
+        }
+    })
+    .is_some()
+    {
+        let parent_hir_node = cx.tcx.parent_hir_node(expr.hir_id);
+        let return_type = cx.typeck_results().expr_ty(expr);
+
+        if let Node::Local(l) = parent_hir_node {
+            array_span_lint(
+                cx,
+                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 { "" },
+                ),
+                Applicability::Unspecified,
+            );
+        }
+    }
+}
+
+fn array_span_lint(
+    cx: &LateContext<'_>,
+    expr_span: Span,
+    func_call_span: Span,
+    variable_name_span: Span,
+    expr_ty: Option<Ty<'_>>,
+    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()
+            },
+            if is_assign { "" } else { ";" }
+        ),
+        Applicability::Unspecified,
+    );
+}

tests/ui/zero_repeat_side_effects.fixed

@@ -0,0 +1,60 @@
+#![warn(clippy::zero_repeat_side_effects)]
+#![allow(clippy::unnecessary_operation)]
+#![allow(clippy::useless_vec)]
+#![allow(clippy::needless_late_init)]
+
+fn f() -> i32 {
+    println!("side effect");
+    10
+}
+
+fn main() {
+    const N: usize = 0;
+    const M: usize = 1;
+
+    // should trigger
+
+    // 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
+    f(); let c: std::vec::Vec<i32> = vec![];
+    let d;
+    f(); d = vec![] as std::vec::Vec<i32>;
+
+    // for macros
+    println!("side effect"); let e: [(); 0] = [];
+
+    // for nested calls
+    { f() }; let g: [i32; 0] = [];
+
+    // as function param
+    drop({ f(); vec![] as std::vec::Vec<i32> });
+
+    // when singled out/not part of assignment/local
+    { f(); vec![] as std::vec::Vec<i32> };
+    { f(); [] as [i32; 0] };
+    { f(); [] as [i32; 0] };
+
+    // should not trigger
+
+    // on arrays with > 0 repeat
+    let a = [f(); 1];
+    let a = [f(); M];
+    let mut b;
+    b = [f(); 1];
+    b = [f(); M];
+
+    // on vecs with > 0 repeat
+    let c = vec![f(); 1];
+    let d;
+    d = vec![f(); 1];
+
+    // as function param
+    drop(vec![f(); 1]);
+}

tests/ui/zero_repeat_side_effects.rs

@@ -0,0 +1,60 @@
+#![warn(clippy::zero_repeat_side_effects)]
+#![allow(clippy::unnecessary_operation)]
+#![allow(clippy::useless_vec)]
+#![allow(clippy::needless_late_init)]
+
+fn f() -> i32 {
+    println!("side effect");
+    10
+}
+
+fn main() {
+    const N: usize = 0;
+    const M: usize = 1;
+
+    // should trigger
+
+    // 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
+    let c = vec![f(); 0];
+    let d;
+    d = vec![f(); 0];
+
+    // for macros
+    let e = [println!("side effect"); 0];
+
+    // for nested calls
+    let g = [{ f() }; 0];
+
+    // as function param
+    drop(vec![f(); 0]);
+
+    // when singled out/not part of assignment/local
+    vec![f(); 0];
+    [f(); 0];
+    [f(); N];
+
+    // should not trigger
+
+    // on arrays with > 0 repeat
+    let a = [f(); 1];
+    let a = [f(); M];
+    let mut b;
+    b = [f(); 1];
+    b = [f(); M];
+
+    // on vecs with > 0 repeat
+    let c = vec![f(); 1];
+    let d;
+    d = vec![f(); 1];
+
+    // as function param
+    drop(vec![f(); 1]);
+}

tests/ui/zero_repeat_side_effects.stderr

@@ -0,0 +1,77 @@
+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
+   |
+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)]`
+
+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
+   |
+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
+   |
+LL |     let c = vec![f(); 0];
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `f(); let c: std::vec::Vec<i32> = 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
+   |
+LL |     d = vec![f(); 0];
+   |     ^^^^^^^^^^^^^^^^ help: consider using: `f(); d = vec![] as std::vec::Vec<i32>`
+
+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
+   |
+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
+   |
+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
+   |
+LL |     drop(vec![f(); 0]);
+   |          ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec<i32> }`
+
+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
+   |
+LL |     vec![f(); 0];
+   |     ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec<i32> }`
+
+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
+   |
+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
+