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

Add new lint `zero_repeat_side_effects`

Fixes https://github.com/rust-lang/rust-clippy/issues/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`

7 files changed, 355 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5e783b593f9..2e0e14ca06e 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 7a0d57c7859..5e22ba22e27 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/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,
 ];
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index b930175c4d8..adafc92d8a3 100644
--- a/clippy_lints/src/lib.rs
+++ b/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`
 }
 
diff --git a/clippy_lints/src/zero_repeat_side_effects.rs b/clippy_lints/src/zero_repeat_side_effects.rs
new file mode 100644
index 00000000000..852d04cd21b
--- /dev/null
+++ b/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,
+    );
+}
diff --git a/tests/ui/zero_repeat_side_effects.fixed b/tests/ui/zero_repeat_side_effects.fixed
new file mode 100644
index 00000000000..6f132521926
--- /dev/null
+++ b/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]);
+}
diff --git a/tests/ui/zero_repeat_side_effects.rs b/tests/ui/zero_repeat_side_effects.rs
new file mode 100644
index 00000000000..9d9c367375a
--- /dev/null
+++ b/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]);
+}
diff --git a/tests/ui/zero_repeat_side_effects.stderr b/tests/ui/zero_repeat_side_effects.stderr
new file mode 100644
index 00000000000..afdc6054253
--- /dev/null
+++ b/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
+