Skip to content

Commit

Permalink
Auto merge of #10822 - Alexendoo:needless-else-cfg, r=llogiq
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
bors committed May 27, 2023
2 parents f1fd467 + 021b739 commit c9ddcf0
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 20 deletions.
42 changes: 23 additions & 19 deletions clippy_lints/src/needless_else.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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,
);
}
}
}
17 changes: 17 additions & 0 deletions tests/ui/needless_else.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ macro_rules! mac {
};
}

macro_rules! empty_expansion {
() => {};
}

fn main() {
let b = std::hint::black_box(true);

Expand All @@ -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!();
}
}
17 changes: 17 additions & 0 deletions tests/ui/needless_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ macro_rules! mac {
};
}

macro_rules! empty_expansion {
() => {};
}

fn main() {
let b = std::hint::black_box(true);

Expand All @@ -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!();
}
}
2 changes: 1 addition & 1 deletion tests/ui/needless_else.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this else branch is empty
--> $DIR/needless_else.rs:20:7
--> $DIR/needless_else.rs:24:7
|
LL | } else {
| _______^
Expand Down

0 comments on commit c9ddcf0

Please sign in to comment.