about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJason Newcomb <jsnewcomb@pm.me>2022-02-17 13:43:59 -0500
committerJason Newcomb <jsnewcomb@pm.me>2022-02-17 14:04:10 -0500
commit8ce2d46cacc8b91f1d252d52cb38f397ee990435 (patch)
tree8d53186846f9cd2cc8d82e912822c5e73da77a58
parent9bfcbf4f027fea5096e6a721b1b73fbf057e14cf (diff)
downloadrust-8ce2d46cacc8b91f1d252d52cb38f397ee990435.tar.gz
rust-8ce2d46cacc8b91f1d252d52cb38f397ee990435.zip
Check for `cfg` attrubutes before linting `match` expressions
-rw-r--r--clippy_lints/src/matches/mod.rs119
-rw-r--r--tests/ui/match_as_ref.fixed10
-rw-r--r--tests/ui/match_as_ref.rs10
-rw-r--r--tests/ui/match_bool.rs8
-rw-r--r--tests/ui/match_expr_like_matches_macro.fixed15
-rw-r--r--tests/ui/match_expr_like_matches_macro.rs15
-rw-r--r--tests/ui/match_same_arms2.rs10
-rw-r--r--tests/ui/single_match.rs8
8 files changed, 171 insertions, 24 deletions
diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs
index 614a824010f..14dd7cb1dac 100644
--- a/clippy_lints/src/matches/mod.rs
+++ b/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;
@@ -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);
@@ -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
+}
diff --git a/tests/ui/match_as_ref.fixed b/tests/ui/match_as_ref.fixed
index c61eb921664..ddfa1e741ad 100644
--- a/tests/ui/match_as_ref.fixed
+++ b/tests/ui/match_as_ref.fixed
@@ -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,
+    };
+}
diff --git a/tests/ui/match_as_ref.rs b/tests/ui/match_as_ref.rs
index 2fbd0b255fa..025d475ae13 100644
--- a/tests/ui/match_as_ref.rs
+++ b/tests/ui/match_as_ref.rs
@@ -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,
+    };
+}
diff --git a/tests/ui/match_bool.rs b/tests/ui/match_bool.rs
index 9ed55ca7ae7..bcc999a4942 100644
--- a/tests/ui/match_bool.rs
+++ b/tests/ui/match_bool.rs
@@ -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() {}
diff --git a/tests/ui/match_expr_like_matches_macro.fixed b/tests/ui/match_expr_like_matches_macro.fixed
index c611f76bf96..36f233f3346 100644
--- a/tests/ui/match_expr_like_matches_macro.fixed
+++ b/tests/ui/match_expr_like_matches_macro.fixed
@@ -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,
+        };
+    }
 }
diff --git a/tests/ui/match_expr_like_matches_macro.rs b/tests/ui/match_expr_like_matches_macro.rs
index 2deeb84e741..750f69fa508 100644
--- a/tests/ui/match_expr_like_matches_macro.rs
+++ b/tests/ui/match_expr_like_matches_macro.rs
@@ -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,
+        };
+    }
 }
diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs
index da4e3020d5b..67e1d518483 100644
--- a/tests/ui/match_same_arms2.rs
+++ b/tests/ui/match_same_arms2.rs
@@ -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,
+    };
+}
diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs
index bd371888046..dd148edf529 100644
--- a/tests/ui/single_match.rs
+++ b/tests/ui/single_match.rs
@@ -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,
+    };
 }