Auto merge of rust-lang#8519 - tysg:redundant-modulo, r=giraffate

Check if lhs < rhs in modulos in `identity_op`

Fixes rust-lang#8508

changelog: [`identity_op`] now checks for modulos, e.g. `1 % 3`

Author: bors

clippy_lints/src/identity_op.rs

@@ -5,7 +5,7 @@ use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 
-use clippy_utils::consts::{constant_simple, Constant};
+use clippy_utils::consts::{constant_full_int, constant_simple, Constant, FullInt};
 use clippy_utils::diagnostics::span_lint;
 use clippy_utils::{clip, unsext};
 
@@ -54,6 +54,7 @@ impl<'tcx> LateLintPass<'tcx> for IdentityOp {
                     check(cx, left, -1, e.span, right.span);
                     check(cx, right, -1, e.span, left.span);
                 },
+                BinOpKind::Rem => check_remainder(cx, left, right, e.span, left.span),
                 _ => (),
             }
         }
@@ -70,6 +71,18 @@ fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_
             && constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1)))
 }
 
+fn check_remainder(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>, span: Span, arg: Span) {
+    let lhs_const = constant_full_int(cx, cx.typeck_results(), left);
+    let rhs_const = constant_full_int(cx, cx.typeck_results(), right);
+    if match (lhs_const, rhs_const) {
+        (Some(FullInt::S(lv)), Some(FullInt::S(rv))) => lv.abs() < rv.abs(),
+        (Some(FullInt::U(lv)), Some(FullInt::U(rv))) => lv < rv,
+        _ => return,
+    } {
+        span_ineffective_operation(cx, span, arg);
+    }
+}
+
 fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) {
     if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e).map(Constant::peel_refs) {
         let check = match *cx.typeck_results().expr_ty(e).peel_refs().kind() {
@@ -83,15 +96,19 @@ fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) {
             1 => v == 1,
             _ => unreachable!(),
         } {
-            span_lint(
-                cx,
-                IDENTITY_OP,
-                span,
-                &format!(
-                    "the operation is ineffective. Consider reducing it to `{}`",
-                    snippet(cx, arg, "..")
-                ),
-            );
+            span_ineffective_operation(cx, span, arg);
         }
     }
 }
+
+fn span_ineffective_operation(cx: &LateContext<'_>, span: Span, arg: Span) {
+    span_lint(
+        cx,
+        IDENTITY_OP,
+        span,
+        &format!(
+            "the operation is ineffective. Consider reducing it to `{}`",
+            snippet(cx, arg, "..")
+        ),
+    );
+}

tests/ui/identity_op.rs

@@ -66,4 +66,13 @@ fn main() {
     let b = a << 0; // no error: non-integer
 
     1 * Meter; // no error: non-integer
+
+    2 % 3;
+    -2 % 3;
+    2 % -3 + x;
+    -2 % -3 + x;
+    x + 1 % 3;
+    (x + 1) % 3; // no error
+    4 % 3; // no error
+    4 % -3; // no error
 }

tests/ui/identity_op.stderr

@@ -78,5 +78,35 @@ error: the operation is ineffective. Consider reducing it to `x`
 LL |     x >> &0;
    |     ^^^^^^^
 
-error: aborting due to 13 previous errors
+error: the operation is ineffective. Consider reducing it to `2`
+  --> $DIR/identity_op.rs:70:5
+   |
+LL |     2 % 3;
+   |     ^^^^^
+
+error: the operation is ineffective. Consider reducing it to `-2`
+  --> $DIR/identity_op.rs:71:5
+   |
+LL |     -2 % 3;
+   |     ^^^^^^
+
+error: the operation is ineffective. Consider reducing it to `2`
+  --> $DIR/identity_op.rs:72:5
+   |
+LL |     2 % -3 + x;
+   |     ^^^^^^
+
+error: the operation is ineffective. Consider reducing it to `-2`
+  --> $DIR/identity_op.rs:73:5
+   |
+LL |     -2 % -3 + x;
+   |     ^^^^^^^
+
+error: the operation is ineffective. Consider reducing it to `1`
+  --> $DIR/identity_op.rs:74:9
+   |
+LL |     x + 1 % 3;
+   |         ^^^^^
+
+error: aborting due to 18 previous errors
 

tests/ui/modulo_arithmetic_integral_const.rs

@@ -1,5 +1,10 @@
 #![warn(clippy::modulo_arithmetic)]
-#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::modulo_one)]
+#![allow(
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::modulo_one,
+    clippy::identity_op
+)]
 
 fn main() {
     // Lint when both sides are const and of the opposite sign

tests/ui/modulo_arithmetic_integral_const.stderr

@@ -1,5 +1,5 @@
 error: you are using modulo operator on constants with different signs: `-1 % 2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:6:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:11:5
    |
 LL |     -1 % 2;
    |     ^^^^^^
@@ -9,7 +9,7 @@ LL |     -1 % 2;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `1 % -2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:7:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:12:5
    |
 LL |     1 % -2;
    |     ^^^^^^
@@ -18,7 +18,7 @@ LL |     1 % -2;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `-1 % 3`
-  --> $DIR/modulo_arithmetic_integral_const.rs:8:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:13:5
    |
 LL |     (1 - 2) % (1 + 2);
    |     ^^^^^^^^^^^^^^^^^
@@ -27,7 +27,7 @@ LL |     (1 - 2) % (1 + 2);
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `3 % -1`
-  --> $DIR/modulo_arithmetic_integral_const.rs:9:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:14:5
    |
 LL |     (1 + 2) % (1 - 2);
    |     ^^^^^^^^^^^^^^^^^
@@ -36,7 +36,7 @@ LL |     (1 + 2) % (1 - 2);
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `-35 % 300000`
-  --> $DIR/modulo_arithmetic_integral_const.rs:10:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:15:5
    |
 LL |     35 * (7 - 4 * 2) % (-500 * -600);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -45,7 +45,7 @@ LL |     35 * (7 - 4 * 2) % (-500 * -600);
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `-1 % 2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:12:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:17:5
    |
 LL |     -1i8 % 2i8;
    |     ^^^^^^^^^^
@@ -54,7 +54,7 @@ LL |     -1i8 % 2i8;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `1 % -2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:13:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:18:5
    |
 LL |     1i8 % -2i8;
    |     ^^^^^^^^^^
@@ -63,7 +63,7 @@ LL |     1i8 % -2i8;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `-1 % 2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:14:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:19:5
    |
 LL |     -1i16 % 2i16;
    |     ^^^^^^^^^^^^
@@ -72,7 +72,7 @@ LL |     -1i16 % 2i16;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `1 % -2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:15:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:20:5
    |
 LL |     1i16 % -2i16;
    |     ^^^^^^^^^^^^
@@ -81,7 +81,7 @@ LL |     1i16 % -2i16;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `-1 % 2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:16:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:21:5
    |
 LL |     -1i32 % 2i32;
    |     ^^^^^^^^^^^^
@@ -90,7 +90,7 @@ LL |     -1i32 % 2i32;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `1 % -2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:17:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:22:5
    |
 LL |     1i32 % -2i32;
    |     ^^^^^^^^^^^^
@@ -99,7 +99,7 @@ LL |     1i32 % -2i32;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `-1 % 2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:18:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:23:5
    |
 LL |     -1i64 % 2i64;
    |     ^^^^^^^^^^^^
@@ -108,7 +108,7 @@ LL |     -1i64 % 2i64;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `1 % -2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:19:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:24:5
    |
 LL |     1i64 % -2i64;
    |     ^^^^^^^^^^^^
@@ -117,7 +117,7 @@ LL |     1i64 % -2i64;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `-1 % 2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:20:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:25:5
    |
 LL |     -1i128 % 2i128;
    |     ^^^^^^^^^^^^^^
@@ -126,7 +126,7 @@ LL |     -1i128 % 2i128;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `1 % -2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:21:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:26:5
    |
 LL |     1i128 % -2i128;
    |     ^^^^^^^^^^^^^^
@@ -135,7 +135,7 @@ LL |     1i128 % -2i128;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `-1 % 2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:22:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:27:5
    |
 LL |     -1isize % 2isize;
    |     ^^^^^^^^^^^^^^^^
@@ -144,7 +144,7 @@ LL |     -1isize % 2isize;
    = note: or consider using `rem_euclid` or similar function
 
 error: you are using modulo operator on constants with different signs: `1 % -2`
-  --> $DIR/modulo_arithmetic_integral_const.rs:23:5
+  --> $DIR/modulo_arithmetic_integral_const.rs:28:5
    |
 LL |     1isize % -2isize;
    |     ^^^^^^^^^^^^^^^^