Auto merge of #10822 - Alexendoo:needless-else-cfg, r=llogiq

Ignore `#[cfg]`'d out code in `needless_else`

changelog: none (same release as #10810)

`#[cfg]` making things fun once more

This lead me to think about macro calls that expand to nothing as well, but apparently they produce an empty stmt in the AST so are already handled, added a test for that

r? `@llogiq`

4 files changed, 58 insertions, 20 deletions

diff --git a/clippy_lints/src/needless_else.rs b/clippy_lints/src/needless_else.rs
index 8faf033e406..4ff1bf7ffc0 100644
--- a/clippy_lints/src/needless_else.rs
+++ b/clippy_lints/src/needless_else.rs
@@ -1,4 +1,5 @@
-use clippy_utils::{diagnostics::span_lint_and_sugg, source::trim_span, span_extract_comment};
+use clippy_utils::source::snippet_opt;
+use clippy_utils::{diagnostics::span_lint_and_sugg, source::trim_span};
 use rustc_ast::ast::{Expr, ExprKind};
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
@@ -35,23 +36,26 @@ declare_lint_pass!(NeedlessElse => [NEEDLESS_ELSE]);
 
 impl EarlyLintPass for NeedlessElse {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
-        if let ExprKind::If(_, then_block, Some(else_clause)) = &expr.kind &&
-            let ExprKind::Block(block, _) = &else_clause.kind &&
-            !expr.span.from_expansion() &&
-            !else_clause.span.from_expansion() &&
-            block.stmts.is_empty() {
-                let span = trim_span(cx.sess().source_map(), expr.span.trim_start(then_block.span).unwrap());
-                if span_extract_comment(cx.sess().source_map(), span).is_empty() {
-                    span_lint_and_sugg(
-                        cx,
-                        NEEDLESS_ELSE,
-                        span,
-                        "this else branch is empty",
-                        "you can remove it",
-                        String::new(),
-                        Applicability::MachineApplicable,
-                    );
-                }
-            }
+        if let ExprKind::If(_, then_block, Some(else_clause)) = &expr.kind
+            && let ExprKind::Block(block, _) = &else_clause.kind
+            && !expr.span.from_expansion()
+            && !else_clause.span.from_expansion()
+            && block.stmts.is_empty()
+            && let Some(trimmed) = expr.span.trim_start(then_block.span)
+            && let span = trim_span(cx.sess().source_map(), trimmed)
+            && let Some(else_snippet) = snippet_opt(cx, span)
+            // Ignore else blocks that contain comments or #[cfg]s
+            && !else_snippet.contains(['/', '#'])
+        {
+            span_lint_and_sugg(
+                cx,
+                NEEDLESS_ELSE,
+                span,
+                "this else branch is empty",
+                "you can remove it",
+                String::new(),
+                Applicability::MachineApplicable,
+            );
+        }
     }
 }
diff --git a/tests/ui/needless_else.fixed b/tests/ui/needless_else.fixed
index 14f81728a86..06a16162790 100644
--- a/tests/ui/needless_else.fixed
+++ b/tests/ui/needless_else.fixed
@@ -12,6 +12,10 @@ macro_rules! mac {
     };
 }
 
+macro_rules! empty_expansion {
+    () => {};
+}
+
 fn main() {
     let b = std::hint::black_box(true);
 
@@ -37,4 +41,17 @@ fn main() {
 
     // Do not lint because inside a macro
     mac!(b);
+
+    if b {
+        println!("Foobar");
+    } else {
+        #[cfg(foo)]
+        "Do not lint cfg'd out code"
+    }
+
+    if b {
+        println!("Foobar");
+    } else {
+        empty_expansion!();
+    }
 }
diff --git a/tests/ui/needless_else.rs b/tests/ui/needless_else.rs
index fae11818141..728032c47a6 100644
--- a/tests/ui/needless_else.rs
+++ b/tests/ui/needless_else.rs
@@ -12,6 +12,10 @@ macro_rules! mac {
     };
 }
 
+macro_rules! empty_expansion {
+    () => {};
+}
+
 fn main() {
     let b = std::hint::black_box(true);
 
@@ -38,4 +42,17 @@ fn main() {
 
     // Do not lint because inside a macro
     mac!(b);
+
+    if b {
+        println!("Foobar");
+    } else {
+        #[cfg(foo)]
+        "Do not lint cfg'd out code"
+    }
+
+    if b {
+        println!("Foobar");
+    } else {
+        empty_expansion!();
+    }
 }
diff --git a/tests/ui/needless_else.stderr b/tests/ui/needless_else.stderr
index a7b2f1959c7..ea693085164 100644
--- a/tests/ui/needless_else.stderr
+++ b/tests/ui/needless_else.stderr
@@ -1,5 +1,5 @@
 error: this else branch is empty
-  --> $DIR/needless_else.rs:20:7
+  --> $DIR/needless_else.rs:24:7
    |
 LL |       } else {
    |  _______^