about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMichael Wright <mikerite@lavabit.com>2021-11-01 06:12:43 +0200
committerMichael Wright <mikerite@lavabit.com>2021-11-01 06:12:43 +0200
commitc3d45775c438c57450317ccc859d3af10ce08972 (patch)
tree8c7c2634e3adee255148ed7154fb9a831a34dc50
parent310ecb0a5c0d9161679c256052822f34c6eb548a (diff)
downloadrust-c3d45775c438c57450317ccc859d3af10ce08972.tar.gz
rust-c3d45775c438c57450317ccc859d3af10ce08972.zip
Fix `match_overlapping_arm` false negative
Fixes #7816
-rw-r--r--clippy_lints/src/matches.rs46
-rw-r--r--tests/ui/match_overlapping_arm.rs9
-rw-r--r--tests/ui/match_overlapping_arm.stderr14
3 files changed, 44 insertions, 25 deletions
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index f1289a36e77..3511defc59c 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -34,7 +34,6 @@ use rustc_span::source_map::{Span, Spanned};
 use rustc_span::sym;
 use std::cmp::Ordering;
 use std::collections::hash_map::Entry;
-use std::iter;
 use std::ops::Bound;
 
 declare_clippy_lint! {
@@ -1703,12 +1702,6 @@ where
     }
 
     impl<'a, T: Copy> Kind<'a, T> {
-        fn range(&self) -> &'a SpannedRange<T> {
-            match *self {
-                Kind::Start(_, r) | Kind::End(_, r) => r,
-            }
-        }
-
         fn value(self) -> Bound<T> {
             match self {
                 Kind::Start(t, _) => Bound::Included(t),
@@ -1726,7 +1719,19 @@ where
     impl<'a, T: Copy + Ord> Ord for Kind<'a, T> {
         fn cmp(&self, other: &Self) -> Ordering {
             match (self.value(), other.value()) {
-                (Bound::Included(a), Bound::Included(b)) | (Bound::Excluded(a), Bound::Excluded(b)) => a.cmp(&b),
+                (Bound::Included(a), Bound::Included(b)) | (Bound::Excluded(a), Bound::Excluded(b)) => {
+                    let value_cmp = a.cmp(&b);
+                    // In the case of ties, starts come before ends
+                    if value_cmp == Ordering::Equal {
+                        match (self, other) {
+                            (Kind::Start(..), Kind::End(..)) => Ordering::Less,
+                            (Kind::End(..), Kind::Start(..)) => Ordering::Greater,
+                            _ => Ordering::Equal,
+                        }
+                    } else {
+                        value_cmp
+                    }
+                },
                 // Range patterns cannot be unbounded (yet)
                 (Bound::Unbounded, _) | (_, Bound::Unbounded) => unimplemented!(),
                 (Bound::Included(a), Bound::Excluded(b)) => match a.cmp(&b) {
@@ -1750,24 +1755,17 @@ where
 
     values.sort();
 
-    for (a, b) in iter::zip(&values, values.iter().skip(1)) {
-        match (a, b) {
-            (&Kind::Start(_, ra), &Kind::End(_, rb)) => {
-                if ra.node != rb.node {
-                    return Some((ra, rb));
-                }
-            },
-            (&Kind::End(a, _), &Kind::Start(b, _)) if a != Bound::Included(b) => (),
-            _ => {
-                // skip if the range `a` is completely included into the range `b`
-                if let Ordering::Equal | Ordering::Less = a.cmp(b) {
-                    let kind_a = Kind::End(a.range().node.1, a.range());
-                    let kind_b = Kind::End(b.range().node.1, b.range());
-                    if let Ordering::Equal | Ordering::Greater = kind_a.cmp(&kind_b) {
-                        return None;
+    let mut started = vec![];
+
+    for value in values {
+        match value {
+            Kind::Start(_, r) => started.push(r),
+            Kind::End(_, er) => {
+                if let Some(sr) = started.pop() {
+                    if sr != er {
+                        return Some((er, sr));
                     }
                 }
-                return Some((a.range(), b.range()));
             },
         }
     }
diff --git a/tests/ui/match_overlapping_arm.rs b/tests/ui/match_overlapping_arm.rs
index 845986a4ead..c25f59c62a5 100644
--- a/tests/ui/match_overlapping_arm.rs
+++ b/tests/ui/match_overlapping_arm.rs
@@ -100,6 +100,15 @@ fn overlapping() {
         _ => (),
     }
 
+    // Issue #7816 - overlap after included range
+    match 42 {
+        5..=10 => (),
+        0..=20 => (),
+        21..=30 => (),
+        21..=40 => (),
+        _ => (),
+    }
+
     // Issue #7829
     match 0 {
         -1..=1 => (),
diff --git a/tests/ui/match_overlapping_arm.stderr b/tests/ui/match_overlapping_arm.stderr
index c2b3f173c2b..69f794edb6a 100644
--- a/tests/ui/match_overlapping_arm.stderr
+++ b/tests/ui/match_overlapping_arm.stderr
@@ -71,5 +71,17 @@ note: overlaps with this
 LL |         ..26 => println!("..26"),
    |         ^^^^
 
-error: aborting due to 6 previous errors
+error: some ranges overlap
+  --> $DIR/match_overlapping_arm.rs:107:9
+   |
+LL |         21..=30 => (),
+   |         ^^^^^^^
+   |
+note: overlaps with this
+  --> $DIR/match_overlapping_arm.rs:108:9
+   |
+LL |         21..=40 => (),
+   |         ^^^^^^^
+
+error: aborting due to 7 previous errors