Auto merge of #10004 - Jarcho:issue_9904, r=llogiq

Don't suggest keeping borrows in `identity_op` fixes #9904 changelog: `identity_op`: Don't suggest keeping borrows
rust-lang · Dec 9, 2022 · 5a3a722 · 5a3a722
2 parents d4cd91c + 03ba0ea
commit 5a3a722
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 13 deletions.
diff --git a/clippy_lints/src/operators/identity_op.rs b/clippy_lints/src/operators/identity_op.rs
@@ -1,7 +1,7 @@
 use clippy_utils::consts::{constant_full_int, constant_simple, Constant, FullInt};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::{clip, unsext};
+use clippy_utils::{clip, peel_hir_expr_refs, unsext};
 use rustc_errors::Applicability;
 use rustc_hir::{BinOpKind, Expr, ExprKind, Node};
 use rustc_lint::LateContext;
@@ -20,20 +20,76 @@ pub(crate) fn check<'tcx>(
     if !is_allowed(cx, op, left, right) {
         match op {
             BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => {
-                check_op(cx, left, 0, expr.span, right.span, needs_parenthesis(cx, expr, right));
-                check_op(cx, right, 0, expr.span, left.span, Parens::Unneeded);
+                check_op(
+                    cx,
+                    left,
+                    0,
+                    expr.span,
+                    peel_hir_expr_refs(right).0.span,
+                    needs_parenthesis(cx, expr, right),
+                );
+                check_op(
+                    cx,
+                    right,
+                    0,
+                    expr.span,
+                    peel_hir_expr_refs(left).0.span,
+                    Parens::Unneeded,
+                );
             },
             BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => {
-                check_op(cx, right, 0, expr.span, left.span, Parens::Unneeded);
+                check_op(
+                    cx,
+                    right,
+                    0,
+                    expr.span,
+                    peel_hir_expr_refs(left).0.span,
+                    Parens::Unneeded,
+                );
             },
             BinOpKind::Mul => {
-                check_op(cx, left, 1, expr.span, right.span, needs_parenthesis(cx, expr, right));
-                check_op(cx, right, 1, expr.span, left.span, Parens::Unneeded);
+                check_op(
+                    cx,
+                    left,
+                    1,
+                    expr.span,
+                    peel_hir_expr_refs(right).0.span,
+                    needs_parenthesis(cx, expr, right),
+                );
+                check_op(
+                    cx,
+                    right,
+                    1,
+                    expr.span,
+                    peel_hir_expr_refs(left).0.span,
+                    Parens::Unneeded,
+                );
             },
-            BinOpKind::Div => check_op(cx, right, 1, expr.span, left.span, Parens::Unneeded),
+            BinOpKind::Div => check_op(
+                cx,
+                right,
+                1,
+                expr.span,
+                peel_hir_expr_refs(left).0.span,
+                Parens::Unneeded,
+            ),
             BinOpKind::BitAnd => {
-                check_op(cx, left, -1, expr.span, right.span, needs_parenthesis(cx, expr, right));
-                check_op(cx, right, -1, expr.span, left.span, Parens::Unneeded);
+                check_op(
+                    cx,
+                    left,
+                    -1,
+                    expr.span,
+                    peel_hir_expr_refs(right).0.span,
+                    needs_parenthesis(cx, expr, right),
+                );
+                check_op(
+                    cx,
+                    right,
+                    -1,
+                    expr.span,
+                    peel_hir_expr_refs(left).0.span,
+                    Parens::Unneeded,
+                );
             },
             BinOpKind::Rem => check_remainder(cx, left, right, expr.span, left.span),
             _ => (),

diff --git a/tests/ui/identity_op.fixed b/tests/ui/identity_op.fixed
@@ -65,7 +65,7 @@ fn main() {
     42;
     1;
     42;
-    &x;
+    x;
     x;
 
     let mut a = A(String::new());
@@ -112,6 +112,10 @@ fn main() {
     2 * { a };
     (({ a } + 4));
     1;
+
+    // Issue #9904
+    let x = 0i32;
+    let _: i32 = x;
 }
 
 pub fn decide(a: bool, b: bool) -> u32 {

diff --git a/tests/ui/identity_op.rs b/tests/ui/identity_op.rs
@@ -112,6 +112,10 @@ fn main() {
     2 * (0 + { a });
     1 * ({ a } + 4);
     1 * 1;
+
+    // Issue #9904
+    let x = 0i32;
+    let _: i32 = &x + 0;
 }
 
 pub fn decide(a: bool, b: bool) -> u32 {

diff --git a/tests/ui/identity_op.stderr b/tests/ui/identity_op.stderr
@@ -70,7 +70,7 @@ error: this operation has no effect
   --> $DIR/identity_op.rs:68:5
    |
 LL |     &x >> 0;
-   |     ^^^^^^^ help: consider reducing it to: `&x`
+   |     ^^^^^^^ help: consider reducing it to: `x`
 
 error: this operation has no effect
   --> $DIR/identity_op.rs:69:5
@@ -229,10 +229,16 @@ LL |     1 * 1;
    |     ^^^^^ help: consider reducing it to: `1`
 
 error: this operation has no effect
-  --> $DIR/identity_op.rs:118:5
+  --> $DIR/identity_op.rs:118:18
+   |
+LL |     let _: i32 = &x + 0;
+   |                  ^^^^^^ help: consider reducing it to: `x`
+
+error: this operation has no effect
+  --> $DIR/identity_op.rs:122:5
    |
 LL |     0 + if a { 1 } else { 2 } + if b { 3 } else { 5 }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(if a { 1 } else { 2 })`
 
-error: aborting due to 39 previous errors
+error: aborting due to 40 previous errors