Skip to content

Commit

Permalink
Apply review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@JohnTitor
JohnTitor committed Jan 11, 2020
1 parent 8daa278 commit 10cf141
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 37 deletions.
57 changes: 42 additions & 15 deletions clippy_lints/src/matches.rs
View file
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
}
}

Expand Down
9 changes: 3 additions & 6 deletions clippy_lints/src/utils/author.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down
7 changes: 2 additions & 5 deletions clippy_lints/src/utils/hir_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) => {
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/match_overlapping_arm.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![feature(exclusive_range_pattern)]
#![feature(half_open_range_patterns)]
#![warn(clippy::match_overlapping_arm)]
#![allow(clippy::redundant_pattern_matching)]

Expand Down Expand Up @@ -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) {
Expand Down
46 changes: 35 additions & 11 deletions tests/ui/match_overlapping_arm.stderr
View file
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 10cf141

Please sign in to comment.