Skip to content

Commit

Permalink
Check for cfg attrubutes before linting match expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarcho committed Feb 17, 2022
1 parent 9bfcbf4 commit 8ce2d46
Show file tree
Hide file tree
Showing 8 changed files with 171 additions and 24 deletions.
119 changes: 98 additions & 21 deletions clippy_lints/src/matches/mod.rs
@@ -1,8 +1,11 @@
use clippy_utils::source::{snippet_opt, walk_span_to_context};
use clippy_utils::{meets_msrv, msrvs};
use rustc_hir::{Expr, ExprKind, Local, MatchSource, Pat};
use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat};
use rustc_lexer::{tokenize, TokenKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{Span, SpanData, SyntaxContext};

mod infalliable_detructuring_match;
mod match_as_ref;
Expand Down Expand Up @@ -605,29 +608,33 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
}

if let ExprKind::Match(ex, arms, source) = expr.kind {
if source == MatchSource::Normal {
if !(meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO)
&& match_like_matches::check_match(cx, expr, ex, arms))
{
match_same_arms::check(cx, arms);
}
if !contains_cfg_arm(cx, expr, ex, arms) {
if source == MatchSource::Normal {
if !(meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO)
&& match_like_matches::check_match(cx, expr, ex, arms))
{
match_same_arms::check(cx, arms);
}

redundant_pattern_match::check_match(cx, expr, ex, arms);
single_match::check(cx, ex, arms, expr);
match_bool::check(cx, ex, arms, expr);
overlapping_arms::check(cx, ex, arms);
match_wild_enum::check(cx, ex, arms);
match_as_ref::check(cx, ex, arms, expr);

redundant_pattern_match::check_match(cx, expr, ex, arms);
single_match::check(cx, ex, arms, expr);
match_bool::check(cx, ex, arms, expr);
overlapping_arms::check(cx, ex, arms);
match_wild_err_arm::check(cx, ex, arms);
match_wild_enum::check(cx, ex, arms);
match_as_ref::check(cx, ex, arms, expr);
wild_in_or_pats::check(cx, arms);

if self.infallible_destructuring_match_linted {
self.infallible_destructuring_match_linted = false;
} else {
match_single_binding::check(cx, ex, arms, expr);
if self.infallible_destructuring_match_linted {
self.infallible_destructuring_match_linted = false;
} else {
match_single_binding::check(cx, ex, arms, expr);
}
}
match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
}
match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);

// These don't depend on a relationship between multiple arms
match_wild_err_arm::check(cx, ex, arms);
wild_in_or_pats::check(cx, arms);
} else {
if meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO) {
match_like_matches::check(cx, expr);
Expand All @@ -646,3 +653,73 @@ impl<'tcx> LateLintPass<'tcx> for Matches {

extract_msrv_attr!(LateContext);
}

/// Checks if there are any arms with a `#[cfg(..)]` attribute.
fn contains_cfg_arm(cx: &LateContext<'_>, e: &Expr<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]) -> bool {
let Some(scrutinee_span) = walk_span_to_context(scrutinee.span, SyntaxContext::root()) else {
// Shouldn't happen, but treat this as though a `cfg` attribute were found
return true;
};

let start = scrutinee_span.hi();
let mut arm_spans = arms.iter().map(|arm| {
let data = arm.span.data();
(data.ctxt == SyntaxContext::root()).then(|| (data.lo, data.hi))
});
let end = e.span.hi();

let found = arm_spans.try_fold(start, |start, range| {
let Some((end, next_start)) = range else {
// Shouldn't happen, but treat this as though a `cfg` attribute were found
return Err(());
};
let span = SpanData {
lo: start,
hi: end,
ctxt: SyntaxContext::root(),
parent: None,
}
.span();
(!span_contains_cfg(cx, span)).then(|| next_start).ok_or(())
});
match found {
Ok(start) => {
let span = SpanData {
lo: start,
hi: end,
ctxt: SyntaxContext::root(),
parent: None,
}
.span();
span_contains_cfg(cx, span)
},
Err(()) => true,
}
}

fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool {
let Some(snip) = snippet_opt(cx, s) else {
// Assume true. This would require either an invalid span, or one which crosses file boundaries.
return true;
};
let mut pos = 0usize;
let mut iter = tokenize(&snip).map(|t| {
let start = pos;
pos += t.len;
(t.kind, start..pos)
});
while iter.any(|(t, _)| matches!(t, TokenKind::Pound)) {
let mut iter = iter.by_ref().skip_while(|(t, _)| {
matches!(
t,
TokenKind::Whitespace | TokenKind::LineComment { .. } | TokenKind::BlockComment { .. }
)
});
if matches!(iter.next(), Some((TokenKind::OpenBracket, _)))
&& matches!(iter.next(), Some((TokenKind::Ident, range)) if &snip[range.clone()] == "cfg")
{
return true;
}
}
false
}
10 changes: 9 additions & 1 deletion tests/ui/match_as_ref.fixed
Expand Up @@ -32,4 +32,12 @@ mod issue4437 {
}
}

fn main() {}
fn main() {
// Don't lint
let _ = match Some(0) {
#[cfg(feature = "foo")]
Some(ref x) if *x > 50 => None,
Some(ref x) => Some(x),
None => None,
};
}
10 changes: 9 additions & 1 deletion tests/ui/match_as_ref.rs
Expand Up @@ -41,4 +41,12 @@ mod issue4437 {
}
}

fn main() {}
fn main() {
// Don't lint
let _ = match Some(0) {
#[cfg(feature = "foo")]
Some(ref x) if *x > 50 => None,
Some(ref x) => Some(x),
None => None,
};
}
8 changes: 8 additions & 0 deletions tests/ui/match_bool.rs
Expand Up @@ -50,6 +50,14 @@ fn match_bool() {
11..=20 => 2,
_ => 3,
};

// Don't lint
let _ = match test {
#[cfg(feature = "foo")]
true if option == 5 => 10,
true => 0,
false => 1,
};
}

fn main() {}
15 changes: 15 additions & 0 deletions tests/ui/match_expr_like_matches_macro.fixed
Expand Up @@ -146,4 +146,19 @@ fn main() {
let _res = matches!(&val, &Some(ref _a));
fun(val);
}

{
enum E {
A,
B,
C,
}

let _ = match E::A {
E::B => true,
#[cfg(feature = "foo")]
E::A => true,
_ => false,
};
}
}
15 changes: 15 additions & 0 deletions tests/ui/match_expr_like_matches_macro.rs
Expand Up @@ -181,4 +181,19 @@ fn main() {
};
fun(val);
}

{
enum E {
A,
B,
C,
}

let _ = match E::A {
E::B => true,
#[cfg(feature = "foo")]
E::A => true,
_ => false,
};
}
}
10 changes: 9 additions & 1 deletion tests/ui/match_same_arms2.rs
Expand Up @@ -166,4 +166,12 @@ fn match_expr_like_matches_macro_priority() {
};
}

fn main() {}
fn main() {
let _ = match Some(0) {
Some(0) => 0,
Some(1) => 1,
#[cfg(feature = "foo")]
Some(2) => 2,
_ => 1,
};
}
8 changes: 8 additions & 0 deletions tests/ui/single_match.rs
Expand Up @@ -234,4 +234,12 @@ macro_rules! single_match {

fn main() {
single_match!(5);

// Don't lint
let _ = match Some(0) {
#[cfg(feature = "foo")]
Some(10) => 11,
Some(x) => x,
_ => 0,
};
}

0 comments on commit 8ce2d46

Please sign in to comment.