[arithmetic_side_effects] Fix rust-lang#11393

c410-f3r · Aug 25, 2023 · e58a5fb · e58a5fb
1 parent d802ab2
commit e58a5fb
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 24 deletions.
diff --git a/clippy_lints/src/operators/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs
@@ -91,35 +91,22 @@ impl ArithmeticSideEffects {
     ) -> bool {
         const SATURATING: &[&str] = &["core", "num", "saturating", "Saturating"];
         const WRAPPING: &[&str] = &["core", "num", "wrapping", "Wrapping"];
-        let is_non_zero = |symbol: Option<Symbol>| {
+        let is_div_or_rem = matches!(op.node, hir::BinOpKind::Div | hir::BinOpKind::Rem);
+        let is_non_zero_u = |symbol: Option<Symbol>| {
             matches!(
                 symbol,
-                Some(
-                    sym::NonZeroI128
-                        | sym::NonZeroI16
-                        | sym::NonZeroI32
-                        | sym::NonZeroI64
-                        | sym::NonZeroI8
-                        | sym::NonZeroU128
-                        | sym::NonZeroU16
-                        | sym::NonZeroU32
-                        | sym::NonZeroU64
-                        | sym::NonZeroU8
-                )
+                Some(sym::NonZeroU128 | sym::NonZeroU16 | sym::NonZeroU32 | sym::NonZeroU64 | sym::NonZeroU8)
             )
         };
-        // If the RHS is NonZero*, then division or module by zero will never occur
-        if is_non_zero(type_diagnostic_name(cx, rhs_ty)) && let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node {
+        // If the RHS is NonZeroU*, then division or module by zero will never occur
+        if is_non_zero_u(type_diagnostic_name(cx, rhs_ty)) && is_div_or_rem {
             return true;
         }
-        // For `Saturation` or `Wrapping` (RHS), all but division and module are allowed.
-        let is_div_or_rem = matches!(op.node, hir::BinOpKind::Div | hir::BinOpKind::Rem);
-        if (match_type(cx, rhs_ty, SATURATING) || match_type(cx, rhs_ty, WRAPPING)) && !is_div_or_rem {
-            return true;
-        }
-        // For `Saturation` or `Wrapping` (LHS), everything is allowed
-        if match_type(cx, lhs_ty, SATURATING) || match_type(cx, lhs_ty, WRAPPING) {
-            return true;
+        // `Saturation` and `Wrapping` can overflow if the RHS is zero in a division or module
+        let is_lhs_sat_or_wrap = match_type(cx, lhs_ty, SATURATING) || match_type(cx, lhs_ty, WRAPPING);
+        let is_rhs_sat_or_wrap = match_type(cx, rhs_ty, SATURATING) || match_type(cx, rhs_ty, WRAPPING);
+        if is_lhs_sat_or_wrap || is_rhs_sat_or_wrap {
+            return !is_div_or_rem;
         }
         false
     }

diff --git a/tests/ui/arithmetic_side_effects.stderr b/tests/ui/arithmetic_side_effects.stderr
@@ -714,5 +714,17 @@ error: arithmetic operation that can potentially result in unexpected side-effec
 LL |         unsigned % nonzero_unsigned
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 119 previous errors
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:512:9
+   |
+LL |         x / maybe_zero
+   |         ^^^^^^^^^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:516:9
+   |
+LL |         x % maybe_zero
+   |         ^^^^^^^^^^^^^^
+
+error: aborting due to 121 previous errors