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 { | _______^