Skip to content

Commit

Permalink
Auto merge of rust-lang#12409 - cookie-s:fix-identityop-duplicate-err…
Browse files Browse the repository at this point in the history
…ors, r=Alexendoo

[`identity_op`]: Fix duplicate diagnostics

Relates to rust-lang#12379

In the `identity_op` lint, the following diagnostic was emitted two times

```
  --> tests/ui/identity_op.rs:156:5
   |
LL |     1 * 1;
   |     ^^^^^ help: consider reducing it to: `1`
   |
```

because both of the left operand and the right operand are the identity element of the multiplication.

This PR fixes the issue so that if a diagnostic is created for an operand, the check of the other operand will be skipped. It's fine because the result is always the same in the affected operators.

---

changelog: [`identity_op`]: Fix duplicate diagnostics
  • Loading branch information
bors committed Mar 4, 2024
2 parents e970fa5 + 3b9939e commit c0939b1
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 76 deletions.
41 changes: 21 additions & 20 deletions clippy_lints/src/operators/identity_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,15 @@ pub(crate) fn check<'tcx>(

match op {
BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => {
check_op(
let _ = check_op(
cx,
left,
0,
expr.span,
peeled_right_span,
needs_parenthesis(cx, expr, right),
right_is_coerced_to_value,
);
check_op(
) || check_op(
cx,
right,
0,
Expand All @@ -62,7 +61,7 @@ pub(crate) fn check<'tcx>(
);
},
BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => {
check_op(
let _ = check_op(
cx,
right,
0,
Expand All @@ -73,16 +72,26 @@ pub(crate) fn check<'tcx>(
);
},
BinOpKind::Mul => {
check_op(
let _ = check_op(
cx,
left,
1,
expr.span,
peeled_right_span,
needs_parenthesis(cx, expr, right),
right_is_coerced_to_value,
) || check_op(
cx,
right,
1,
expr.span,
peeled_left_span,
Parens::Unneeded,
left_is_coerced_to_value,
);
check_op(
},
BinOpKind::Div => {
let _ = check_op(
cx,
right,
1,
Expand All @@ -92,26 +101,16 @@ pub(crate) fn check<'tcx>(
left_is_coerced_to_value,
);
},
BinOpKind::Div => check_op(
cx,
right,
1,
expr.span,
peeled_left_span,
Parens::Unneeded,
left_is_coerced_to_value,
),
BinOpKind::BitAnd => {
check_op(
let _ = check_op(
cx,
left,
-1,
expr.span,
peeled_right_span,
needs_parenthesis(cx, expr, right),
right_is_coerced_to_value,
);
check_op(
) || check_op(
cx,
right,
-1,
Expand Down Expand Up @@ -201,12 +200,12 @@ fn check_remainder(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>, span
}
}

fn check_op(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens, is_erased: bool) {
fn check_op(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, parens: Parens, is_erased: bool) -> bool {
if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e).map(Constant::peel_refs) {
let check = match *cx.typeck_results().expr_ty(e).peel_refs().kind() {
ty::Int(ity) => unsext(cx.tcx, -1_i128, ity),
ty::Uint(uty) => clip(cx.tcx, !0, uty),
_ => return,
_ => return false,
};
if match m {
0 => v == 0,
Expand All @@ -215,8 +214,10 @@ fn check_op(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span, pa
_ => unreachable!(),
} {
span_ineffective_operation(cx, span, arg, parens, is_erased);
return true;
}
}
false
}

fn span_ineffective_operation(
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/identity_op.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
//@compile-flags: -Zdeduplicate-diagnostics=yes

#![warn(clippy::identity_op)]
#![allow(unused)]
#![allow(
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/identity_op.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
//@compile-flags: -Zdeduplicate-diagnostics=yes

#![warn(clippy::identity_op)]
#![allow(unused)]
#![allow(
Expand Down
Loading

0 comments on commit c0939b1

Please sign in to comment.