about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJacherr <jwc2002@outlook.com>2023-12-10 22:46:25 +0000
committerJacherr <jwc2002@outlook.com>2024-03-09 18:53:14 +0000
commit0e59259add77fbe673eb14566a3bceac67735853 (patch)
tree5aede1b49df8291c92bef7f9b3daeaa66316a434
parentc173ea64b7c3ff289098569de04217e34454c59f (diff)
downloadrust-0e59259add77fbe673eb14566a3bceac67735853.tar.gz
rust-0e59259add77fbe673eb14566a3bceac67735853.zip
add new lint `zero_repeat_side_effects`
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--clippy_lints/src/zero_repeat_side_effects.rs154
-rw-r--r--tests/ui/zero_repeat_side_effects.fixed60
-rw-r--r--tests/ui/zero_repeat_side_effects.rs60
-rw-r--r--tests/ui/zero_repeat_side_effects.stderr77
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..b3a80ab93a7
--- /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
+