Auto merge of #9840 - c410-f3r:arith-2, r=Alexendoo

[`arithmetic-side-effects`]: Consider user-provided pairs

Depends on https://github.com/rust-lang/rust-clippy/pull/9592.

Because of https://github.com/rust-lang/rust-clippy/pull/9559#discussion_r985293523, r? `@Alexendoo`

```
changelog: [`arithmetic-side-effects`]: Consider user-provided pairs
```

clippy_lints/src/lib.rs

@@ -507,9 +507,20 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     }
 
     let arithmetic_side_effects_allowed = conf.arithmetic_side_effects_allowed.clone();
+    let arithmetic_side_effects_allowed_binary = conf.arithmetic_side_effects_allowed_binary.clone();
+    let arithmetic_side_effects_allowed_unary = conf.arithmetic_side_effects_allowed_unary.clone();
     store.register_late_pass(move |_| {
         Box::new(operators::arithmetic_side_effects::ArithmeticSideEffects::new(
-            arithmetic_side_effects_allowed.clone(),
+            arithmetic_side_effects_allowed
+                .iter()
+                .flat_map(|el| [[el.clone(), "*".to_string()], ["*".to_string(), el.clone()]])
+                .chain(arithmetic_side_effects_allowed_binary.clone())
+                .collect(),
+            arithmetic_side_effects_allowed
+                .iter()
+                .chain(arithmetic_side_effects_allowed_unary.iter())
+                .cloned()
+                .collect(),
         ))
     });
     store.register_late_pass(|_| Box::new(utils::dump_hir::DumpHir));

clippy_lints/src/operators/arithmetic_side_effects.rs

@@ -5,25 +5,26 @@ use clippy_utils::{
     peel_hir_expr_refs,
 };
 use rustc_ast as ast;
-use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::Ty;
 use rustc_session::impl_lint_pass;
 use rustc_span::source_map::{Span, Spanned};
 
-const HARD_CODED_ALLOWED: &[&str] = &[
+const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[
-    "&str",
+    ["f32", "f32"],
-    "f32",
+    ["f64", "f64"],
-    "f64",
+    ["std::num::Saturating", "std::num::Saturating"],
-    "std::num::Saturating",
+    ["std::num::Wrapping", "std::num::Wrapping"],
-    "std::num::Wrapping",
+    ["std::string::String", "&str"],
-    "std::string::String",
 ];
+const HARD_CODED_ALLOWED_UNARY: &[&str] = &["f32", "f64", "std::num::Saturating", "std::num::Wrapping"];
 
 #[derive(Debug)]
 pub struct ArithmeticSideEffects {
-    allowed: FxHashSet<String>,
+    allowed_binary: FxHashMap<String, FxHashSet<String>>,
+    allowed_unary: FxHashSet<String>,
     // Used to check whether expressions are constants, such as in enum discriminants and consts
     const_span: Option<Span>,
     expr_span: Option<Span>,
@@ -33,19 +34,55 @@ impl_lint_pass!(ArithmeticSideEffects => [ARITHMETIC_SIDE_EFFECTS]);
 
 impl ArithmeticSideEffects {
     #[must_use]
-    pub fn new(mut allowed: FxHashSet<String>) -> Self {
+    pub fn new(user_allowed_binary: Vec<[String; 2]>, user_allowed_unary: Vec<String>) -> Self {
-        allowed.extend(HARD_CODED_ALLOWED.iter().copied().map(String::from));
+        let mut allowed_binary: FxHashMap<String, FxHashSet<String>> = <_>::default();
+        for [lhs, rhs] in user_allowed_binary.into_iter().chain(
+            HARD_CODED_ALLOWED_BINARY
+                .iter()
+                .copied()
+                .map(|[lhs, rhs]| [lhs.to_string(), rhs.to_string()]),
+        ) {
+            allowed_binary.entry(lhs).or_default().insert(rhs);
+        }
+        let allowed_unary = user_allowed_unary
+            .into_iter()
+            .chain(HARD_CODED_ALLOWED_UNARY.iter().copied().map(String::from))
+            .collect();
         Self {
-            allowed,
+            allowed_binary,
+            allowed_unary,
             const_span: None,
             expr_span: None,
         }
     }
 
-    /// Checks if the given `expr` has any of the inner `allowed` elements.
+    /// Checks if the lhs and the rhs types of a binary operation like "addition" or
-    fn is_allowed_ty(&self, ty: Ty<'_>) -> bool {
+    /// "multiplication" are present in the inner set of allowed types.
-        self.allowed
+    fn has_allowed_binary(&self, lhs_ty: Ty<'_>, rhs_ty: Ty<'_>) -> bool {
-            .contains(ty.to_string().split('<').next().unwrap_or_default())
+        let lhs_ty_string = lhs_ty.to_string();
+        let lhs_ty_string_elem = lhs_ty_string.split('<').next().unwrap_or_default();
+        let rhs_ty_string = rhs_ty.to_string();
+        let rhs_ty_string_elem = rhs_ty_string.split('<').next().unwrap_or_default();
+        if let Some(rhs_from_specific) = self.allowed_binary.get(lhs_ty_string_elem)
+            && {
+                let rhs_has_allowed_ty = rhs_from_specific.contains(rhs_ty_string_elem);
+                rhs_has_allowed_ty || rhs_from_specific.contains("*")
+            }
+        {
+           true
+        } else if let Some(rhs_from_glob) = self.allowed_binary.get("*") {
+            rhs_from_glob.contains(rhs_ty_string_elem)
+        } else {
+            false
+        }
+    }
+
+    /// Checks if the type of an unary operation like "negation" is present in the inner set of
+    /// allowed types.
+    fn has_allowed_unary(&self, ty: Ty<'_>) -> bool {
+        let ty_string = ty.to_string();
+        let ty_string_elem = ty_string.split('<').next().unwrap_or_default();
+        self.allowed_unary.contains(ty_string_elem)
     }
 
     // For example, 8i32 or &i64::MAX.
@@ -97,8 +134,7 @@ impl ArithmeticSideEffects {
         };
         let lhs_ty = cx.typeck_results().expr_ty(lhs);
         let rhs_ty = cx.typeck_results().expr_ty(rhs);
-        let lhs_and_rhs_have_the_same_ty = lhs_ty == rhs_ty;
+        if self.has_allowed_binary(lhs_ty, rhs_ty) {
-        if lhs_and_rhs_have_the_same_ty && self.is_allowed_ty(lhs_ty) && self.is_allowed_ty(rhs_ty) {
             return;
         }
         let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
@@ -137,7 +173,7 @@ impl ArithmeticSideEffects {
             return;
         }
         let ty = cx.typeck_results().expr_ty(expr).peel_refs();
-        if self.is_allowed_ty(ty) {
+        if self.has_allowed_unary(ty) {
             return;
         }
         let actual_un_expr = peel_hir_expr_refs(un_expr).0;

clippy_lints/src/operators/mod.rs

@@ -90,9 +90,6 @@ declare_clippy_lint! {
     /// use rust_decimal::Decimal;
     /// let _n = Decimal::MAX + Decimal::MAX;
     /// ```
-    ///
-    /// ### Allowed types
-    /// Custom allowed types can be specified through the "arithmetic-side-effects-allowed" filter.
     #[clippy::version = "1.64.0"]
     pub ARITHMETIC_SIDE_EFFECTS,
     restriction,

clippy_lints/src/utils/conf.rs

@@ -205,10 +205,49 @@ macro_rules! define_Conf {
 }
 
 define_Conf! {
-    /// Lint: Arithmetic.
+    /// Lint: ARITHMETIC_SIDE_EFFECTS.
     ///
-    /// Suppress checking of the passed type names.
+    /// Suppress checking of the passed type names in all types of operations.
+    ///
+    /// If a specific operation is desired, consider using `arithmetic_side_effects_allowed_binary` or `arithmetic_side_effects_allowed_unary` instead.
+    ///
+    /// #### Example
+    ///
+    /// ```toml
+    /// arithmetic-side-effects-allowed = ["SomeType", "AnotherType"]
+    /// ```
+    ///
+    /// #### Noteworthy
+    ///
+    /// A type, say `SomeType`, listed in this configuration has the same behavior of `["SomeType" , "*"], ["*", "SomeType"]` in `arithmetic_side_effects_allowed_binary`.
     (arithmetic_side_effects_allowed: rustc_data_structures::fx::FxHashSet<String> = <_>::default()),
+    /// Lint: ARITHMETIC_SIDE_EFFECTS.
+    ///
+    /// Suppress checking of the passed type pair names in binary operations like addition or
+    /// multiplication.
+    ///
+    /// Supports the "*" wildcard to indicate that a certain type won't trigger the lint regardless
+    /// of the involved counterpart. For example, `["SomeType", "*"]` or `["*", "AnotherType"]`.
+    ///
+    /// Pairs are asymmetric, which means that `["SomeType", "AnotherType"]` is not the same as
+    /// `["AnotherType", "SomeType"]`.
+    ///
+    /// #### Example
+    ///
+    /// ```toml
+    /// arithmetic-side-effects-allowed-binary = [["SomeType" , "f32"], ["AnotherType", "*"]]
+    /// ```
+    (arithmetic_side_effects_allowed_binary: Vec<[String; 2]> = <_>::default()),
+    /// Lint: ARITHMETIC_SIDE_EFFECTS.
+    ///
+    /// Suppress checking of the passed type names in unary operations like "negation" (`-`).
+    ///
+    /// #### Example
+    ///
+    /// ```toml
+    /// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
+    /// ```
+    (arithmetic_side_effects_allowed_unary: 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_side_effects_allowed/arithmetic_side_effects_allowed.rs

@@ -2,32 +2,117 @@
 
 use core::ops::{Add, Neg};
 
-#[derive(Clone, Copy)]
+macro_rules! create {
-struct Point {
+    ($name:ident) => {
-    x: i32,
+        #[allow(clippy::arithmetic_side_effects)]
-    y: i32,
+        #[derive(Clone, Copy)]
+        struct $name;
+
+        impl Add<$name> for $name {
+            type Output = $name;
+            fn add(self, other: $name) -> Self::Output {
+                todo!()
+            }
+        }
+
+        impl Add<i32> for $name {
+            type Output = $name;
+            fn add(self, other: i32) -> Self::Output {
+                todo!()
+            }
+        }
+
+        impl Add<$name> for i32 {
+            type Output = $name;
+            fn add(self, other: $name) -> Self::Output {
+                todo!()
+            }
+        }
+
+        impl Add<i64> for $name {
+            type Output = $name;
+            fn add(self, other: i64) -> Self::Output {
+                todo!()
+            }
+        }
+
+        impl Add<$name> for i64 {
+            type Output = $name;
+            fn add(self, other: $name) -> Self::Output {
+                todo!()
+            }
+        }
+
+        impl Neg for $name {
+            type Output = $name;
+            fn neg(self) -> Self::Output {
+                todo!()
+            }
+        }
+    };
 }
 
-impl Add for Point {
+create!(Foo);
-    type Output = Self;
+create!(Bar);
+create!(Baz);
+create!(OutOfNames);
 
-    fn add(self, other: Self) -> Self {
+fn lhs_and_rhs_are_equal() {
-        todo!()
+    // is explicitly on the list
-    }
+    let _ = OutOfNames + OutOfNames;
+    // is explicitly on the list
+    let _ = Foo + Foo;
+    // is implicitly on the list
+    let _ = Bar + Bar;
+    // not on the list
+    let _ = Baz + Baz;
 }
 
-impl Neg for Point {
+fn lhs_is_different() {
-    type Output = Self;
+    // is explicitly on the list
+    let _ = 1i32 + OutOfNames;
+    // is explicitly on the list
+    let _ = 1i32 + Foo;
+    // is implicitly on the list
+    let _ = 1i32 + Bar;
+    // not on the list
+    let _ = 1i32 + Baz;
 
-    fn neg(self) -> Self::Output {
+    // not on the list
-        todo!()
+    let _ = 1i64 + Foo;
-    }
+    // is implicitly on the list
+    let _ = 1i64 + Bar;
+    // not on the list
+    let _ = 1i64 + Baz;
 }
 
-fn main() {
+fn rhs_is_different() {
-    let _ = Point { x: 1, y: 0 } + Point { x: 2, y: 3 };
+    // is explicitly on the list
+    let _ = OutOfNames + 1i32;
+    // is explicitly on the list
+    let _ = Foo + 1i32;
+    // is implicitly on the list
+    let _ = Bar + 1i32;
+    // not on the list
+    let _ = Baz + 1i32;
 
-    let point: Point = Point { x: 1, y: 0 };
+    // not on the list
-    let _ = point + point;
+    let _ = Foo + 1i64;
-    let _ = -point;
+    // is implicitly on the list
+    let _ = Bar + 1i64;
+    // not on the list
+    let _ = Baz + 1i64;
 }
+
+fn unary() {
+    // is explicitly on the list
+    let _ = -OutOfNames;
+    // is specifically on the list
+    let _ = -Foo;
+    // not on the list
+    let _ = -Bar;
+    // not on the list
+    let _ = -Baz;
+}
+
+fn main() {}

tests/ui-toml/arithmetic_side_effects_allowed/arithmetic_side_effects_allowed.stderr

@@ -0,0 +1,58 @@
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:68:13
+   |
+LL |     let _ = Baz + Baz;
+   |             ^^^^^^^^^
+   |
+   = note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:79:13
+   |
+LL |     let _ = 1i32 + Baz;
+   |             ^^^^^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:82:13
+   |
+LL |     let _ = 1i64 + Foo;
+   |             ^^^^^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:86:13
+   |
+LL |     let _ = 1i64 + Baz;
+   |             ^^^^^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:97:13
+   |
+LL |     let _ = Baz + 1i32;
+   |             ^^^^^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:100:13
+   |
+LL |     let _ = Foo + 1i64;
+   |             ^^^^^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:104:13
+   |
+LL |     let _ = Baz + 1i64;
+   |             ^^^^^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:113:13
+   |
+LL |     let _ = -Bar;
+   |             ^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects_allowed.rs:115:13
+   |
+LL |     let _ = -Baz;
+   |             ^^^^
+
+error: aborting due to 9 previous errors
+

tests/ui-toml/arithmetic_side_effects_allowed/clippy.toml

@@ -1 +1,11 @@
-arithmetic-side-effects-allowed = ["Point"]
+arithmetic-side-effects-allowed = [
+    "OutOfNames"
+]
+arithmetic-side-effects-allowed-binary = [
+    ["Foo", "Foo"],
+    ["Foo", "i32"],
+    ["i32", "Foo"],
+    ["Bar", "*"],
+    ["*", "Bar"],
+]
+arithmetic-side-effects-allowed-unary = ["Foo"]

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

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

tests/ui/arithmetic_side_effects.stderr

@@ -1,28 +1,10 @@
-error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:78:13
-   |
-LL |     let _ = String::new() + "";
-   |             ^^^^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
-
-error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:86:27
-   |
-LL |     let inferred_string = string + "";
-   |                           ^^^^^^^^^^^
-
-error: arithmetic operation that can potentially result in unexpected side-effects
-  --> $DIR/arithmetic_side_effects.rs:90:13
-   |
-LL |     let _ = inferred_string + "";
-   |             ^^^^^^^^^^^^^^^^^^^^
-
 error: arithmetic operation that can potentially result in unexpected side-effects
   --> $DIR/arithmetic_side_effects.rs:165:5
    |
 LL |     _n += 1;
    |     ^^^^^^^
+   |
+   = note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
 
 error: arithmetic operation that can potentially result in unexpected side-effects
   --> $DIR/arithmetic_side_effects.rs:166:5
@@ -348,5 +330,5 @@ error: arithmetic operation that can potentially result in unexpected side-effec
 LL |     _n = -&_n;
    |          ^^^^
 
-error: aborting due to 58 previous errors
+error: aborting due to 55 previous errors