Skip to content

Commit

Permalink
stop [bool_comparison]'s suggestion from consuming parentheses
Browse files Browse the repository at this point in the history
  • Loading branch information
J-ZhengLi committed Dec 21, 2023
1 parent 7e650b7 commit a556d00
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 12 deletions.
38 changes: 27 additions & 11 deletions clippy_lints/src/needless_bool.rs
Expand Up @@ -340,20 +340,36 @@ fn check_comparison<'a, 'tcx>(
}
if l_ty.is_bool() && r_ty.is_bool() {
let mut applicability = Applicability::MachineApplicable;
// Eliminate parentheses in `e` by using the lo pos of lhs and hi pos of rhs,
// calling `source_callsite` make sure macros are handled correctly, see issue #9907
let binop_span = left_side
.span
.source_callsite()
.with_hi(right_side.span.source_callsite().hi());

if op.node == BinOpKind::Eq {
let expression_info = one_side_is_unary_not(left_side, right_side);
if expression_info.one_side_is_unary_not {
span_lint_and_sugg(
cx,
BOOL_COMPARISON,
e.span,
binop_span,
"this comparison might be written more concisely",
"try simplifying it as shown",
format!(
"{} != {}",
snippet_with_applicability(cx, expression_info.left_span, "..", &mut applicability),
snippet_with_applicability(cx, expression_info.right_span, "..", &mut applicability)
snippet_with_applicability(
cx,
expression_info.left_span.source_callsite(),
"..",
&mut applicability
),
snippet_with_applicability(
cx,
expression_info.right_span.source_callsite(),
"..",
&mut applicability
)
),
applicability,
);
Expand All @@ -362,24 +378,24 @@ fn check_comparison<'a, 'tcx>(

match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
(Some(true), None) => left_true.map_or((), |(h, m)| {
suggest_bool_comparison(cx, e, right_side, applicability, m, h);
suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h);
}),
(None, Some(true)) => right_true.map_or((), |(h, m)| {
suggest_bool_comparison(cx, e, left_side, applicability, m, h);
suggest_bool_comparison(cx, binop_span, left_side, applicability, m, h);
}),
(Some(false), None) => left_false.map_or((), |(h, m)| {
suggest_bool_comparison(cx, e, right_side, applicability, m, h);
suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h);
}),
(None, Some(false)) => right_false.map_or((), |(h, m)| {
suggest_bool_comparison(cx, e, left_side, applicability, m, h);
suggest_bool_comparison(cx, binop_span, left_side, applicability, m, h);
}),
(None, None) => no_literal.map_or((), |(h, m)| {
let left_side = Sugg::hir_with_applicability(cx, left_side, "..", &mut applicability);
let right_side = Sugg::hir_with_applicability(cx, right_side, "..", &mut applicability);
span_lint_and_sugg(
cx,
BOOL_COMPARISON,
e.span,
binop_span,
m,
"try simplifying it as shown",
h(left_side, right_side).to_string(),
Expand All @@ -394,17 +410,17 @@ fn check_comparison<'a, 'tcx>(

fn suggest_bool_comparison<'a, 'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx Expr<'_>,
span: Span,
expr: &Expr<'_>,
mut app: Applicability,
message: &str,
conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>,
) {
let hint = Sugg::hir_with_context(cx, expr, e.span.ctxt(), "..", &mut app);
let hint = Sugg::hir_with_context(cx, expr, span.ctxt(), "..", &mut app);
span_lint_and_sugg(
cx,
BOOL_COMPARISON,
e.span,
span,
message,
"try simplifying it as shown",
conv_hint(hint).to_string(),
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/bool_comparison.fixed
Expand Up @@ -165,3 +165,12 @@ fn issue3973() {
if is_debug == m!(func) {}
if m!(func) == is_debug {}
}

#[allow(clippy::unnecessary_cast)]
fn issue9907() {
let _ = (1 >= 2) as usize;
let _ = (!m!(func)) as usize;
// This is not part of the issue, but an unexpected found when fixing the issue,
// the provided span was inside of macro rather than the macro callsite.
let _ = ((1 < 2) != m!(func)) as usize;
}
9 changes: 9 additions & 0 deletions tests/ui/bool_comparison.rs
Expand Up @@ -165,3 +165,12 @@ fn issue3973() {
if is_debug == m!(func) {}
if m!(func) == is_debug {}
}

#[allow(clippy::unnecessary_cast)]
fn issue9907() {
let _ = ((1 < 2) == false) as usize;
let _ = (false == m!(func)) as usize;
// This is not part of the issue, but an unexpected found when fixing the issue,
// the provided span was inside of macro rather than the macro callsite.
let _ = ((1 < 2) == !m!(func)) as usize;
}
20 changes: 19 additions & 1 deletion tests/ui/bool_comparison.stderr
Expand Up @@ -133,5 +133,23 @@ error: equality checks against true are unnecessary
LL | if m!(func) == true {}
| ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)`

error: aborting due to 22 previous errors
error: equality checks against false can be replaced by a negation
--> $DIR/bool_comparison.rs:171:14
|
LL | let _ = ((1 < 2) == false) as usize;
| ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `1 >= 2`

error: equality checks against false can be replaced by a negation
--> $DIR/bool_comparison.rs:172:14
|
LL | let _ = (false == m!(func)) as usize;
| ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)`

error: this comparison might be written more concisely
--> $DIR/bool_comparison.rs:175:14
|
LL | let _ = ((1 < 2) == !m!(func)) as usize;
| ^^^^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `(1 < 2) != m!(func)`

error: aborting due to 25 previous errors

0 comments on commit a556d00

Please sign in to comment.