Auto merge of #7909 - mikerite:fix-7816, r=camsteffen

Fix false negative in [`match_overlapping_arms`]

changelog: Fix false negative in [`match_overlapping_arms`]

Author: bors

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! {
@@ -1707,12 +1706,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),
@@ -1730,7 +1723,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) {
@@ -1754,24 +1759,24 @@ 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) => {
+                let mut overlap = None;
+
+                while let Some(sr) = started.pop() {
+                    if sr == er {
+                        break;
                     }
+                    overlap = Some(sr);
+                }
+
+                if let Some(sr) = overlap {
+                    return Some((er, sr));
                 }
-                return Some((a.range(), b.range()));
             },
         }
     }

tests/ui/match_overlapping_arm.rs

@@ -100,13 +100,31 @@ fn overlapping() {
         _ => (),
     }
 
+    // Issue #7816 - overlap after included range
+    match 42 {
+        5..=10 => (),
+        0..=20 => (),
+        21..=30 => (),
+        21..=40 => (),
+        _ => (),
+    }
+
     // Issue #7829
     match 0 {
         -1..=1 => (),
         -2..=2 => (),
         _ => (),
     }
 
+    // Only warn about the first if there are multiple overlaps
+    match 42u128 {
+        0..=0x0000_0000_0000_00ff => (),
+        0..=0x0000_0000_0000_ffff => (),
+        0..=0x0000_0000_ffff_ffff => (),
+        0..=0xffff_ffff_ffff_ffff => (),
+        _ => (),
+    }
+
     if let None = Some(42) {
         // nothing
     } else if let None = Some(42) {

tests/ui/match_overlapping_arm.stderr

@@ -71,5 +71,29 @@ 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: some ranges overlap
+  --> $DIR/match_overlapping_arm.rs:121:9
+   |
+LL |         0..=0x0000_0000_0000_00ff => (),
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: overlaps with this
+  --> $DIR/match_overlapping_arm.rs:122:9
+   |
+LL |         0..=0x0000_0000_0000_ffff => (),
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 8 previous errors