Apply review comments

5 files changed, 95 insertions, 37 deletions

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index 5aed9770d2e..fca8e1446a8 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -1,4 +1,4 @@
-use crate::consts::{constant, Constant};
+use crate::consts::{constant, miri_to_const, Constant};
 use crate::utils::paths;
 use crate::utils::sugg::Sugg;
 use crate::utils::{
@@ -444,7 +444,7 @@ fn check_match_bool(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], e
 
 fn check_overlapping_arms<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ex: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) {
     if arms.len() >= 2 && cx.tables.expr_ty(ex).is_integral() {
-        let ranges = all_ranges(cx, arms);
+        let ranges = all_ranges(cx, arms, cx.tables.expr_ty(ex));
         let type_ranges = type_ranges(&ranges);
         if !type_ranges.is_empty() {
             if let Some((start, end)) = overlapping(&type_ranges) {
@@ -705,25 +705,52 @@ fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) {
 }
 
 /// Gets all arms that are unbounded `PatRange`s.
-fn all_ranges<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm<'_>]) -> Vec<SpannedRange<Constant>> {
+fn all_ranges<'a, 'tcx>(
+    cx: &LateContext<'a, 'tcx>,
+    arms: &'tcx [Arm<'_>],
+    ty: Ty<'tcx>,
+) -> Vec<SpannedRange<Constant>> {
     arms.iter()
         .flat_map(|arm| {
             if let Arm {
                 ref pat, guard: None, ..
             } = *arm
             {
-                if let PatKind::Range(ref lhs, ref rhs, ref range_end) = pat.kind {
-                    if let (Some(l), Some(r)) = (lhs, rhs) {
-                        let lhs = constant(cx, cx.tables, l)?.0;
-                        let rhs = constant(cx, cx.tables, r)?.0;
-                        let rhs = match *range_end {
-                            RangeEnd::Included => Bound::Included(rhs),
-                            RangeEnd::Excluded => Bound::Excluded(rhs),
-                        };
-                        return Some(SpannedRange {
-                            span: pat.span,
-                            node: (lhs, rhs),
-                        });
+                if let PatKind::Range(ref lhs, ref rhs, range_end) = pat.kind {
+                    match (lhs, rhs) {
+                        (Some(lhs), Some(rhs)) => {
+                            let lhs = constant(cx, cx.tables, lhs)?.0;
+                            let rhs = constant(cx, cx.tables, rhs)?.0;
+                            let rhs = match range_end {
+                                RangeEnd::Included => Bound::Included(rhs),
+                                RangeEnd::Excluded => Bound::Excluded(rhs),
+                            };
+                            return Some(SpannedRange {
+                                span: pat.span,
+                                node: (lhs, rhs),
+                            });
+                        },
+                        (None, Some(rhs)) => {
+                            let lhs = miri_to_const(ty.numeric_min_val(cx.tcx)?)?;
+                            let rhs = constant(cx, cx.tables, rhs)?.0;
+                            let rhs = match range_end {
+                                RangeEnd::Included => Bound::Included(rhs),
+                                RangeEnd::Excluded => Bound::Excluded(rhs),
+                            };
+                            return Some(SpannedRange {
+                                span: pat.span,
+                                node: (lhs, rhs),
+                            });
+                        },
+                        (Some(lhs), None) => {
+                            let lhs = constant(cx, cx.tables, lhs)?.0;
+                            let rhs = miri_to_const(ty.numeric_max_val(cx.tcx)?)?;
+                            return Some(SpannedRange {
+                                span: pat.span,
+                                node: (lhs, Bound::Excluded(rhs)),
+                            });
+                        },
+                        _ => return None,
                     }
                 }
 
diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs
index b9f11f79a37..cfbb32806da 100644
--- a/clippy_lints/src/utils/author.rs
+++ b/clippy_lints/src/utils/author.rs
@@ -12,6 +12,7 @@ use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
 use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Pat, PatKind, QPath, Stmt, StmtKind, TyKind};
 use rustc_session::declare_tool_lint;
 use syntax::ast::{Attribute, LitFloatType, LitKind};
+use syntax::walk_list;
 
 declare_clippy_lint! {
     /// **What it does:** Generates clippy code that detects the offending pattern
@@ -617,13 +618,9 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
                     start_pat, end_pat, end_kind, current
                 );
                 self.current = start_pat;
-                if let Some(expr) = start {
-                    self.visit_expr(expr);
-                }
+                walk_list!(self, visit_expr, start);
                 self.current = end_pat;
-                if let Some(expr) = end {
-                    self.visit_expr(expr);
-                }
+                walk_list!(self, visit_expr, end);
             },
             PatKind::Slice(ref start, ref middle, ref end) => {
                 let start_pat = self.next("start");
diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs
index 00baad72e6d..01f719f4ab8 100644
--- a/clippy_lints/src/utils/hir_utils.rs
+++ b/clippy_lints/src/utils/hir_utils.rs
@@ -194,11 +194,8 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
             (&PatKind::Tuple(ref l, ls), &PatKind::Tuple(ref r, rs)) => {
                 ls == rs && over(l, r, |l, r| self.eq_pat(l, r))
             },
-            (&PatKind::Range(ref ls, ref le, ref li), &PatKind::Range(ref rs, ref re, ref ri)) => {
-                if let (Some(ls), Some(rs), Some(le), Some(re)) = (ls, rs, le, re) {
-                    return self.eq_expr(ls, rs) && self.eq_expr(le, re) && (*li == *ri);
-                }
-                false
+            (&PatKind::Range(ref ls, ref le, li), &PatKind::Range(ref rs, ref re, ri)) => {
+                both(ls, rs, |a, b| self.eq_expr(a, b)) && both(le, re, |a, b| self.eq_expr(a, b)) && (li == ri)
             },
             (&PatKind::Ref(ref le, ref lm), &PatKind::Ref(ref re, ref rm)) => lm == rm && self.eq_pat(le, re),
             (&PatKind::Slice(ref ls, ref li, ref le), &PatKind::Slice(ref rs, ref ri, ref re)) => {
diff --git a/tests/ui/match_overlapping_arm.rs b/tests/ui/match_overlapping_arm.rs
index a48de440652..ce7761119cb 100644
--- a/tests/ui/match_overlapping_arm.rs
+++ b/tests/ui/match_overlapping_arm.rs
@@ -1,4 +1,5 @@
 #![feature(exclusive_range_pattern)]
+#![feature(half_open_range_patterns)]
 #![warn(clippy::match_overlapping_arm)]
 #![allow(clippy::redundant_pattern_matching)]
 
@@ -56,6 +57,18 @@ fn overlapping() {
         _ => (),
     }
 
+    match 42 {
+        0.. => println!("0 .. 42"),
+        3.. => println!("3 .. 42"),
+        _ => (),
+    }
+
+    match 42 {
+        ..=23 => println!("0 ... 23"),
+        ..26 => println!("0 .. 26"),
+        _ => (),
+    }
+
     if let None = Some(42) {
         // nothing
     } else if let None = Some(42) {
diff --git a/tests/ui/match_overlapping_arm.stderr b/tests/ui/match_overlapping_arm.stderr
index dcd42fca7b4..32dcefa86d8 100644
--- a/tests/ui/match_overlapping_arm.stderr
+++ b/tests/ui/match_overlapping_arm.stderr
@@ -1,63 +1,87 @@
 error: some ranges overlap
-  --> $DIR/match_overlapping_arm.rs:11:9
+  --> $DIR/match_overlapping_arm.rs:12:9
    |
 LL |         0..=10 => println!("0 ... 10"),
    |         ^^^^^^
    |
    = note: `-D clippy::match-overlapping-arm` implied by `-D warnings`
 note: overlaps with this
-  --> $DIR/match_overlapping_arm.rs:12:9
+  --> $DIR/match_overlapping_arm.rs:13:9
    |
 LL |         0..=11 => println!("0 ... 11"),
    |         ^^^^^^
 
 error: some ranges overlap
-  --> $DIR/match_overlapping_arm.rs:17:9
+  --> $DIR/match_overlapping_arm.rs:18:9
    |
 LL |         0..=5 => println!("0 ... 5"),
    |         ^^^^^
    |
 note: overlaps with this
-  --> $DIR/match_overlapping_arm.rs:19:9
+  --> $DIR/match_overlapping_arm.rs:20:9
    |
 LL |         FOO..=11 => println!("0 ... 11"),
    |         ^^^^^^^^
 
 error: some ranges overlap
-  --> $DIR/match_overlapping_arm.rs:25:9
+  --> $DIR/match_overlapping_arm.rs:26:9
    |
 LL |         0..=5 => println!("0 ... 5"),
    |         ^^^^^
    |
 note: overlaps with this
-  --> $DIR/match_overlapping_arm.rs:24:9
+  --> $DIR/match_overlapping_arm.rs:25:9
    |
 LL |         2 => println!("2"),
    |         ^
 
 error: some ranges overlap
-  --> $DIR/match_overlapping_arm.rs:31:9
+  --> $DIR/match_overlapping_arm.rs:32:9
    |
 LL |         0..=2 => println!("0 ... 2"),
    |         ^^^^^
    |
 note: overlaps with this
-  --> $DIR/match_overlapping_arm.rs:30:9
+  --> $DIR/match_overlapping_arm.rs:31:9
    |
 LL |         2 => println!("2"),
    |         ^
 
 error: some ranges overlap
-  --> $DIR/match_overlapping_arm.rs:54:9
+  --> $DIR/match_overlapping_arm.rs:55:9
    |
 LL |         0..11 => println!("0 .. 11"),
    |         ^^^^^
    |
 note: overlaps with this
-  --> $DIR/match_overlapping_arm.rs:55:9
+  --> $DIR/match_overlapping_arm.rs:56:9
    |
 LL |         0..=11 => println!("0 ... 11"),
    |         ^^^^^^
 
-error: aborting due to 5 previous errors
+error: some ranges overlap
+  --> $DIR/match_overlapping_arm.rs:61:9
+   |
+LL |         0.. => println!("0 .. 42"),
+   |         ^^^
+   |
+note: overlaps with this
+  --> $DIR/match_overlapping_arm.rs:62:9
+   |
+LL |         3.. => println!("3 .. 42"),
+   |         ^^^
+
+error: some ranges overlap
+  --> $DIR/match_overlapping_arm.rs:67:9
+   |
+LL |         ..=23 => println!("0 ... 23"),
+   |         ^^^^^
+   |
+note: overlaps with this
+  --> $DIR/match_overlapping_arm.rs:68:9
+   |
+LL |         ..26 => println!("0 .. 26"),
+   |         ^^^^
+
+error: aborting due to 7 previous errors