Skip to content

Commit

Permalink
Fix match_overlapping_arm false negative
Browse files Browse the repository at this point in the history
Fixes #7816
  • Loading branch information
Michael Wright committed Nov 1, 2021
1 parent 310ecb0 commit c3d4577
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 25 deletions.
46 changes: 22 additions & 24 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -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),
Expand All @@ -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) {
Expand All @@ -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()));
},
}
}
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/match_overlapping_arm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => (),
Expand Down
14 changes: 13 additions & 1 deletion tests/ui/match_overlapping_arm.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit c3d4577

Please sign in to comment.