Skip to content

Commit 97598e9

Browse files
borsbors
committed
Auto merge of #10810 - samueltardieu:needless-else, r=llogiq
needless_else: new lint to check for empty `else` clauses Empty `else` clauses are useless. They happen in the wild and are not linted yet: https://github.com/uutils/coreutils/pull/4880/files `else` clauses containing or preceded by comments are not linted as the comments might be important. changelog: [`needless_else`]: new lint
2 parents fe792d9 + e6646eb commit 97598e9

18 files changed

+231
-70
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4874,6 +4874,7 @@ Released 2018-09-13
48744874
[`needless_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
48754875
[`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue
48764876
[`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
4877+
[`needless_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_else
48774878
[`needless_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each
48784879
[`needless_late_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_late_init
48794880
[`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
449449
crate::needless_bool::NEEDLESS_BOOL_ASSIGN_INFO,
450450
crate::needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE_INFO,
451451
crate::needless_continue::NEEDLESS_CONTINUE_INFO,
452+
crate::needless_else::NEEDLESS_ELSE_INFO,
452453
crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
453454
crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
454455
crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,

‎clippy_lints/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ mod needless_arbitrary_self_type;
218218
mod needless_bool;
219219
mod needless_borrowed_ref;
220220
mod needless_continue;
221+
mod needless_else;
221222
mod needless_for_each;
222223
mod needless_late_init;
223224
mod needless_parens_on_range_literals;
@@ -992,6 +993,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
992993
store.register_late_pass(|_| Box::new(items_after_test_module::ItemsAfterTestModule));
993994
store.register_early_pass(|| Box::new(ref_patterns::RefPatterns));
994995
store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs));
996+
store.register_early_pass(|| Box::new(needless_else::NeedlessElse));
995997
// add lints here, do not remove this comment, it's used in `new_lint`
996998
}
997999

‎clippy_lints/src/needless_else.rs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
use clippy_utils::{diagnostics::span_lint_and_sugg, source::trim_span, span_extract_comment};
2+
use rustc_ast::ast::{Expr, ExprKind};
3+
use rustc_errors::Applicability;
4+
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
5+
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
7+
declare_clippy_lint! {
8+
/// ### What it does
9+
/// Checks for empty `else` branches.
10+
///
11+
/// ### Why is this bad?
12+
/// An empty else branch does nothing and can be removed.
13+
///
14+
/// ### Example
15+
/// ```rust
16+
///# fn check() -> bool { true }
17+
/// if check() {
18+
/// println!("Check successful!");
19+
/// } else {
20+
/// }
21+
/// ```
22+
/// Use instead:
23+
/// ```rust
24+
///# fn check() -> bool { true }
25+
/// if check() {
26+
/// println!("Check successful!");
27+
/// }
28+
/// ```
29+
#[clippy::version = "1.71.0"]
30+
pub NEEDLESS_ELSE,
31+
style,
32+
"empty else branch"
33+
}
34+
declare_lint_pass!(NeedlessElse => [NEEDLESS_ELSE]);
35+
36+
impl EarlyLintPass for NeedlessElse {
37+
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
38+
if let ExprKind::If(_, then_block, Some(else_clause)) = &expr.kind &&
39+
let ExprKind::Block(block, _) = &else_clause.kind &&
40+
!expr.span.from_expansion() &&
41+
!else_clause.span.from_expansion() &&
42+
block.stmts.is_empty() {
43+
let span = trim_span(cx.sess().source_map(), expr.span.trim_start(then_block.span).unwrap());
44+
if span_extract_comment(cx.sess().source_map(), span).is_empty() {
45+
span_lint_and_sugg(
46+
cx,
47+
NEEDLESS_ELSE,
48+
span,
49+
"this else branch is empty",
50+
"you can remove it",
51+
String::new(),
52+
Applicability::MachineApplicable,
53+
);
54+
}
55+
}
56+
}
57+
}

‎tests/ui-toml/ifs_same_cond/ifs_same_cond.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::ifs_same_cond)]
2-
#![allow(clippy::if_same_then_else, clippy::comparison_chain)]
2+
#![allow(clippy::if_same_then_else, clippy::comparison_chain, clippy::needless_else)]
33

44
fn main() {}
55

‎tests/ui/branches_sharing_code/valid_if_blocks.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
#![deny(clippy::branches_sharing_code, clippy::if_same_then_else)]
22
#![allow(dead_code)]
3-
#![allow(clippy::mixed_read_write_in_expression, clippy::uninlined_format_args)]
3+
#![allow(
4+
clippy::mixed_read_write_in_expression,
5+
clippy::uninlined_format_args,
6+
clippy::needless_else
7+
)]
48

59
// This tests valid if blocks that shouldn't trigger the lint
610

‎tests/ui/branches_sharing_code/valid_if_blocks.stderr‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: this `if` has identical blocks
2-
--> $DIR/valid_if_blocks.rs:105:14
2+
--> $DIR/valid_if_blocks.rs:109:14
33
|
44
LL | if false {
55
| ______________^
66
LL | | } else {
77
| |_____^
88
|
99
note: same as this
10-
--> $DIR/valid_if_blocks.rs:106:12
10+
--> $DIR/valid_if_blocks.rs:110:12
1111
|
1212
LL | } else {
1313
| ____________^
@@ -20,7 +20,7 @@ LL | #![deny(clippy::branches_sharing_code, clippy::if_same_then_else)]
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^
2121

2222
error: this `if` has identical blocks
23-
--> $DIR/valid_if_blocks.rs:116:15
23+
--> $DIR/valid_if_blocks.rs:120:15
2424
|
2525
LL | if x == 0 {
2626
| _______________^
@@ -31,7 +31,7 @@ LL | | } else {
3131
| |_____^
3232
|
3333
note: same as this
34-
--> $DIR/valid_if_blocks.rs:120:12
34+
--> $DIR/valid_if_blocks.rs:124:12
3535
|
3636
LL | } else {
3737
| ____________^
@@ -42,19 +42,19 @@ LL | | }
4242
| |_____^
4343

4444
error: this `if` has identical blocks
45-
--> $DIR/valid_if_blocks.rs:127:23
45+
--> $DIR/valid_if_blocks.rs:131:23
4646
|
4747
LL | let _ = if x == 6 { 7 } else { 7 };
4848
| ^^^^^
4949
|
5050
note: same as this
51-
--> $DIR/valid_if_blocks.rs:127:34
51+
--> $DIR/valid_if_blocks.rs:131:34
5252
|
5353
LL | let _ = if x == 6 { 7 } else { 7 };
5454
| ^^^^^
5555

5656
error: this `if` has identical blocks
57-
--> $DIR/valid_if_blocks.rs:133:23
57+
--> $DIR/valid_if_blocks.rs:137:23
5858
|
5959
LL | } else if x == 68 {
6060
| _______________________^
@@ -66,7 +66,7 @@ LL | | } else {
6666
| |_____^
6767
|
6868
note: same as this
69-
--> $DIR/valid_if_blocks.rs:138:12
69+
--> $DIR/valid_if_blocks.rs:142:12
7070
|
7171
LL | } else {
7272
| ____________^
@@ -78,7 +78,7 @@ LL | | };
7878
| |_____^
7979

8080
error: this `if` has identical blocks
81-
--> $DIR/valid_if_blocks.rs:147:23
81+
--> $DIR/valid_if_blocks.rs:151:23
8282
|
8383
LL | } else if x == 68 {
8484
| _______________________^
@@ -88,7 +88,7 @@ LL | | } else {
8888
| |_____^
8989
|
9090
note: same as this
91-
--> $DIR/valid_if_blocks.rs:150:12
91+
--> $DIR/valid_if_blocks.rs:154:12
9292
|
9393
LL | } else {
9494
| ____________^

‎tests/ui/crashes/ice-7410.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#![no_std]
77
#![allow(clippy::if_same_then_else)]
88
#![allow(clippy::redundant_pattern_matching)]
9+
#![allow(clippy::needless_else)]
910

1011
use core::panic::PanicInfo;
1112

‎tests/ui/ifs_same_cond.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::ifs_same_cond)]
2-
#![allow(clippy::if_same_then_else, clippy::comparison_chain)] // all empty blocks
2+
#![allow(clippy::if_same_then_else, clippy::comparison_chain, clippy::needless_else)] // all empty blocks
33

44
fn ifs_same_cond() {
55
let a = 0;

‎tests/ui/needless_else.fixed‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
//@run-rustfix
2+
#![allow(unused)]
3+
#![warn(clippy::needless_else)]
4+
#![allow(clippy::suspicious_else_formatting)]
5+
6+
macro_rules! mac {
7+
($test:expr) => {
8+
if $test {
9+
println!("Test successful!");
10+
} else {
11+
}
12+
};
13+
}
14+
15+
fn main() {
16+
let b = std::hint::black_box(true);
17+
18+
if b {
19+
println!("Foobar");
20+
}
21+
22+
if b {
23+
println!("Foobar");
24+
} else {
25+
// Do not lint because this comment might be important
26+
}
27+
28+
if b {
29+
println!("Foobar");
30+
} else
31+
/* Do not lint because this comment might be important */
32+
{
33+
}
34+
35+
// Do not lint because of the expression
36+
let _ = if b { 1 } else { 2 };
37+
38+
// Do not lint because inside a macro
39+
mac!(b);
40+
}

0 commit comments

Comments
 (0)