diff options
| -rw-r--r-- | clippy_lints/src/redundant_else.rs | 46 | ||||
| -rw-r--r-- | tests/ui/redundant_else.fixed | 154 | ||||
| -rw-r--r-- | tests/ui/redundant_else.stderr | 77 |
3 files changed, 252 insertions, 25 deletions
diff --git a/clippy_lints/src/redundant_else.rs b/clippy_lints/src/redundant_else.rs index a27f9b63114..2995cd8d9f7 100644 --- a/clippy_lints/src/redundant_else.rs +++ b/clippy_lints/src/redundant_else.rs @@ -1,9 +1,13 @@ -use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::{indent_of, reindent_multiline, snippet}; use rustc_ast::ast::{Block, Expr, ExprKind, Stmt, StmtKind}; use rustc_ast::visit::{Visitor, walk_expr}; +use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::declare_lint_pass; +use rustc_span::Span; +use std::borrow::Cow; declare_clippy_lint! { /// ### What it does @@ -76,13 +80,27 @@ impl EarlyLintPass for RedundantElse { _ => break, } } - span_lint_and_help( + + let mut app = Applicability::MachineApplicable; + if let ExprKind::Block(block, _) = &els.kind { + for stmt in &block.stmts { + // If the `else` block contains a local binding or a macro invocation, Clippy shouldn't auto-fix it + if matches!(&stmt.kind, StmtKind::Let(_) | StmtKind::MacCall(_)) { + app = Applicability::Unspecified; + break; + } + } + } + + // FIXME: The indentation of the suggestion would be the same as the one of the macro invocation in this implementation, see https://github.com/rust-lang/rust-clippy/pull/13936#issuecomment-2569548202 + span_lint_and_sugg( cx, REDUNDANT_ELSE, - els.span, + els.span.with_lo(then.span.hi()), "redundant else block", - None, "remove the `else` block and move the contents out", + make_sugg(cx, els.span, "..", Some(expr.span)).to_string(), + app, ); } } @@ -137,3 +155,23 @@ impl BreakVisitor { self.check(stmt, Self::visit_stmt) } } + +// Extract the inner contents of an `else` block str +// e.g. `{ foo(); bar(); }` -> `foo(); bar();` +fn extract_else_block(mut block: &str) -> String { + block = block.strip_prefix("{").unwrap_or(block); + block = block.strip_suffix("}").unwrap_or(block); + block.trim_end().to_string() +} + +fn make_sugg<'a>( + cx: &EarlyContext<'_>, + els_span: Span, + default: &'a str, + indent_relative_to: Option<Span>, +) -> Cow<'a, str> { + let extracted = extract_else_block(&snippet(cx, els_span, default)); + let indent = indent_relative_to.and_then(|s| indent_of(cx, s)); + + reindent_multiline(extracted.into(), false, indent) +} diff --git a/tests/ui/redundant_else.fixed b/tests/ui/redundant_else.fixed new file mode 100644 index 00000000000..47aa79302d2 --- /dev/null +++ b/tests/ui/redundant_else.fixed @@ -0,0 +1,154 @@ +#![warn(clippy::redundant_else)] +#![allow(clippy::needless_return, clippy::if_same_then_else, clippy::needless_late_init)] + +fn main() { + loop { + // break + if foo() { + println!("Love your neighbor;"); + break; + } + //~^ ERROR: redundant else block + println!("yet don't pull down your hedge."); + // continue + if foo() { + println!("He that lies down with Dogs,"); + continue; + } + //~^ ERROR: redundant else block + println!("shall rise up with fleas."); + // match block + if foo() { + match foo() { + 1 => break, + _ => return, + } + } + //~^ ERROR: redundant else block + println!("You may delay, but time will not."); + } + // else if + if foo() { + return; + } else if foo() { + return; + } + //~^ ERROR: redundant else block + println!("A fat kitchen makes a lean will."); + // let binding outside of block + let _ = { + if foo() { + return; + } + //~^ ERROR: redundant else block + 1 + }; + // else if with let binding outside of block + let _ = { + if foo() { + return; + } else if foo() { + return; + } + //~^ ERROR: redundant else block + 2 + }; + // inside if let + let _ = if let Some(1) = foo() { + let _ = 1; + if foo() { + return; + } + //~^ ERROR: redundant else block + 1 + } else { + 1 + }; + + // + // non-lint cases + // + + // sanity check + if foo() { + let _ = 1; + } else { + println!("Who is wise? He that learns from every one."); + } + // else if without else + if foo() { + return; + } else if foo() { + foo() + }; + // nested if return + if foo() { + if foo() { + return; + } + } else { + foo() + }; + // match with non-breaking branch + if foo() { + match foo() { + 1 => foo(), + _ => return, + } + } else { + println!("Three may keep a secret, if two of them are dead."); + } + // let binding + let _ = if foo() { + return; + } else { + 1 + }; + // assign + let mut a; + a = if foo() { + return; + } else { + 1 + }; + // assign-op + a += if foo() { + return; + } else { + 1 + }; + // if return else if else + if foo() { + return; + } else if foo() { + 1 + } else { + 2 + }; + // if else if return else + if foo() { + 1 + } else if foo() { + return; + } else { + 2 + }; + // else if with let binding + let _ = if foo() { + return; + } else if foo() { + return; + } else { + 2 + }; + // inside function call + Box::new(if foo() { + return; + } else { + 1 + }); +} + +fn foo<T>() -> T { + unimplemented!("I'm not Santa Claus") +} diff --git a/tests/ui/redundant_else.stderr b/tests/ui/redundant_else.stderr index b649a210b5f..ecc16f7cda5 100644 --- a/tests/ui/redundant_else.stderr +++ b/tests/ui/redundant_else.stderr @@ -1,88 +1,123 @@ error: redundant else block - --> tests/ui/redundant_else.rs:10:16 + --> tests/ui/redundant_else.rs:10:10 | LL | } else { - | ________________^ + | __________^ LL | | LL | | println!("yet don't pull down your hedge."); LL | | } | |_________^ | - = help: remove the `else` block and move the contents out = note: `-D clippy::redundant-else` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::redundant_else)]` +help: remove the `else` block and move the contents out + | +LL ~ } +LL + +LL + println!("yet don't pull down your hedge."); + | error: redundant else block - --> tests/ui/redundant_else.rs:18:16 + --> tests/ui/redundant_else.rs:18:10 | LL | } else { - | ________________^ + | __________^ LL | | LL | | println!("shall rise up with fleas."); LL | | } | |_________^ | - = help: remove the `else` block and move the contents out +help: remove the `else` block and move the contents out + | +LL ~ } +LL + +LL + println!("shall rise up with fleas."); + | error: redundant else block - --> tests/ui/redundant_else.rs:28:16 + --> tests/ui/redundant_else.rs:28:10 | LL | } else { - | ________________^ + | __________^ LL | | LL | | println!("You may delay, but time will not."); LL | | } | |_________^ | - = help: remove the `else` block and move the contents out +help: remove the `else` block and move the contents out + | +LL ~ } +LL + +LL + println!("You may delay, but time will not."); + | error: redundant else block - --> tests/ui/redundant_else.rs:38:12 + --> tests/ui/redundant_else.rs:38:6 | LL | } else { - | ____________^ + | ______^ LL | | LL | | println!("A fat kitchen makes a lean will."); LL | | } | |_____^ | - = help: remove the `else` block and move the contents out +help: remove the `else` block and move the contents out + | +LL ~ } +LL + +LL + println!("A fat kitchen makes a lean will."); + | error: redundant else block - --> tests/ui/redundant_else.rs:46:16 + --> tests/ui/redundant_else.rs:46:10 | LL | } else { - | ________________^ + | __________^ LL | | LL | | 1 LL | | } | |_________^ | - = help: remove the `else` block and move the contents out +help: remove the `else` block and move the contents out + | +LL ~ } +LL + +LL + 1 + | error: redundant else block - --> tests/ui/redundant_else.rs:57:16 + --> tests/ui/redundant_else.rs:57:10 | LL | } else { - | ________________^ + | __________^ LL | | LL | | 2 LL | | } | |_________^ | - = help: remove the `else` block and move the contents out +help: remove the `else` block and move the contents out + | +LL ~ } +LL + +LL + 2 + | error: redundant else block - --> tests/ui/redundant_else.rs:67:16 + --> tests/ui/redundant_else.rs:67:10 | LL | } else { - | ________________^ + | __________^ LL | | LL | | 1 LL | | } | |_________^ | - = help: remove the `else` block and move the contents out +help: remove the `else` block and move the contents out + | +LL ~ } +LL + +LL + 1 + | error: aborting due to 7 previous errors |