Fix parens getting removed for non-associative operators

4 files changed, 18 insertions, 26 deletions

diff --git a/clippy_lints/src/operators/identity_op.rs b/clippy_lints/src/operators/identity_op.rs
index d8430f6492d..e3ce1ae9427 100644
--- a/clippy_lints/src/operators/identity_op.rs
+++ b/clippy_lints/src/operators/identity_op.rs
@@ -52,13 +52,8 @@ pub(crate) fn check<'tcx>(
         },
         BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => {
             if is_redundant_op(cx, right, 0) {
-                span_ineffective_operation(
-                    cx,
-                    expr.span,
-                    peeled_left_span,
-                    Parens::Unneeded,
-                    left_is_coerced_to_value,
-                );
+                let paren = needs_parenthesis(cx, expr, left);
+                span_ineffective_operation(cx, expr.span, peeled_left_span, paren, left_is_coerced_to_value);
             }
         },
         BinOpKind::Mul => {
@@ -127,17 +122,12 @@ fn needs_parenthesis(cx: &LateContext<'_>, binary: &Expr<'_>, child: &Expr<'_>)
     match child.kind {
         ExprKind::Binary(_, lhs, _) | ExprKind::Cast(lhs, _) => {
             // For casts and binary expressions, we want to add parenthesis if
-            // the parent HIR node is an expression with a different precedence,
-            // or if the parent HIR node is a Block or Stmt, and the new left hand side
-            // would be treated as a statement rather than an expression.
+            // the parent HIR node is an expression, or if the parent HIR node
+            // is a Block or Stmt, and the new left hand side would need
+            // parenthesis be treated as a statement rather than an expression.
             if let Some((_, parent)) = cx.tcx.hir().parent_iter(binary.hir_id).next() {
-                if let Node::Expr(expr) = parent {
-                    if child.precedence().order() != expr.precedence().order() {
-                        return Parens::Needed;
-                    }
-                    return Parens::Unneeded;
-                }
                 match parent {
+                    Node::Expr(_) => return Parens::Needed,
                     Node::Block(_) | Node::Stmt(_) => {
                         // ensure we're checking against the leftmost expression of `child`
                         //
diff --git a/tests/ui/identity_op.fixed b/tests/ui/identity_op.fixed
index 18b7401768d..927142d5f80 100644
--- a/tests/ui/identity_op.fixed
+++ b/tests/ui/identity_op.fixed
@@ -235,8 +235,9 @@ fn issue_13470() {
     // Maintain parenthesis if the parent expr is of higher precedence
     let _ = 2i32 * (x + y);
     //~^ ERROR: this operation has no effect
-    // No need for parenthesis if the parent expr is of equal precedence
-    let _ = 2i32 + x + y;
+    // Maintain parenthesis if the parent expr is the same precedence
+    // as not all operations are associative
+    let _ = 2i32 - (x - y);
     //~^ ERROR: this operation has no effect
     // But make sure that inner parens still exist
     let z = 1i32;
diff --git a/tests/ui/identity_op.rs b/tests/ui/identity_op.rs
index 55483f4e5fb..a50546929a8 100644
--- a/tests/ui/identity_op.rs
+++ b/tests/ui/identity_op.rs
@@ -235,8 +235,9 @@ fn issue_13470() {
     // Maintain parenthesis if the parent expr is of higher precedence
     let _ = 2i32 * (x + y + 0i32);
     //~^ ERROR: this operation has no effect
-    // No need for parenthesis if the parent expr is of equal precedence
-    let _ = 2i32 + (x + y + 0i32);
+    // Maintain parenthesis if the parent expr is the same precedence
+    // as not all operations are associative
+    let _ = 2i32 - (x - y - 0i32);
     //~^ ERROR: this operation has no effect
     // But make sure that inner parens still exist
     let z = 1i32;
diff --git a/tests/ui/identity_op.stderr b/tests/ui/identity_op.stderr
index 856bebfa1ff..79d1a7bd81c 100644
--- a/tests/ui/identity_op.stderr
+++ b/tests/ui/identity_op.stderr
@@ -350,25 +350,25 @@ LL |     let _ = 2i32 * (x + y + 0i32);
    |                    ^^^^^^^^^^^^^^ help: consider reducing it to: `(x + y)`
 
 error: this operation has no effect
-  --> tests/ui/identity_op.rs:239:20
+  --> tests/ui/identity_op.rs:240:20
    |
-LL |     let _ = 2i32 + (x + y + 0i32);
-   |                    ^^^^^^^^^^^^^^ help: consider reducing it to: `x + y`
+LL |     let _ = 2i32 - (x - y - 0i32);
+   |                    ^^^^^^^^^^^^^^ help: consider reducing it to: `(x - y)`
 
 error: this operation has no effect
-  --> tests/ui/identity_op.rs:243:17
+  --> tests/ui/identity_op.rs:244:17
    |
 LL |     let _ = 2 + (x + (y * z) + 0);
    |                 ^^^^^^^^^^^^^^^^^ help: consider reducing it to: `x + (y * z)`
 
 error: this operation has no effect
-  --> tests/ui/identity_op.rs:247:20
+  --> tests/ui/identity_op.rs:248:20
    |
 LL |     let _ = 2i32 + (x * y * 1i32);
    |                    ^^^^^^^^^^^^^^ help: consider reducing it to: `(x * y)`
 
 error: this operation has no effect
-  --> tests/ui/identity_op.rs:252:25
+  --> tests/ui/identity_op.rs:253:25
    |
 LL |     let _: u64 = 1u64 + ((x as i32 + y as i32) as u64 + 0u64);
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider reducing it to: `(x as i32 + y as i32) as u64`