about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-10-25 11:37:18 +0000
committerbors <bors@rust-lang.org>2020-10-25 11:37:18 +0000
commit718bb28f68bf1b693a6e3f1abd36360926a5707c (patch)
tree56a409e1c96244aef8fd080e460f706ed00060a1
parent38be214724f44ce4ff5944068cb1be3a79b0d218 (diff)
parent62f60e1ae5bd2287497746bf90a302903e0ae462 (diff)
downloadrust-718bb28f68bf1b693a6e3f1abd36360926a5707c.tar.gz
rust-718bb28f68bf1b693a6e3f1abd36360926a5707c.zip
Auto merge of #6211 - ThibsG:NeedlessBoolCfg, r=flip1995
No lint with `cfg!` and fix sugg for macro in `needless_bool` lint

Don't lint if `cfg!` macro is one of the operand.
Fix suggestion when the span originated from a macro, using `hir_with_macro_callsite`.

Fixes: #3973

changelog: none
-rw-r--r--clippy_lints/src/needless_bool.rs16
-rw-r--r--tests/ui/bool_comparison.fixed40
-rw-r--r--tests/ui/bool_comparison.rs40
-rw-r--r--tests/ui/bool_comparison.stderr62
4 files changed, 135 insertions, 23 deletions
diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
index dc5aa669139..a799a644e97 100644
--- a/clippy_lints/src/needless_bool.rs
+++ b/clippy_lints/src/needless_bool.rs
@@ -3,7 +3,9 @@
 //! This lint is **warn** by default
 
 use crate::utils::sugg::Sugg;
-use crate::utils::{higher, parent_node_is_if_expr, snippet_with_applicability, span_lint, span_lint_and_sugg};
+use crate::utils::{
+    higher, is_expn_of, parent_node_is_if_expr, snippet_with_applicability, span_lint, span_lint_and_sugg,
+};
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
@@ -233,6 +235,9 @@ fn check_comparison<'a, 'tcx>(
             cx.typeck_results().expr_ty(left_side),
             cx.typeck_results().expr_ty(right_side),
         );
+        if is_expn_of(left_side.span, "cfg").is_some() || is_expn_of(right_side.span, "cfg").is_some() {
+            return;
+        }
         if l_ty.is_bool() && r_ty.is_bool() {
             let mut applicability = Applicability::MachineApplicable;
 
@@ -295,7 +300,14 @@ fn suggest_bool_comparison<'a, 'tcx>(
     message: &str,
     conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>,
 ) {
-    let hint = Sugg::hir_with_applicability(cx, expr, "..", &mut applicability);
+    let hint = if expr.span.from_expansion() {
+        if applicability != Applicability::Unspecified {
+            applicability = Applicability::MaybeIncorrect;
+        }
+        Sugg::hir_with_macro_callsite(cx, expr, "..")
+    } else {
+        Sugg::hir_with_applicability(cx, expr, "..", &mut applicability)
+    };
     span_lint_and_sugg(
         cx,
         BOOL_COMPARISON,
diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed
index 91211764759..5a012ff4d27 100644
--- a/tests/ui/bool_comparison.fixed
+++ b/tests/ui/bool_comparison.fixed
@@ -1,6 +1,7 @@
 // run-rustfix
 
-#[warn(clippy::bool_comparison)]
+#![warn(clippy::bool_comparison)]
+
 fn main() {
     let x = true;
     if x {
@@ -127,3 +128,40 @@ fn issue4983() {
     if b == a {};
     if !b == !a {};
 }
+
+macro_rules! m {
+    ($func:ident) => {
+        $func()
+    };
+}
+
+fn func() -> bool {
+    true
+}
+
+#[allow(dead_code)]
+fn issue3973() {
+    // ok, don't lint on `cfg` invocation
+    if false == cfg!(feature = "debugging") {}
+    if cfg!(feature = "debugging") == false {}
+    if true == cfg!(feature = "debugging") {}
+    if cfg!(feature = "debugging") == true {}
+
+    // lint, could be simplified
+    if !m!(func) {}
+    if !m!(func) {}
+    if m!(func) {}
+    if m!(func) {}
+
+    // no lint with a variable
+    let is_debug = false;
+    if is_debug == cfg!(feature = "debugging") {}
+    if cfg!(feature = "debugging") == is_debug {}
+    if is_debug == m!(func) {}
+    if m!(func) == is_debug {}
+    let is_debug = true;
+    if is_debug == cfg!(feature = "debugging") {}
+    if cfg!(feature = "debugging") == is_debug {}
+    if is_debug == m!(func) {}
+    if m!(func) == is_debug {}
+}
diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs
index 01ee35859f0..c534bc25c20 100644
--- a/tests/ui/bool_comparison.rs
+++ b/tests/ui/bool_comparison.rs
@@ -1,6 +1,7 @@
 // run-rustfix
 
-#[warn(clippy::bool_comparison)]
+#![warn(clippy::bool_comparison)]
+
 fn main() {
     let x = true;
     if x == true {
@@ -127,3 +128,40 @@ fn issue4983() {
     if b == a {};
     if !b == !a {};
 }
+
+macro_rules! m {
+    ($func:ident) => {
+        $func()
+    };
+}
+
+fn func() -> bool {
+    true
+}
+
+#[allow(dead_code)]
+fn issue3973() {
+    // ok, don't lint on `cfg` invocation
+    if false == cfg!(feature = "debugging") {}
+    if cfg!(feature = "debugging") == false {}
+    if true == cfg!(feature = "debugging") {}
+    if cfg!(feature = "debugging") == true {}
+
+    // lint, could be simplified
+    if false == m!(func) {}
+    if m!(func) == false {}
+    if true == m!(func) {}
+    if m!(func) == true {}
+
+    // no lint with a variable
+    let is_debug = false;
+    if is_debug == cfg!(feature = "debugging") {}
+    if cfg!(feature = "debugging") == is_debug {}
+    if is_debug == m!(func) {}
+    if m!(func) == is_debug {}
+    let is_debug = true;
+    if is_debug == cfg!(feature = "debugging") {}
+    if cfg!(feature = "debugging") == is_debug {}
+    if is_debug == m!(func) {}
+    if m!(func) == is_debug {}
+}
diff --git a/tests/ui/bool_comparison.stderr b/tests/ui/bool_comparison.stderr
index 55d94b8257d..31522d4a525 100644
--- a/tests/ui/bool_comparison.stderr
+++ b/tests/ui/bool_comparison.stderr
@@ -1,5 +1,5 @@
 error: equality checks against true are unnecessary
-  --> $DIR/bool_comparison.rs:6:8
+  --> $DIR/bool_comparison.rs:7:8
    |
 LL |     if x == true {
    |        ^^^^^^^^^ help: try simplifying it as shown: `x`
@@ -7,106 +7,130 @@ LL |     if x == true {
    = note: `-D clippy::bool-comparison` implied by `-D warnings`
 
 error: equality checks against false can be replaced by a negation
-  --> $DIR/bool_comparison.rs:11:8
+  --> $DIR/bool_comparison.rs:12:8
    |
 LL |     if x == false {
    |        ^^^^^^^^^^ help: try simplifying it as shown: `!x`
 
 error: equality checks against true are unnecessary
-  --> $DIR/bool_comparison.rs:16:8
+  --> $DIR/bool_comparison.rs:17:8
    |
 LL |     if true == x {
    |        ^^^^^^^^^ help: try simplifying it as shown: `x`
 
 error: equality checks against false can be replaced by a negation
-  --> $DIR/bool_comparison.rs:21:8
+  --> $DIR/bool_comparison.rs:22:8
    |
 LL |     if false == x {
    |        ^^^^^^^^^^ help: try simplifying it as shown: `!x`
 
 error: inequality checks against true can be replaced by a negation
-  --> $DIR/bool_comparison.rs:26:8
+  --> $DIR/bool_comparison.rs:27:8
    |
 LL |     if x != true {
    |        ^^^^^^^^^ help: try simplifying it as shown: `!x`
 
 error: inequality checks against false are unnecessary
-  --> $DIR/bool_comparison.rs:31:8
+  --> $DIR/bool_comparison.rs:32:8
    |
 LL |     if x != false {
    |        ^^^^^^^^^^ help: try simplifying it as shown: `x`
 
 error: inequality checks against true can be replaced by a negation
-  --> $DIR/bool_comparison.rs:36:8
+  --> $DIR/bool_comparison.rs:37:8
    |
 LL |     if true != x {
    |        ^^^^^^^^^ help: try simplifying it as shown: `!x`
 
 error: inequality checks against false are unnecessary
-  --> $DIR/bool_comparison.rs:41:8
+  --> $DIR/bool_comparison.rs:42:8
    |
 LL |     if false != x {
    |        ^^^^^^^^^^ help: try simplifying it as shown: `x`
 
 error: less than comparison against true can be replaced by a negation
-  --> $DIR/bool_comparison.rs:46:8
+  --> $DIR/bool_comparison.rs:47:8
    |
 LL |     if x < true {
    |        ^^^^^^^^ help: try simplifying it as shown: `!x`
 
 error: greater than checks against false are unnecessary
-  --> $DIR/bool_comparison.rs:51:8
+  --> $DIR/bool_comparison.rs:52:8
    |
 LL |     if false < x {
    |        ^^^^^^^^^ help: try simplifying it as shown: `x`
 
 error: greater than checks against false are unnecessary
-  --> $DIR/bool_comparison.rs:56:8
+  --> $DIR/bool_comparison.rs:57:8
    |
 LL |     if x > false {
    |        ^^^^^^^^^ help: try simplifying it as shown: `x`
 
 error: less than comparison against true can be replaced by a negation
-  --> $DIR/bool_comparison.rs:61:8
+  --> $DIR/bool_comparison.rs:62:8
    |
 LL |     if true > x {
    |        ^^^^^^^^ help: try simplifying it as shown: `!x`
 
 error: order comparisons between booleans can be simplified
-  --> $DIR/bool_comparison.rs:67:8
+  --> $DIR/bool_comparison.rs:68:8
    |
 LL |     if x < y {
    |        ^^^^^ help: try simplifying it as shown: `!x & y`
 
 error: order comparisons between booleans can be simplified
-  --> $DIR/bool_comparison.rs:72:8
+  --> $DIR/bool_comparison.rs:73:8
    |
 LL |     if x > y {
    |        ^^^^^ help: try simplifying it as shown: `x & !y`
 
 error: this comparison might be written more concisely
-  --> $DIR/bool_comparison.rs:120:8
+  --> $DIR/bool_comparison.rs:121:8
    |
 LL |     if a == !b {};
    |        ^^^^^^^ help: try simplifying it as shown: `a != b`
 
 error: this comparison might be written more concisely
-  --> $DIR/bool_comparison.rs:121:8
+  --> $DIR/bool_comparison.rs:122:8
    |
 LL |     if !a == b {};
    |        ^^^^^^^ help: try simplifying it as shown: `a != b`
 
 error: this comparison might be written more concisely
-  --> $DIR/bool_comparison.rs:125:8
+  --> $DIR/bool_comparison.rs:126:8
    |
 LL |     if b == !a {};
    |        ^^^^^^^ help: try simplifying it as shown: `b != a`
 
 error: this comparison might be written more concisely
-  --> $DIR/bool_comparison.rs:126:8
+  --> $DIR/bool_comparison.rs:127:8
    |
 LL |     if !b == a {};
    |        ^^^^^^^ help: try simplifying it as shown: `b != a`
 
-error: aborting due to 18 previous errors
+error: equality checks against false can be replaced by a negation
+  --> $DIR/bool_comparison.rs:151:8
+   |
+LL |     if false == m!(func) {}
+   |        ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)`
+
+error: equality checks against false can be replaced by a negation
+  --> $DIR/bool_comparison.rs:152:8
+   |
+LL |     if m!(func) == false {}
+   |        ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)`
+
+error: equality checks against true are unnecessary
+  --> $DIR/bool_comparison.rs:153:8
+   |
+LL |     if true == m!(func) {}
+   |        ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)`
+
+error: equality checks against true are unnecessary
+  --> $DIR/bool_comparison.rs:154:8
+   |
+LL |     if m!(func) == true {}
+   |        ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)`
+
+error: aborting due to 22 previous errors