Skip to content

Commit 6da6c07

Browse files
committed
fix: collapsible_else_if FP on conditionally compiled stmt
1 parent 3927a61 commit 6da6c07

File tree

3 files changed

+49
-1
lines changed

3 files changed

+49
-1
lines changed

clippy_lints/src/collapsible_if.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
33
use clippy_utils::msrvs::{self, Msrv};
4-
use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block, snippet_block_with_applicability};
4+
use clippy_utils::source::{
5+
IntoSpan as _, SpanRangeExt, snippet, snippet_block, snippet_block_with_applicability, snippet_opt,
6+
};
7+
use clippy_utils::tokenize_with_text;
58
use rustc_ast::BinOpKind;
69
use rustc_errors::Applicability;
710
use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind};
11+
use rustc_lexer::TokenKind;
812
use rustc_lint::{LateContext, LateLintPass};
913
use rustc_session::impl_lint_pass;
1014
use rustc_span::Span;
@@ -96,6 +100,8 @@ impl CollapsibleIf {
96100
&& cx.tcx.hir_attrs(else_.hir_id).is_empty()
97101
&& !else_.span.from_expansion()
98102
&& let ExprKind::If(..) = else_.kind
103+
&& let up_to_if = else_block.span.until(else_.span)
104+
&& span_not_contain_stmt(cx, up_to_if)
99105
{
100106
// Prevent "elseif"
101107
// Check that the "else" is followed by whitespace
@@ -198,6 +204,12 @@ fn block_starts_with_comment(cx: &LateContext<'_>, block: &Block<'_>) -> bool {
198204
trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*")
199205
}
200206

207+
fn span_not_contain_stmt(cx: &LateContext<'_>, span: Span) -> bool {
208+
matches!(snippet_opt(cx, span), Some(text) if tokenize_with_text(&text[1..]).all(|(token, _, _)| {
209+
matches!(token, TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace)
210+
}))
211+
}
212+
201213
/// If `block` is a block with either one expression or a statement containing an expression,
202214
/// return the expression. We don't peel blocks recursively, as extra blocks might be intentional.
203215
fn expr_block<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {

tests/ui/collapsible_else_if.fixed

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,21 @@ fn issue_7318() {
8686
}else if false {}
8787
//~^^^ collapsible_else_if
8888
}
89+
90+
fn issue14799() {
91+
use std::ops::ControlFlow;
92+
93+
let c: ControlFlow<_, ()> = ControlFlow::Break(Some(42));
94+
if let ControlFlow::Break(Some(_)) = c {
95+
todo!();
96+
} else {
97+
#[cfg(target_os = "freebsd")]
98+
todo!();
99+
100+
if let ControlFlow::Break(None) = c {
101+
todo!();
102+
} else {
103+
todo!();
104+
}
105+
}
106+
}

tests/ui/collapsible_else_if.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,21 @@ fn issue_7318() {
102102
}
103103
//~^^^ collapsible_else_if
104104
}
105+
106+
fn issue14799() {
107+
use std::ops::ControlFlow;
108+
109+
let c: ControlFlow<_, ()> = ControlFlow::Break(Some(42));
110+
if let ControlFlow::Break(Some(_)) = c {
111+
todo!();
112+
} else {
113+
#[cfg(target_os = "freebsd")]
114+
todo!();
115+
116+
if let ControlFlow::Break(None) = c {
117+
todo!();
118+
} else {
119+
todo!();
120+
}
121+
}
122+
}

0 commit comments

Comments
 (0)