Auto merge of rust-lang#12449 - Jacherr:issue-6439, r=llogiq

Add new lint `zero_repeat_side_effects`

Fixes rust-lang/rust-clippy#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`

Author: bors

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 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     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 visibility;
 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
+