Auto merge of rust-lang#11991 - J-ZhengLi:issue9911, r=llogiq

stop [`bool_comparison`]'s suggestion from consuming parentheses

fixes: rust-lang#9907

---

changelog: stop [`bool_comparison`]'s suggestion from consuming parentheses

clippy_lints/src/needless_bool.rs

@@ -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,
                     );
@@ -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(),
@@ -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(),

tests/ui/bool_comparison.fixed

@@ -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;
+}

tests/ui/bool_comparison.rs

@@ -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;
+}

tests/ui/bool_comparison.stderr

@@ -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