Skip to content

Commit 88a00a8

Browse files
blyxyasblyxyas
authored
autofix for redundant_else lint (#13936)
changelog: [`redundant_else`]: autofix for some cases `redundant_else` can be fixed automatically.
2 parents e02c885 + 7aae4f4 commit 88a00a8

File tree

3 files changed

+252
-25
lines changed

3 files changed

+252
-25
lines changed

‎clippy_lints/src/redundant_else.rs

+42-4
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1-
use clippy_utils::diagnostics::span_lint_and_help;
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::{indent_of, reindent_multiline, snippet};
23
use rustc_ast::ast::{Block, Expr, ExprKind, Stmt, StmtKind};
34
use rustc_ast::visit::{Visitor, walk_expr};
5+
use rustc_errors::Applicability;
46
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
57
use rustc_middle::lint::in_external_macro;
68
use rustc_session::declare_lint_pass;
9+
use rustc_span::Span;
10+
use std::borrow::Cow;
711

812
declare_clippy_lint! {
913
/// ### What it does
@@ -76,13 +80,27 @@ impl EarlyLintPass for RedundantElse {
7680
_ => break,
7781
}
7882
}
79-
span_lint_and_help(
83+
84+
let mut app = Applicability::MachineApplicable;
85+
if let ExprKind::Block(block, _) = &els.kind {
86+
for stmt in &block.stmts {
87+
// If the `else` block contains a local binding or a macro invocation, Clippy shouldn't auto-fix it
88+
if matches!(&stmt.kind, StmtKind::Let(_) | StmtKind::MacCall(_)) {
89+
app = Applicability::Unspecified;
90+
break;
91+
}
92+
}
93+
}
94+
95+
// 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
96+
span_lint_and_sugg(
8097
cx,
8198
REDUNDANT_ELSE,
82-
els.span,
99+
els.span.with_lo(then.span.hi()),
83100
"redundant else block",
84-
None,
85101
"remove the `else` block and move the contents out",
102+
make_sugg(cx, els.span, "..", Some(expr.span)).to_string(),
103+
app,
86104
);
87105
}
88106
}
@@ -137,3 +155,23 @@ impl BreakVisitor {
137155
self.check(stmt, Self::visit_stmt)
138156
}
139157
}
158+
159+
// Extract the inner contents of an `else` block str
160+
// e.g. `{ foo(); bar(); }` -> `foo(); bar();`
161+
fn extract_else_block(mut block: &str) -> String {
162+
block = block.strip_prefix("{").unwrap_or(block);
163+
block = block.strip_suffix("}").unwrap_or(block);
164+
block.trim_end().to_string()
165+
}
166+
167+
fn make_sugg<'a>(
168+
cx: &EarlyContext<'_>,
169+
els_span: Span,
170+
default: &'a str,
171+
indent_relative_to: Option<Span>,
172+
) -> Cow<'a, str> {
173+
let extracted = extract_else_block(&snippet(cx, els_span, default));
174+
let indent = indent_relative_to.and_then(|s| indent_of(cx, s));
175+
176+
reindent_multiline(extracted.into(), false, indent)
177+
}

‎tests/ui/redundant_else.fixed

+154
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
#![warn(clippy::redundant_else)]
2+
#![allow(clippy::needless_return, clippy::if_same_then_else, clippy::needless_late_init)]
3+
4+
fn main() {
5+
loop {
6+
// break
7+
if foo() {
8+
println!("Love your neighbor;");
9+
break;
10+
}
11+
//~^ ERROR: redundant else block
12+
println!("yet don't pull down your hedge.");
13+
// continue
14+
if foo() {
15+
println!("He that lies down with Dogs,");
16+
continue;
17+
}
18+
//~^ ERROR: redundant else block
19+
println!("shall rise up with fleas.");
20+
// match block
21+
if foo() {
22+
match foo() {
23+
1 => break,
24+
_ => return,
25+
}
26+
}
27+
//~^ ERROR: redundant else block
28+
println!("You may delay, but time will not.");
29+
}
30+
// else if
31+
if foo() {
32+
return;
33+
} else if foo() {
34+
return;
35+
}
36+
//~^ ERROR: redundant else block
37+
println!("A fat kitchen makes a lean will.");
38+
// let binding outside of block
39+
let _ = {
40+
if foo() {
41+
return;
42+
}
43+
//~^ ERROR: redundant else block
44+
1
45+
};
46+
// else if with let binding outside of block
47+
let _ = {
48+
if foo() {
49+
return;
50+
} else if foo() {
51+
return;
52+
}
53+
//~^ ERROR: redundant else block
54+
2
55+
};
56+
// inside if let
57+
let _ = if let Some(1) = foo() {
58+
let _ = 1;
59+
if foo() {
60+
return;
61+
}
62+
//~^ ERROR: redundant else block
63+
1
64+
} else {
65+
1
66+
};
67+
68+
//
69+
// non-lint cases
70+
//
71+
72+
// sanity check
73+
if foo() {
74+
let _ = 1;
75+
} else {
76+
println!("Who is wise? He that learns from every one.");
77+
}
78+
// else if without else
79+
if foo() {
80+
return;
81+
} else if foo() {
82+
foo()
83+
};
84+
// nested if return
85+
if foo() {
86+
if foo() {
87+
return;
88+
}
89+
} else {
90+
foo()
91+
};
92+
// match with non-breaking branch
93+
if foo() {
94+
match foo() {
95+
1 => foo(),
96+
_ => return,
97+
}
98+
} else {
99+
println!("Three may keep a secret, if two of them are dead.");
100+
}
101+
// let binding
102+
let _ = if foo() {
103+
return;
104+
} else {
105+
1
106+
};
107+
// assign
108+
let mut a;
109+
a = if foo() {
110+
return;
111+
} else {
112+
1
113+
};
114+
// assign-op
115+
a += if foo() {
116+
return;
117+
} else {
118+
1
119+
};
120+
// if return else if else
121+
if foo() {
122+
return;
123+
} else if foo() {
124+
1
125+
} else {
126+
2
127+
};
128+
// if else if return else
129+
if foo() {
130+
1
131+
} else if foo() {
132+
return;
133+
} else {
134+
2
135+
};
136+
// else if with let binding
137+
let _ = if foo() {
138+
return;
139+
} else if foo() {
140+
return;
141+
} else {
142+
2
143+
};
144+
// inside function call
145+
Box::new(if foo() {
146+
return;
147+
} else {
148+
1
149+
});
150+
}
151+
152+
fn foo<T>() -> T {
153+
unimplemented!("I'm not Santa Claus")
154+
}

‎tests/ui/redundant_else.stderr

+56-21
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,123 @@
11
error: redundant else block
2-
--> tests/ui/redundant_else.rs:10:16
2+
--> tests/ui/redundant_else.rs:10:10
33
|
44
LL | } else {
5-
| ________________^
5+
| __________^
66
LL | |
77
LL | | println!("yet don't pull down your hedge.");
88
LL | | }
99
| |_________^
1010
|
11-
= help: remove the `else` block and move the contents out
1211
= note: `-D clippy::redundant-else` implied by `-D warnings`
1312
= help: to override `-D warnings` add `#[allow(clippy::redundant_else)]`
13+
help: remove the `else` block and move the contents out
14+
|
15+
LL ~ }
16+
LL +
17+
LL + println!("yet don't pull down your hedge.");
18+
|
1419

1520
error: redundant else block
16-
--> tests/ui/redundant_else.rs:18:16
21+
--> tests/ui/redundant_else.rs:18:10
1722
|
1823
LL | } else {
19-
| ________________^
24+
| __________^
2025
LL | |
2126
LL | | println!("shall rise up with fleas.");
2227
LL | | }
2328
| |_________^
2429
|
25-
= help: remove the `else` block and move the contents out
30+
help: remove the `else` block and move the contents out
31+
|
32+
LL ~ }
33+
LL +
34+
LL + println!("shall rise up with fleas.");
35+
|
2636

2737
error: redundant else block
28-
--> tests/ui/redundant_else.rs:28:16
38+
--> tests/ui/redundant_else.rs:28:10
2939
|
3040
LL | } else {
31-
| ________________^
41+
| __________^
3242
LL | |
3343
LL | | println!("You may delay, but time will not.");
3444
LL | | }
3545
| |_________^
3646
|
37-
= help: remove the `else` block and move the contents out
47+
help: remove the `else` block and move the contents out
48+
|
49+
LL ~ }
50+
LL +
51+
LL + println!("You may delay, but time will not.");
52+
|
3853

3954
error: redundant else block
40-
--> tests/ui/redundant_else.rs:38:12
55+
--> tests/ui/redundant_else.rs:38:6
4156
|
4257
LL | } else {
43-
| ____________^
58+
| ______^
4459
LL | |
4560
LL | | println!("A fat kitchen makes a lean will.");
4661
LL | | }
4762
| |_____^
4863
|
49-
= help: remove the `else` block and move the contents out
64+
help: remove the `else` block and move the contents out
65+
|
66+
LL ~ }
67+
LL +
68+
LL + println!("A fat kitchen makes a lean will.");
69+
|
5070

5171
error: redundant else block
52-
--> tests/ui/redundant_else.rs:46:16
72+
--> tests/ui/redundant_else.rs:46:10
5373
|
5474
LL | } else {
55-
| ________________^
75+
| __________^
5676
LL | |
5777
LL | | 1
5878
LL | | }
5979
| |_________^
6080
|
61-
= help: remove the `else` block and move the contents out
81+
help: remove the `else` block and move the contents out
82+
|
83+
LL ~ }
84+
LL +
85+
LL + 1
86+
|
6287

6388
error: redundant else block
64-
--> tests/ui/redundant_else.rs:57:16
89+
--> tests/ui/redundant_else.rs:57:10
6590
|
6691
LL | } else {
67-
| ________________^
92+
| __________^
6893
LL | |
6994
LL | | 2
7095
LL | | }
7196
| |_________^
7297
|
73-
= help: remove the `else` block and move the contents out
98+
help: remove the `else` block and move the contents out
99+
|
100+
LL ~ }
101+
LL +
102+
LL + 2
103+
|
74104

75105
error: redundant else block
76-
--> tests/ui/redundant_else.rs:67:16
106+
--> tests/ui/redundant_else.rs:67:10
77107
|
78108
LL | } else {
79-
| ________________^
109+
| __________^
80110
LL | |
81111
LL | | 1
82112
LL | | }
83113
| |_________^
84114
|
85-
= help: remove the `else` block and move the contents out
115+
help: remove the `else` block and move the contents out
116+
|
117+
LL ~ }
118+
LL +
119+
LL + 1
120+
|
86121

87122
error: aborting due to 7 previous errors
88123

0 commit comments

Comments
 (0)