Skip to content

Commit a728ff5

Browse files
committed
fix: collapsible_if FP on block stmt before expr
1 parent 56f0182 commit a728ff5

File tree

3 files changed

+33
-24
lines changed

3 files changed

+33
-24
lines changed

clippy_lints/src/collapsible_if.rs

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
3+
use clippy_utils::peel_blocks_with_stmt;
34
use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block, snippet_block_with_applicability};
45
use rustc_ast::BinOpKind;
56
use rustc_errors::Applicability;
6-
use rustc_hir::{Block, Expr, ExprKind, StmtKind};
7+
use rustc_hir::{Expr, ExprKind};
78
use rustc_lint::{LateContext, LateLintPass};
89
use rustc_middle::ty::TyCtxt;
910
use rustc_session::impl_lint_pass;
@@ -90,9 +91,10 @@ impl CollapsibleIf {
9091
}
9192
}
9293

93-
fn check_collapsible_else_if(cx: &LateContext<'_>, then_span: Span, else_block: &Block<'_>) {
94+
fn check_collapsible_else_if(cx: &LateContext<'_>, then_span: Span, else_block: &Expr<'_>) {
9495
if !block_starts_with_comment(cx, else_block)
95-
&& let Some(else_) = expr_block(else_block)
96+
&& let else_ = peel_blocks_with_stmt(else_block)
97+
&& else_.hir_id != else_block.hir_id
9698
&& cx.tcx.hir_attrs(else_.hir_id).is_empty()
9799
&& !else_.span.from_expansion()
98100
&& let ExprKind::If(..) = else_.kind
@@ -123,8 +125,9 @@ impl CollapsibleIf {
123125
}
124126
}
125127

126-
fn check_collapsible_if_if(&self, cx: &LateContext<'_>, expr: &Expr<'_>, check: &Expr<'_>, then: &Block<'_>) {
127-
if let Some(inner) = expr_block(then)
128+
fn check_collapsible_if_if(&self, cx: &LateContext<'_>, expr: &Expr<'_>, check: &Expr<'_>, then: &Expr<'_>) {
129+
let inner = peel_blocks_with_stmt(then);
130+
if inner.hir_id != then.hir_id
128131
&& cx.tcx.hir_attrs(inner.hir_id).is_empty()
129132
&& let ExprKind::If(check_inner, _, None) = &inner.kind
130133
&& self.eligible_condition(check_inner)
@@ -176,43 +179,27 @@ impl LateLintPass<'_> for CollapsibleIf {
176179
&& !expr.span.from_expansion()
177180
{
178181
if let Some(else_) = else_
179-
&& let ExprKind::Block(else_, None) = else_.kind
182+
&& let ExprKind::Block(_, None) = else_.kind
180183
{
181184
Self::check_collapsible_else_if(cx, then.span, else_);
182185
} else if else_.is_none()
183186
&& self.eligible_condition(cond)
184-
&& let ExprKind::Block(then, None) = then.kind
187+
&& let ExprKind::Block(_, None) = then.kind
185188
{
186189
self.check_collapsible_if_if(cx, expr, cond, then);
187190
}
188191
}
189192
}
190193
}
191194

192-
fn block_starts_with_comment(cx: &LateContext<'_>, block: &Block<'_>) -> bool {
195+
fn block_starts_with_comment(cx: &LateContext<'_>, block: &Expr<'_>) -> bool {
193196
// We trim all opening braces and whitespaces and then check if the next string is a comment.
194197
let trimmed_block_text = snippet_block(cx, block.span, "..", None)
195198
.trim_start_matches(|c: char| c.is_whitespace() || c == '{')
196199
.to_owned();
197200
trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*")
198201
}
199202

200-
/// If `block` is a block with either one expression or a statement containing an expression,
201-
/// return the expression. We don't peel blocks recursively, as extra blocks might be intentional.
202-
fn expr_block<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {
203-
match block.stmts {
204-
[] => block.expr,
205-
[stmt] => {
206-
if let StmtKind::Semi(expr) = stmt.kind {
207-
Some(expr)
208-
} else {
209-
None
210-
}
211-
},
212-
_ => None,
213-
}
214-
}
215-
216203
/// If the expression is a `||`, suggest parentheses around it.
217204
fn parens_around(expr: &Expr<'_>) -> Vec<(Span, String)> {
218205
if let ExprKind::Binary(op, _, _) = expr.peel_drop_temps().kind

tests/ui/collapsible_if.fixed

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,3 +162,14 @@ fn layout_check() -> u32 {
162162
; 3
163163
//~^^^^^ collapsible_if
164164
}
165+
166+
fn issue14722() {
167+
let x = if true {
168+
Some(1)
169+
} else {
170+
if true {
171+
println!("Some debug information");
172+
};
173+
None
174+
};
175+
}

tests/ui/collapsible_if.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,3 +172,14 @@ fn layout_check() -> u32 {
172172
}; 3
173173
//~^^^^^ collapsible_if
174174
}
175+
176+
fn issue14722() {
177+
let x = if true {
178+
Some(1)
179+
} else {
180+
if true {
181+
println!("Some debug information");
182+
};
183+
None
184+
};
185+
}

0 commit comments

Comments
 (0)