Skip to content

Commit

Permalink
fix incorrect suggestion for !(a >= b) as i32 == c
Browse files Browse the repository at this point in the history
  • Loading branch information
CoCo-Japan-pan committed Sep 3, 2024
1 parent a81f1c8 commit 9dce27c
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 6 deletions.
22 changes: 17 additions & 5 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,18 @@ fn check_inverted_bool_in_condition(
);
}

fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) {
fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>, need_parens: bool) {
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
{
let suggestion = if need_parens {
format!("({suggestion})")
} else {
suggestion
};
span_lint_and_sugg(
cx,
NONMINIMAL_BOOL,
Expand Down Expand Up @@ -476,7 +481,7 @@ fn terminal_stats(b: &Bool) -> Stats {
}

impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
fn bool_expr(&self, e: &'tcx Expr<'_>) {
fn bool_expr(&self, e: &'tcx Expr<'_>, need_parens: bool) {
let mut h2q = Hir2Qmm {
terminals: Vec::new(),
cx: self.cx,
Expand Down Expand Up @@ -569,7 +574,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
}
};
if improvements.is_empty() {
check_simplify_not(self.cx, e);
check_simplify_not(self.cx, e, need_parens);
} else {
nonminimal_bool_lint(
improvements
Expand All @@ -586,8 +591,15 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if !e.span.from_expansion() {
match &e.kind {
ExprKind::Cast(inner, _) => {
if self.cx.typeck_results().expr_ty(inner).is_bool() {
// we are casting bool into some type, so we may need a parenthesis (#12761)
self.bool_expr(inner, true);
return;
}
},
ExprKind::Binary(binop, _, _) if binop.node == BinOpKind::Or || binop.node == BinOpKind::And => {
self.bool_expr(e);
self.bool_expr(e, false);
},
ExprKind::Unary(UnOp::Not, inner) => {
if let ExprKind::Unary(UnOp::Not, ex) = inner.kind
Expand All @@ -596,7 +608,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
return;
}
if self.cx.typeck_results().node_types()[inner.hir_id].is_bool() {
self.bool_expr(e);
self.bool_expr(e, false);
}
},
_ => {},
Expand Down
7 changes: 7 additions & 0 deletions tests/ui/nonminimal_bool_methods.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,11 @@ fn issue_12625() {
if a as u64 > b {} //~ ERROR: this boolean expression can be simplified
}

fn issue_12761() {
let a = 0;
let b = 0;
let c = 0;
if (a < b) as i32 == c {} //~ ERROR: this boolean expression can be simplified
}

fn main() {}
7 changes: 7 additions & 0 deletions tests/ui/nonminimal_bool_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,11 @@ fn issue_12625() {
if !(a as u64 <= b) {} //~ ERROR: this boolean expression can be simplified
}

fn issue_12761() {
let a = 0;
let b = 0;
let c = 0;
if !(a >= b) as i32 == c {} //~ ERROR: this boolean expression can be simplified
}

fn main() {}
8 changes: 7 additions & 1 deletion tests/ui/nonminimal_bool_methods.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,11 @@ error: this boolean expression can be simplified
LL | if !(a as u64 <= b) {}
| ^^^^^^^^^^^^^^^^ help: try: `a as u64 > b`

error: aborting due to 16 previous errors
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:122:8
|
LL | if !(a >= b) as i32 == c {}
| ^^^^^^^^^ help: try: `(a < b)`

error: aborting due to 17 previous errors

0 comments on commit 9dce27c

Please sign in to comment.