Auto merge of #9130 - c410-f3r:arith, r=llogiq

Add `Arithmetic` lint

Fixes https://github.com/rust-lang/rust-clippy/issues/8903

r? `@llogiq`

changelog: Add `Arithmetic` lint

CHANGELOG.md

@@ -3437,6 +3437,7 @@ Released 2018-09-13
 [`almost_complete_letter_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_letter_range
 [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
 [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
+[`arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic
 [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions
 [`as_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore
 [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants

clippy_lints/src/lib.register_lints.rs

@@ -419,6 +419,7 @@ store.register_lints(&[
     only_used_in_recursion::ONLY_USED_IN_RECURSION,
     open_options::NONSENSICAL_OPEN_OPTIONS,
     operators::ABSURD_EXTREME_COMPARISONS,
+    operators::ARITHMETIC,
     operators::ASSIGN_OP_PATTERN,
     operators::BAD_BIT_MASK,
     operators::CMP_NAN,

clippy_lints/src/lib.register_restriction.rs

@@ -48,6 +48,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
     LintId::of(mixed_read_write_in_expression::MIXED_READ_WRITE_IN_EXPRESSION),
     LintId::of(module_style::MOD_MODULE_FILES),
     LintId::of(module_style::SELF_NAMED_MODULE_FILES),
+    LintId::of(operators::ARITHMETIC),
     LintId::of(operators::FLOAT_ARITHMETIC),
     LintId::of(operators::FLOAT_CMP_CONST),
     LintId::of(operators::INTEGER_ARITHMETIC),

clippy_lints/src/lib.rs

@@ -549,6 +549,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         store.register_late_pass(|| Box::new(utils::internal_lints::MsrvAttrImpl));
     }
 
+    let arithmetic_allowed = conf.arithmetic_allowed.clone();
+    store.register_late_pass(move || Box::new(operators::arithmetic::Arithmetic::new(arithmetic_allowed.clone())));
     store.register_late_pass(|| Box::new(utils::dump_hir::DumpHir));
     store.register_late_pass(|| Box::new(utils::author::Author));
     let await_holding_invalid_types = conf.await_holding_invalid_types.clone();

clippy_lints/src/operators/arithmetic.rs

@@ -0,0 +1,119 @@
+#![allow(
+    // False positive
+    clippy::match_same_arms
+)]
+
+use super::ARITHMETIC;
+use clippy_utils::{consts::constant_simple, diagnostics::span_lint};
+use rustc_data_structures::fx::FxHashSet;
+use rustc_hir as hir;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
+use rustc_span::source_map::Span;
+
+const HARD_CODED_ALLOWED: &[&str] = &["std::num::Saturating", "std::string::String", "std::num::Wrapping"];
+
+#[derive(Debug)]
+pub struct Arithmetic {
+    allowed: FxHashSet<String>,
+    // Used to check whether expressions are constants, such as in enum discriminants and consts
+    const_span: Option<Span>,
+    expr_span: Option<Span>,
+}
+
+impl_lint_pass!(Arithmetic => [ARITHMETIC]);
+
+impl Arithmetic {
+    #[must_use]
+    pub fn new(mut allowed: FxHashSet<String>) -> Self {
+        allowed.extend(HARD_CODED_ALLOWED.iter().copied().map(String::from));
+        Self {
+            allowed,
+            const_span: None,
+            expr_span: None,
+        }
+    }
+
+    /// Checks if the given `expr` has any of the inner `allowed` elements.
+    fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
+        self.allowed.contains(
+            cx.typeck_results()
+                .expr_ty(expr)
+                .to_string()
+                .split('<')
+                .next()
+                .unwrap_or_default(),
+        )
+    }
+
+    fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
+        span_lint(cx, ARITHMETIC, expr.span, "arithmetic detected");
+        self.expr_span = Some(expr.span);
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for Arithmetic {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
+        if self.expr_span.is_some() {
+            return;
+        }
+        if let Some(span) = self.const_span && span.contains(expr.span) {
+            return;
+        }
+        match &expr.kind {
+            hir::ExprKind::Binary(op, lhs, rhs) | hir::ExprKind::AssignOp(op, lhs, rhs) => {
+                let (
+                    hir::BinOpKind::Add
+                    | hir::BinOpKind::Sub
+                    | hir::BinOpKind::Mul
+                    | hir::BinOpKind::Div
+                    | hir::BinOpKind::Rem
+                    | hir::BinOpKind::Shl
+                    | hir::BinOpKind::Shr
+                ) = op.node else {
+                    return;
+                };
+                if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
+                    return;
+                }
+                self.issue_lint(cx, expr);
+            },
+            hir::ExprKind::Unary(hir::UnOp::Neg, _) => {
+                // CTFE already takes care of things like `-1` that do not overflow.
+                if constant_simple(cx, cx.typeck_results(), expr).is_none() {
+                    self.issue_lint(cx, expr);
+                }
+            },
+            _ => {},
+        }
+    }
+
+    fn check_body(&mut self, cx: &LateContext<'_>, body: &hir::Body<'_>) {
+        let body_owner = cx.tcx.hir().body_owner_def_id(body.id());
+        match cx.tcx.hir().body_owner_kind(body_owner) {
+            hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => {
+                let body_span = cx.tcx.def_span(body_owner);
+                if let Some(span) = self.const_span && span.contains(body_span) {
+                    return;
+                }
+                self.const_span = Some(body_span);
+            },
+            hir::BodyOwnerKind::Closure | hir::BodyOwnerKind::Fn => {},
+        }
+    }
+
+    fn check_body_post(&mut self, cx: &LateContext<'_>, body: &hir::Body<'_>) {
+        let body_owner = cx.tcx.hir().body_owner(body.id());
+        let body_span = cx.tcx.hir().span(body_owner);
+        if let Some(span) = self.const_span && span.contains(body_span) {
+            return;
+        }
+        self.const_span = None;
+    }
+
+    fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
+        if Some(expr.span) == self.expr_span {
+            self.expr_span = None;
+        }
+    }
+}

clippy_lints/src/operators/mod.rs

@@ -2,6 +2,8 @@ use rustc_hir::{Body, Expr, ExprKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 
+pub(crate) mod arithmetic;
+
 mod absurd_extreme_comparisons;
 mod assign_op_pattern;
 mod bit_mask;
@@ -57,6 +59,42 @@ declare_clippy_lint! {
     "a comparison with a maximum or minimum value that is always true or false"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for any kind of arithmetic operation of any type.
+    ///
+    /// Operators like `+`, `-`, `*` or `<<` are usually capable of overflowing according to the [Rust
+    /// Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow),
+    /// or can panic (`/`, `%`). Known safe built-in types like `Wrapping` or `Saturing` are filtered
+    /// away.
+    ///
+    /// ### Why is this bad?
+    /// Integer overflow will trigger a panic in debug builds or will wrap in
+    /// release mode. Division by zero will cause a panic in either mode. In some applications one
+    /// wants explicitly checked, wrapping or saturating arithmetic.
+    ///
+    /// #### Example
+    /// ```rust
+    /// # let a = 0;
+    /// a + 1;
+    /// ```
+    ///
+    /// Third-party types also tend to overflow.
+    ///
+    /// #### Example
+    /// ```ignore,rust
+    /// use rust_decimal::Decimal;
+    /// let _n = Decimal::MAX + Decimal::MAX;
+    /// ```
+    ///
+    /// ### Allowed types
+    /// Custom allowed types can be specified through the "arithmetic-allowed" filter.
+    #[clippy::version = "1.64.0"]
+    pub ARITHMETIC,
+    restriction,
+    "any arithmetic expression that could overflow or panic"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for integer arithmetic operations which could overflow or panic.
@@ -747,6 +785,7 @@ pub struct Operators {
 }
 impl_lint_pass!(Operators => [
     ABSURD_EXTREME_COMPARISONS,
+    ARITHMETIC,
     INTEGER_ARITHMETIC,
     FLOAT_ARITHMETIC,
     ASSIGN_OP_PATTERN,

clippy_lints/src/utils/conf.rs

@@ -191,6 +191,10 @@ macro_rules! define_Conf {
 }
 
 define_Conf! {
+    /// Lint: Arithmetic.
+    ///
+    /// Suppress checking of the passed type names.
+    (arithmetic_allowed: rustc_data_structures::fx::FxHashSet<String> = <_>::default()),
     /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX.
     ///
     /// Suppress lints whenever the suggested change would cause breakage for other crates.

tests/ui-toml/arithmetic_allowed/arithmetic_allowed.rs

@@ -0,0 +1,24 @@
+#![warn(clippy::arithmetic)]
+
+use core::ops::Add;
+
+#[derive(Clone, Copy)]
+struct Point {
+    x: i32,
+    y: i32,
+}
+
+impl Add for Point {
+    type Output = Self;
+
+    fn add(self, other: Self) -> Self {
+        todo!()
+    }
+}
+
+fn main() {
+    let _ = Point { x: 1, y: 0 } + Point { x: 2, y: 3 };
+
+    let point: Point = Point { x: 1, y: 0 };
+    let _ = point + point;
+}

tests/ui-toml/arithmetic_allowed/clippy.toml

@@ -0,0 +1 @@
+arithmetic-allowed = ["Point"]

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -3,6 +3,7 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie
            allow-expect-in-tests
            allow-unwrap-in-tests
            allowed-scripts
+           arithmetic-allowed
            array-size-threshold
            avoid-breaking-exported-api
            await-holding-invalid-types

tests/ui/arithmetic.fixed

@@ -0,0 +1,27 @@
+// run-rustfix
+
+#![allow(clippy::unnecessary_owned_empty_strings)]
+#![feature(saturating_int_impl)]
+#![warn(clippy::arithmetic)]
+
+use core::num::{Saturating, Wrapping};
+
+pub fn hard_coded_allowed() {
+    let _ = Saturating(0u32) + Saturating(0u32);
+    let _ = String::new() + "";
+    let _ = Wrapping(0u32) + Wrapping(0u32);
+
+    let saturating: Saturating<u32> = Saturating(0u32);
+    let string: String = String::new();
+    let wrapping: Wrapping<u32> = Wrapping(0u32);
+
+    let inferred_saturating = saturating + saturating;
+    let inferred_string = string + "";
+    let inferred_wrapping = wrapping + wrapping;
+
+    let _ = inferred_saturating + inferred_saturating;
+    let _ = inferred_string + "";
+    let _ = inferred_wrapping + inferred_wrapping;
+}
+
+fn main() {}

tests/ui/arithmetic.rs

@@ -0,0 +1,27 @@
+// run-rustfix
+
+#![allow(clippy::unnecessary_owned_empty_strings)]
+#![feature(saturating_int_impl)]
+#![warn(clippy::arithmetic)]
+
+use core::num::{Saturating, Wrapping};
+
+pub fn hard_coded_allowed() {
+    let _ = Saturating(0u32) + Saturating(0u32);
+    let _ = String::new() + "";
+    let _ = Wrapping(0u32) + Wrapping(0u32);
+
+    let saturating: Saturating<u32> = Saturating(0u32);
+    let string: String = String::new();
+    let wrapping: Wrapping<u32> = Wrapping(0u32);
+
+    let inferred_saturating = saturating + saturating;
+    let inferred_string = string + "";
+    let inferred_wrapping = wrapping + wrapping;
+
+    let _ = inferred_saturating + inferred_saturating;
+    let _ = inferred_string + "";
+    let _ = inferred_wrapping + inferred_wrapping;
+}
+
+fn main() {}