feat: also detect non-consts

Author: ada4a

clippy_lints/src/manual_rotate.rs

@@ -85,28 +85,49 @@ impl LateLintPass<'_> for ManualRotate {
             let const_eval = ConstEvalCtxt::new(cx);
 
             let ctxt = expr.span.ctxt();
-            if let Some(Constant::Int(l_amount_val)) = const_eval.eval_local(l_amount, ctxt)
+            let (shift_function, amount) = if let Some(Constant::Int(l_amount_val)) =
+                const_eval.eval_local(l_amount, ctxt)
                 && let Some(Constant::Int(r_amount_val)) = const_eval.eval_local(r_amount, ctxt)
                 && l_amount_val + r_amount_val == u128::from(bit_width)
             {
-                let (shift_function, amount) = if l_amount_val < r_amount_val {
+                if l_amount_val < r_amount_val {
                     (l_shift_dir, l_amount)
                 } else {
                     (r_shift_dir, r_amount)
+                }
+            } else {
+                let (amount1, binop, minuend, amount2, shift_direction) = match (l_amount.kind, r_amount.kind) {
+                    (_, ExprKind::Binary(binop, minuend, other)) => (l_amount, binop, minuend, other, l_shift_dir),
+                    (ExprKind::Binary(binop, minuend, other), _) => (r_amount, binop, minuend, other, r_shift_dir),
+                    _ => return,
                 };
-                let mut applicability = Applicability::MachineApplicable;
-                let expr_sugg = sugg::Sugg::hir_with_applicability(cx, l_expr, "_", &mut applicability).maybe_paren();
-                let amount = sugg::Sugg::hir_with_applicability(cx, amount, "_", &mut applicability);
-                span_lint_and_sugg(
-                    cx,
-                    MANUAL_ROTATE,
-                    expr.span,
-                    "there is no need to manually implement bit rotation",
-                    "this expression can be rewritten as",
-                    format!("{expr_sugg}.{shift_function}({amount})"),
-                    Applicability::MachineApplicable,
-                );
-            }
+
+                if let Some(Constant::Int(minuend)) = const_eval.eval_local(minuend, ctxt)
+                    && clippy_utils::eq_expr_value(cx, amount1, amount2)
+                    // (x << s) | (x >> bit_width - s)
+                    && ((binop.node == BinOpKind::Sub && u128::from(bit_width) == minuend)
+                        // (x << s) | (x >> (bit_width - 1) ^ s)
+                        || (binop.node == BinOpKind::BitXor && u128::from(bit_width).checked_sub(minuend) == Some(1)))
+                {
+                    // NOTE: we take these from the side that _doesn't_ have the binop, since it's probably simpler
+                    (shift_direction, amount1)
+                } else {
+                    return;
+                }
+            };
+
+            let mut applicability = Applicability::MachineApplicable;
+            let expr_sugg = sugg::Sugg::hir_with_applicability(cx, l_expr, "_", &mut applicability).maybe_paren();
+            let amount = sugg::Sugg::hir_with_applicability(cx, amount, "_", &mut applicability);
+            span_lint_and_sugg(
+                cx,
+                MANUAL_ROTATE,
+                expr.span,
+                "there is no need to manually implement bit rotation",
+                "this expression can be rewritten as",
+                format!("{expr_sugg}.{shift_function}({amount})"),
+                Applicability::MachineApplicable,
+            );
         }
     }
 }

tests/ui/manual_rotate.fixed

@@ -44,3 +44,20 @@ fn main() {
     let mut l = vec![12_u8, 34];
     let y = (l.pop().unwrap() << 3) + (l.pop().unwrap() >> 5);
 }
+
+fn issue13028() {
+    let s = 5;
+    let u = 5;
+    let x: u32 = 123456;
+
+    let _ = x.rotate_left(s);
+    //~^ manual_rotate
+    let _ = x.rotate_left(s);
+    //~^ manual_rotate
+    // still works with consts
+    let _ = x.rotate_right(9);
+    //~^ manual_rotate
+
+    // don't lint, because `s` and `u` are different variables, albeit with the same value
+    let _ = (x << s) | (x >> (32 - u));
+}

tests/ui/manual_rotate.rs

@@ -44,3 +44,20 @@ fn main() {
     let mut l = vec![12_u8, 34];
     let y = (l.pop().unwrap() << 3) + (l.pop().unwrap() >> 5);
 }
+
+fn issue13028() {
+    let s = 5;
+    let u = 5;
+    let x: u32 = 123456;
+
+    let _ = (x << s) | (x >> (32 - s));
+    //~^ manual_rotate
+    let _ = (x << s) | (x >> (31 ^ s));
+    //~^ manual_rotate
+    // still works with consts
+    let _ = (x >> 9) | (x << (32 - 9));
+    //~^ manual_rotate
+
+    // don't lint, because `s` and `u` are different variables, albeit with the same value
+    let _ = (x << s) | (x >> (32 - u));
+}

tests/ui/manual_rotate.stderr

@@ -73,5 +73,23 @@ error: there is no need to manually implement bit rotation
 LL |     let _ = (x_i64 >> N) | (x_i64 << (64 - N));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: this expression can be rewritten as: `x_i64.rotate_right(N)`
 
-error: aborting due to 12 previous errors
+error: there is no need to manually implement bit rotation
+  --> tests/ui/manual_rotate.rs:53:13
+   |
+LL |     let _ = (x << s) | (x >> (32 - s));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: this expression can be rewritten as: `x.rotate_left(s)`
+
+error: there is no need to manually implement bit rotation
+  --> tests/ui/manual_rotate.rs:55:13
+   |
+LL |     let _ = (x << s) | (x >> (31 ^ s));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: this expression can be rewritten as: `x.rotate_left(s)`
+
+error: there is no need to manually implement bit rotation
+  --> tests/ui/manual_rotate.rs:58:13
+   |
+LL |     let _ = (x >> 9) | (x << (32 - 9));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: this expression can be rewritten as: `x.rotate_right(9)`
+
+error: aborting due to 15 previous errors