Skip to content

Commit

Permalink
Auto merge of #12845 - cookie-s:dedup-boolmethods-diags, r=Jarcho
Browse files Browse the repository at this point in the history
Dedup nonminimal_bool_methods diags

Relates to #12379

Fix `nonminimal_bool` lint so that it doesn't check the same span multiple times.

`NotSimplificationVisitor` was called for each expression from `NonminimalBoolVisitor` whereas `NotSimplificationVisitor` also recursively checked all expressions.

---

changelog: [`nonminimal_bool`]: Fix duplicate diagnostics
  • Loading branch information
bors committed Jun 5, 2024
2 parents 10d1f32 + b6350e9 commit bc00d7b
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 49 deletions.
49 changes: 20 additions & 29 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,25 @@ fn check_inverted_bool_in_condition(
);
}

fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) {
if let ExprKind::Unary(UnOp::Not, inner) = &expr.kind
&& !expr.span.from_expansion()
&& !inner.span.from_expansion()
&& let Some(suggestion) = simplify_not(cx, inner)
&& cx.tcx.lint_level_at_node(NONMINIMAL_BOOL, expr.hir_id).0 != Level::Allow
{
span_lint_and_sugg(
cx,
NONMINIMAL_BOOL,
expr.span,
"this boolean expression can be simplified",
"try",
suggestion,
Applicability::MachineApplicable,
);
}
}

struct NonminimalBoolVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
}
Expand Down Expand Up @@ -542,8 +561,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
}
};
if improvements.is_empty() {
let mut visitor = NotSimplificationVisitor { cx: self.cx };
visitor.visit_expr(e);
check_simplify_not(self.cx, e);
} else {
nonminimal_bool_lint(
improvements
Expand Down Expand Up @@ -586,30 +604,3 @@ fn implements_ord(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
.get_diagnostic_item(sym::Ord)
.map_or(false, |id| implements_trait(cx, ty, id, &[]))
}

struct NotSimplificationVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
}

impl<'a, 'tcx> Visitor<'tcx> for NotSimplificationVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if let ExprKind::Unary(UnOp::Not, inner) = &expr.kind
&& !expr.span.from_expansion()
&& !inner.span.from_expansion()
&& let Some(suggestion) = simplify_not(self.cx, inner)
&& self.cx.tcx.lint_level_at_node(NONMINIMAL_BOOL, expr.hir_id).0 != Level::Allow
{
span_lint_and_sugg(
self.cx,
NONMINIMAL_BOOL,
expr.span,
"this boolean expression can be simplified",
"try",
suggestion,
Applicability::MachineApplicable,
);
}

walk_expr(self, expr);
}
}
2 changes: 0 additions & 2 deletions tests/ui/nonminimal_bool_methods.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
//@compile-flags: -Zdeduplicate-diagnostics=yes

#![allow(unused, clippy::diverging_sub_expression, clippy::needless_if)]
#![warn(clippy::nonminimal_bool)]

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

#![allow(unused, clippy::diverging_sub_expression, clippy::needless_if)]
#![warn(clippy::nonminimal_bool)]

Expand Down
32 changes: 16 additions & 16 deletions tests/ui/nonminimal_bool_methods.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:10:13
--> tests/ui/nonminimal_bool_methods.rs:8:13
|
LL | let _ = !a.is_some();
| ^^^^^^^^^^^^ help: try: `a.is_none()`
Expand All @@ -8,91 +8,91 @@ LL | let _ = !a.is_some();
= help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:12:13
--> tests/ui/nonminimal_bool_methods.rs:10:13
|
LL | let _ = !a.is_none();
| ^^^^^^^^^^^^ help: try: `a.is_some()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:14:13
--> tests/ui/nonminimal_bool_methods.rs:12:13
|
LL | let _ = !b.is_err();
| ^^^^^^^^^^^ help: try: `b.is_ok()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:16:13
--> tests/ui/nonminimal_bool_methods.rs:14:13
|
LL | let _ = !b.is_ok();
| ^^^^^^^^^^ help: try: `b.is_err()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:18:13
--> tests/ui/nonminimal_bool_methods.rs:16:13
|
LL | let _ = !(a.is_some() && !c);
| ^^^^^^^^^^^^^^^^^^^^ help: try: `a.is_none() || c`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:19:13
--> tests/ui/nonminimal_bool_methods.rs:17:13
|
LL | let _ = !(a.is_some() || !c);
| ^^^^^^^^^^^^^^^^^^^^ help: try: `a.is_none() && c`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:20:26
--> tests/ui/nonminimal_bool_methods.rs:18:26
|
LL | let _ = !(!c ^ c) || !a.is_some();
| ^^^^^^^^^^^^ help: try: `a.is_none()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:21:25
--> tests/ui/nonminimal_bool_methods.rs:19:25
|
LL | let _ = (!c ^ c) || !a.is_some();
| ^^^^^^^^^^^^ help: try: `a.is_none()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:22:23
--> tests/ui/nonminimal_bool_methods.rs:20:23
|
LL | let _ = !c ^ c || !a.is_some();
| ^^^^^^^^^^^^ help: try: `a.is_none()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:94:8
--> tests/ui/nonminimal_bool_methods.rs:92:8
|
LL | if !res.is_ok() {}
| ^^^^^^^^^^^^ help: try: `res.is_err()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:95:8
--> tests/ui/nonminimal_bool_methods.rs:93:8
|
LL | if !res.is_err() {}
| ^^^^^^^^^^^^^ help: try: `res.is_ok()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:98:8
--> tests/ui/nonminimal_bool_methods.rs:96:8
|
LL | if !res.is_some() {}
| ^^^^^^^^^^^^^^ help: try: `res.is_none()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:99:8
--> tests/ui/nonminimal_bool_methods.rs:97:8
|
LL | if !res.is_none() {}
| ^^^^^^^^^^^^^^ help: try: `res.is_some()`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:115:8
--> tests/ui/nonminimal_bool_methods.rs:113:8
|
LL | if !(a as u64 >= b) {}
| ^^^^^^^^^^^^^^^^ help: try: `(a as u64) < b`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:116:8
--> tests/ui/nonminimal_bool_methods.rs:114:8
|
LL | if !((a as u64) >= b) {}
| ^^^^^^^^^^^^^^^^^^ help: try: `(a as u64) < b`

error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:117:8
--> tests/ui/nonminimal_bool_methods.rs:115:8
|
LL | if !(a as u64 <= b) {}
| ^^^^^^^^^^^^^^^^ help: try: `a as u64 > b`
Expand Down

0 comments on commit bc00d7b

Please sign in to comment.