Skip to content

Commit a10ab29

Browse files
Arc8neArc8ne
committed
Handle another type of collapsible if-statement.
1 parent 76118ec commit a10ab29

File tree

4 files changed

+186
-5
lines changed

4 files changed

+186
-5
lines changed

‎clippy_lints/src/collapsible_if.rs

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
use clippy_config::Conf;
2-
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
3+
use clippy_utils::higher::If;
34
use clippy_utils::msrvs::{self, Msrv};
45
use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block_with_applicability};
5-
use clippy_utils::{span_contains_non_whitespace, tokenize_with_text};
6+
use clippy_utils::sugg::{Sugg, make_binop, make_unop};
7+
use clippy_utils::{SpanlessEq, span_contains_non_whitespace, tokenize_with_text};
68
use rustc_ast::BinOpKind;
79
use rustc_errors::Applicability;
8-
use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind};
10+
use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind, UnOp};
911
use rustc_lexer::TokenKind;
1012
use rustc_lint::{LateContext, LateLintPass};
1113
use rustc_session::impl_lint_pass;
@@ -209,6 +211,71 @@ impl CollapsibleIf {
209211
!matches!(cond.kind, ExprKind::Let(..))
210212
|| (cx.tcx.sess.edition().at_least_rust_2024() && self.msrv.meets(cx, msrvs::LET_CHAINS))
211213
}
214+
215+
fn check_collapsible_if_nested_if_else(&self, cx: &LateContext<'_>, if_expr: &Expr<'_>) {
216+
if let Some(If {
217+
cond,
218+
then: Expr {
219+
kind: ExprKind::Block(then_block, ..),
220+
..
221+
},
222+
r#else: Some(else_expr),
223+
}) = If::hir(if_expr)
224+
// && let Some(Expr {
225+
// kind: ExprKind::If(inner_cond, inner_then, ..),
226+
// ..
227+
// }) = expr_block(then_block)
228+
&& let Some(inner_if_expr) = expr_block(then_block)
229+
&& let Expr {
230+
kind: ExprKind::If(inner_cond, inner_then, ..),
231+
..
232+
} = inner_if_expr
233+
&& !block_starts_with_significant_tokens(cx, then_block, inner_if_expr, self.lint_commented_code)
234+
&& SpanlessEq::new(cx).eq_expr(inner_then, else_expr)
235+
{
236+
let first_cond_sugg = match cond.kind {
237+
ExprKind::Binary(bin_op, lhs, rhs) => {
238+
let new_bin_op = if let BinOpKind::Ne = bin_op.node {
239+
BinOpKind::Eq
240+
} else {
241+
BinOpKind::Ne
242+
};
243+
244+
make_binop(
245+
new_bin_op,
246+
&Sugg::hir_with_applicability(cx, lhs, "..", &mut Applicability::HasPlaceholders),
247+
&Sugg::hir_with_applicability(cx, rhs, "..", &mut Applicability::HasPlaceholders),
248+
)
249+
},
250+
ExprKind::Unary(UnOp::Not, expr) => {
251+
Sugg::hir_with_applicability(cx, expr, "..", &mut Applicability::HasPlaceholders)
252+
},
253+
ExprKind::Path(_) => make_unop(
254+
"!",
255+
Sugg::hir_with_applicability(cx, cond, "..", &mut Applicability::HasPlaceholders),
256+
),
257+
_ => Sugg::hir_with_applicability(cx, cond, "..", &mut Applicability::Unspecified),
258+
};
259+
260+
span_lint_and_sugg(
261+
cx,
262+
COLLAPSIBLE_IF,
263+
if_expr.span,
264+
"this `if` statement can be collapsed.",
265+
"collapse else and nested if blocks",
266+
format!(
267+
"if {} {}",
268+
make_binop(
269+
BinOpKind::Or,
270+
&first_cond_sugg,
271+
&Sugg::hir_with_applicability(cx, inner_cond, "..", &mut Applicability::HasPlaceholders)
272+
),
273+
Sugg::hir_with_applicability(cx, else_expr, "..", &mut Applicability::HasPlaceholders)
274+
),
275+
Applicability::MachineApplicable,
276+
);
277+
}
278+
}
212279
}
213280

214281
impl_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF, COLLAPSIBLE_ELSE_IF]);
@@ -222,6 +289,8 @@ impl LateLintPass<'_> for CollapsibleIf {
222289
&& let ExprKind::Block(else_, None) = else_.kind
223290
{
224291
self.check_collapsible_else_if(cx, then.span, else_);
292+
293+
self.check_collapsible_if_nested_if_else(cx, expr);
225294
} else if else_.is_none()
226295
&& self.eligible_condition(cx, cond)
227296
&& let ExprKind::Block(then, None) = then.kind

‎tests/ui/collapsible_if.fixed

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,39 @@ fn main() {
132132
println!("Hello world!");
133133
}
134134
}
135+
136+
let a = false;
137+
138+
let b = false;
139+
140+
if !a || b {
141+
println!("Hello world!");
142+
}
143+
//~^^^^^^^^ collapsible_if
144+
145+
if !a || b {
146+
println!("Hello world!");
147+
}
148+
149+
if a {
150+
// A comment that should not be removed.
151+
if b {
152+
println!("Hello world!");
153+
}
154+
} else {
155+
println!("Hello world!");
156+
}
157+
158+
let s = "foo";
159+
160+
if s == "foobar" || a {
161+
println!("Hello world!");
162+
}
163+
//~^^^^^^^^ collapsible_if
164+
165+
if s == "foobar" || a {
166+
println!("Hello world!");
167+
}
135168
}
136169

137170
#[rustfmt::skip]

‎tests/ui/collapsible_if.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,49 @@ fn main() {
142142
println!("Hello world!");
143143
}
144144
}
145+
146+
let a = false;
147+
148+
let b = false;
149+
150+
if a {
151+
if b {
152+
println!("Hello world!");
153+
}
154+
}
155+
else {
156+
println!("Hello world!");
157+
}
158+
//~^^^^^^^^ collapsible_if
159+
160+
if !a || b {
161+
println!("Hello world!");
162+
}
163+
164+
if a {
165+
// A comment that should not be removed.
166+
if b {
167+
println!("Hello world!");
168+
}
169+
} else {
170+
println!("Hello world!");
171+
}
172+
173+
let s = "foo";
174+
175+
if s != "foobar" {
176+
if a {
177+
println!("Hello world!");
178+
}
179+
}
180+
else {
181+
println!("Hello world!");
182+
}
183+
//~^^^^^^^^ collapsible_if
184+
185+
if s == "foobar" || a {
186+
println!("Hello world!");
187+
}
145188
}
146189

147190
#[rustfmt::skip]

‎tests/ui/collapsible_if.stderr

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,44 @@ LL | println!("No comment, linted");
172172
LL ~ }
173173
|
174174

175+
error: this `if` statement can be collapsed.
176+
--> tests/ui/collapsible_if.rs:150:5
177+
|
178+
LL | / if a {
179+
LL | | if b {
180+
LL | | println!("Hello world!");
181+
... |
182+
LL | | println!("Hello world!");
183+
LL | | }
184+
| |_____^
185+
|
186+
help: collapse else and nested if blocks
187+
|
188+
LL ~ if !a || b {
189+
LL + println!("Hello world!");
190+
LL + }
191+
|
192+
193+
error: this `if` statement can be collapsed.
194+
--> tests/ui/collapsible_if.rs:175:5
195+
|
196+
LL | / if s != "foobar" {
197+
LL | | if a {
198+
LL | | println!("Hello world!");
199+
... |
200+
LL | | println!("Hello world!");
201+
LL | | }
202+
| |_____^
203+
|
204+
help: collapse else and nested if blocks
205+
|
206+
LL ~ if s == "foobar" || a {
207+
LL + println!("Hello world!");
208+
LL + }
209+
|
210+
175211
error: this `if` statement can be collapsed
176-
--> tests/ui/collapsible_if.rs:149:5
212+
--> tests/ui/collapsible_if.rs:192:5
177213
|
178214
LL | / if true {
179215
LL | | if true {
@@ -190,5 +226,5 @@ LL | // This is a comment, do not collapse code to it
190226
LL ~ ; 3
191227
|
192228

193-
error: aborting due to 11 previous errors
229+
error: aborting due to 13 previous errors
194230

0 commit comments

Comments
 (0)