Skip to content

Commit 5290b1e

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

File tree

4 files changed

+52
-3
lines changed

4 files changed

+52
-3
lines changed

clippy_lints/src/collapsible_if.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ use clippy_config::Conf;
22
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
33
use clippy_utils::msrvs::{self, Msrv};
44
use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block, snippet_block_with_applicability};
5+
use clippy_utils::span_contains_non_comment;
56
use rustc_ast::BinOpKind;
67
use rustc_errors::Applicability;
78
use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind};
89
use rustc_lint::{LateContext, LateLintPass};
910
use rustc_session::impl_lint_pass;
10-
use rustc_span::Span;
11+
use rustc_span::{BytePos, Span};
1112

1213
declare_clippy_lint! {
1314
/// ### What it does
@@ -96,6 +97,8 @@ impl CollapsibleIf {
9697
&& cx.tcx.hir_attrs(else_.hir_id).is_empty()
9798
&& !else_.span.from_expansion()
9899
&& let ExprKind::If(..) = else_.kind
100+
&& let up_to_if = else_block.span.until(else_.span)
101+
&& !span_contains_non_comment(cx, up_to_if.with_lo(BytePos(up_to_if.lo().0 + 1)))
99102
{
100103
// Prevent "elseif"
101104
// Check that the "else" is followed by whitespace
@@ -141,7 +144,7 @@ impl CollapsibleIf {
141144
let then_open_bracket = then.span.split_at(1).0.with_leading_whitespace(cx).into_span();
142145
let then_closing_bracket = {
143146
let end = then.span.shrink_to_hi();
144-
end.with_lo(end.lo() - rustc_span::BytePos(1))
147+
end.with_lo(end.lo() - BytePos(1))
145148
.with_leading_whitespace(cx)
146149
.into_span()
147150
};

clippy_utils/src/lib.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ use rustc_span::hygiene::{ExpnKind, MacroKind};
122122
use rustc_span::source_map::SourceMap;
123123
use rustc_span::symbol::{Ident, Symbol, kw};
124124
use rustc_span::{InnerSpan, Span};
125-
use source::walk_span_to_context;
125+
use source::{SpanRangeExt, walk_span_to_context};
126126
use visitors::{Visitable, for_each_unconsumed_temporary};
127127

128128
use crate::consts::{ConstEvalCtxt, Constant, mir_to_const};
@@ -2788,6 +2788,16 @@ pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool {
27882788
});
27892789
}
27902790

2791+
/// Checks whether a given span has any non-comment token. This checks for all types of token other
2792+
/// than line comment "//", block comment "/**", doc "///" "//!" and whitespace
2793+
/// This is useful to determine if there are any actual code tokens in the span that are omitted in
2794+
/// the late pass, such as platform-specific code.
2795+
pub fn span_contains_non_comment(cx: &impl source::HasSession, span: Span) -> bool {
2796+
matches!(span.get_source_text(cx), Some(snippet) if tokenize_with_text(&snippet).any(|(token, _, _)| {
2797+
!matches!(token, TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace)
2798+
}))
2799+
}
2800+
27912801
/// Returns all the comments a given span contains
27922802
///
27932803
/// Comments are returned wrapped with their relevant delimiters

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)