diff --git a/clippy_lints/src/operators/identity_op.rs b/clippy_lints/src/operators/identity_op.rs index d8430f6492d2..e3ce1ae9427d 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 18b7401768df..927142d5f802 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 55483f4e5fb3..a50546929a82 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 856bebfa1ffc..79d1a7bd81c3 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`