Skip to content

Commit

Permalink
Fix parens getting removed for non-associative operators
Browse files Browse the repository at this point in the history
  • Loading branch information
hcbarker committed Nov 3, 2024
1 parent a0b7681 commit 6d738f6
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 26 deletions.
22 changes: 6 additions & 16 deletions clippy_lints/src/operators/identity_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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`
//
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/identity_op.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/identity_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/identity_op.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down

0 comments on commit 6d738f6

Please sign in to comment.