From 021b7398e1c37c75c250e3b81a1b46622ce683c9 Mon Sep 17 00:00:00 2001
From: Alex Macleod <alex@macleod.io>
Date: Wed, 24 May 2023 21:39:06 +0000
Subject: [PATCH] Ignore #[cfg]'d out code in needless_else

---
 clippy_lints/src/needless_else.rs | 42 +++++++++++++++++--------------
 tests/ui/needless_else.fixed      | 17 +++++++++++++
 tests/ui/needless_else.rs         | 17 +++++++++++++
 tests/ui/needless_else.stderr     |  2 +-
 4 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/clippy_lints/src/needless_else.rs b/clippy_lints/src/needless_else.rs
index 8faf033e4..4ff1bf7ff 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 14f81728a..06a161627 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 fae118181..728032c47 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 a7b2f1959..ea6930851 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 {
    |  _______^