Skip to content

Commit 8ce2d46

Browse files
committed
Check for cfg attrubutes before linting match expressions
1 parent 9bfcbf4 commit 8ce2d46

8 files changed

+171
-24
lines changed

clippy_lints/src/matches/mod.rs

+98-21
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
use clippy_utils::source::{snippet_opt, walk_span_to_context};
12
use clippy_utils::{meets_msrv, msrvs};
2-
use rustc_hir::{Expr, ExprKind, Local, MatchSource, Pat};
3+
use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat};
4+
use rustc_lexer::{tokenize, TokenKind};
35
use rustc_lint::{LateContext, LateLintPass};
46
use rustc_semver::RustcVersion;
57
use rustc_session::{declare_tool_lint, impl_lint_pass};
8+
use rustc_span::{Span, SpanData, SyntaxContext};
69

710
mod infalliable_detructuring_match;
811
mod match_as_ref;
@@ -605,29 +608,33 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
605608
}
606609

607610
if let ExprKind::Match(ex, arms, source) = expr.kind {
608-
if source == MatchSource::Normal {
609-
if !(meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO)
610-
&& match_like_matches::check_match(cx, expr, ex, arms))
611-
{
612-
match_same_arms::check(cx, arms);
613-
}
611+
if !contains_cfg_arm(cx, expr, ex, arms) {
612+
if source == MatchSource::Normal {
613+
if !(meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO)
614+
&& match_like_matches::check_match(cx, expr, ex, arms))
615+
{
616+
match_same_arms::check(cx, arms);
617+
}
618+
619+
redundant_pattern_match::check_match(cx, expr, ex, arms);
620+
single_match::check(cx, ex, arms, expr);
621+
match_bool::check(cx, ex, arms, expr);
622+
overlapping_arms::check(cx, ex, arms);
623+
match_wild_enum::check(cx, ex, arms);
624+
match_as_ref::check(cx, ex, arms, expr);
614625

615-
redundant_pattern_match::check_match(cx, expr, ex, arms);
616-
single_match::check(cx, ex, arms, expr);
617-
match_bool::check(cx, ex, arms, expr);
618-
overlapping_arms::check(cx, ex, arms);
619-
match_wild_err_arm::check(cx, ex, arms);
620-
match_wild_enum::check(cx, ex, arms);
621-
match_as_ref::check(cx, ex, arms, expr);
622-
wild_in_or_pats::check(cx, arms);
623-
624-
if self.infallible_destructuring_match_linted {
625-
self.infallible_destructuring_match_linted = false;
626-
} else {
627-
match_single_binding::check(cx, ex, arms, expr);
626+
if self.infallible_destructuring_match_linted {
627+
self.infallible_destructuring_match_linted = false;
628+
} else {
629+
match_single_binding::check(cx, ex, arms, expr);
630+
}
628631
}
632+
match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
629633
}
630-
match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
634+
635+
// These don't depend on a relationship between multiple arms
636+
match_wild_err_arm::check(cx, ex, arms);
637+
wild_in_or_pats::check(cx, arms);
631638
} else {
632639
if meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO) {
633640
match_like_matches::check(cx, expr);
@@ -646,3 +653,73 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
646653

647654
extract_msrv_attr!(LateContext);
648655
}
656+
657+
/// Checks if there are any arms with a `#[cfg(..)]` attribute.
658+
fn contains_cfg_arm(cx: &LateContext<'_>, e: &Expr<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]) -> bool {
659+
let Some(scrutinee_span) = walk_span_to_context(scrutinee.span, SyntaxContext::root()) else {
660+
// Shouldn't happen, but treat this as though a `cfg` attribute were found
661+
return true;
662+
};
663+
664+
let start = scrutinee_span.hi();
665+
let mut arm_spans = arms.iter().map(|arm| {
666+
let data = arm.span.data();
667+
(data.ctxt == SyntaxContext::root()).then(|| (data.lo, data.hi))
668+
});
669+
let end = e.span.hi();
670+
671+
let found = arm_spans.try_fold(start, |start, range| {
672+
let Some((end, next_start)) = range else {
673+
// Shouldn't happen, but treat this as though a `cfg` attribute were found
674+
return Err(());
675+
};
676+
let span = SpanData {
677+
lo: start,
678+
hi: end,
679+
ctxt: SyntaxContext::root(),
680+
parent: None,
681+
}
682+
.span();
683+
(!span_contains_cfg(cx, span)).then(|| next_start).ok_or(())
684+
});
685+
match found {
686+
Ok(start) => {
687+
let span = SpanData {
688+
lo: start,
689+
hi: end,
690+
ctxt: SyntaxContext::root(),
691+
parent: None,
692+
}
693+
.span();
694+
span_contains_cfg(cx, span)
695+
},
696+
Err(()) => true,
697+
}
698+
}
699+
700+
fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool {
701+
let Some(snip) = snippet_opt(cx, s) else {
702+
// Assume true. This would require either an invalid span, or one which crosses file boundaries.
703+
return true;
704+
};
705+
let mut pos = 0usize;
706+
let mut iter = tokenize(&snip).map(|t| {
707+
let start = pos;
708+
pos += t.len;
709+
(t.kind, start..pos)
710+
});
711+
while iter.any(|(t, _)| matches!(t, TokenKind::Pound)) {
712+
let mut iter = iter.by_ref().skip_while(|(t, _)| {
713+
matches!(
714+
t,
715+
TokenKind::Whitespace | TokenKind::LineComment { .. } | TokenKind::BlockComment { .. }
716+
)
717+
});
718+
if matches!(iter.next(), Some((TokenKind::OpenBracket, _)))
719+
&& matches!(iter.next(), Some((TokenKind::Ident, range)) if &snip[range.clone()] == "cfg")
720+
{
721+
return true;
722+
}
723+
}
724+
false
725+
}

tests/ui/match_as_ref.fixed

+9-1
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,12 @@ mod issue4437 {
3232
}
3333
}
3434

35-
fn main() {}
35+
fn main() {
36+
// Don't lint
37+
let _ = match Some(0) {
38+
#[cfg(feature = "foo")]
39+
Some(ref x) if *x > 50 => None,
40+
Some(ref x) => Some(x),
41+
None => None,
42+
};
43+
}

tests/ui/match_as_ref.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,12 @@ mod issue4437 {
4141
}
4242
}
4343

44-
fn main() {}
44+
fn main() {
45+
// Don't lint
46+
let _ = match Some(0) {
47+
#[cfg(feature = "foo")]
48+
Some(ref x) if *x > 50 => None,
49+
Some(ref x) => Some(x),
50+
None => None,
51+
};
52+
}

tests/ui/match_bool.rs

+8
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ fn match_bool() {
5050
11..=20 => 2,
5151
_ => 3,
5252
};
53+
54+
// Don't lint
55+
let _ = match test {
56+
#[cfg(feature = "foo")]
57+
true if option == 5 => 10,
58+
true => 0,
59+
false => 1,
60+
};
5361
}
5462

5563
fn main() {}

tests/ui/match_expr_like_matches_macro.fixed

+15
Original file line numberDiff line numberDiff line change
@@ -146,4 +146,19 @@ fn main() {
146146
let _res = matches!(&val, &Some(ref _a));
147147
fun(val);
148148
}
149+
150+
{
151+
enum E {
152+
A,
153+
B,
154+
C,
155+
}
156+
157+
let _ = match E::A {
158+
E::B => true,
159+
#[cfg(feature = "foo")]
160+
E::A => true,
161+
_ => false,
162+
};
163+
}
149164
}

tests/ui/match_expr_like_matches_macro.rs

+15
Original file line numberDiff line numberDiff line change
@@ -181,4 +181,19 @@ fn main() {
181181
};
182182
fun(val);
183183
}
184+
185+
{
186+
enum E {
187+
A,
188+
B,
189+
C,
190+
}
191+
192+
let _ = match E::A {
193+
E::B => true,
194+
#[cfg(feature = "foo")]
195+
E::A => true,
196+
_ => false,
197+
};
198+
}
184199
}

tests/ui/match_same_arms2.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,12 @@ fn match_expr_like_matches_macro_priority() {
166166
};
167167
}
168168

169-
fn main() {}
169+
fn main() {
170+
let _ = match Some(0) {
171+
Some(0) => 0,
172+
Some(1) => 1,
173+
#[cfg(feature = "foo")]
174+
Some(2) => 2,
175+
_ => 1,
176+
};
177+
}

tests/ui/single_match.rs

+8
Original file line numberDiff line numberDiff line change
@@ -234,4 +234,12 @@ macro_rules! single_match {
234234

235235
fn main() {
236236
single_match!(5);
237+
238+
// Don't lint
239+
let _ = match Some(0) {
240+
#[cfg(feature = "foo")]
241+
Some(10) => 11,
242+
Some(x) => x,
243+
_ => 0,
244+
};
237245
}

0 commit comments

Comments
 (0)