Skip to content

Commit

Permalink
check for overlapping ranges
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Jun 17, 2023
1 parent a70cabe commit ab67745
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 9 deletions.
52 changes: 47 additions & 5 deletions clippy_lints/src/manual_range_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
use rustc_ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_hir::ExprKind;
use rustc_hir::PatKind;
use rustc_hir::RangeEnd;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

Expand Down Expand Up @@ -32,6 +34,16 @@ declare_clippy_lint! {
}
declare_lint_pass!(ManualRangePattern => [MANUAL_RANGE_PATTERN]);

fn expr_as_u128(expr: &Expr<'_>) -> Option<u128> {
if let ExprKind::Lit(lit) = expr.kind
&& let LitKind::Int(num, _) = lit.node
{
Some(num)
} else {
None
}
}

impl LateLintPass<'_> for ManualRangePattern {
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &'_ rustc_hir::Pat<'_>) {
if pat.span.from_expansion() {
Expand All @@ -42,25 +54,55 @@ impl LateLintPass<'_> for ManualRangePattern {
let mut min = u128::MAX;
let mut max = 0;
let mut numbers_found = FxHashSet::default();
let mut ranges_found = Vec::new();

for pat in pats {
if let PatKind::Lit(lit) = pat.kind
&& let ExprKind::Lit(lit) = lit.kind
&& let LitKind::Int(num, _) = lit.node
&& let Some(num) = expr_as_u128(lit)
{
numbers_found.insert(num);

min = min.min(num);
max = max.max(num);
} else if let PatKind::Range(Some(left), Some(right), end) = pat.kind
&& let Some(left) = expr_as_u128(left)
&& let Some(right) = expr_as_u128(right)
&& right >= left
{
min = min.min(left);
max = max.max(right);
ranges_found.push(left..=match end {
RangeEnd::Included => right,
RangeEnd::Excluded => right - 1,
});
} else {
return;
}
}

// a pattern like 1 | 2 seems fine, lint if there are at least 3 alternatives
if numbers_found.len() >= 3 {
let contains_whole_range = (max - min == numbers_found.len() as u128 - 1)
&& (min..=max).all(|num| numbers_found.contains(&num));
if pats.len() >= 3 {
let contains_whole_range = 'contains: {
let mut num = min;
while num <= max {
if numbers_found.contains(&num) {
num += 1;
}
// Given a list of (potentially overlapping) ranges like:
// 1..=5, 3..=7, 6..=10
// We want to find the range with the highest end that still contains the current number
else if let Some(range) = ranges_found
.iter()
.filter(|range| range.contains(&num))
.max_by_key(|range| range.end())
{
num = range.end() + 1;
} else {
break 'contains false;
}
}
break 'contains true;
};

if contains_whole_range {
span_lint_and_sugg(
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/manual_range_pattern.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![allow(unused)]
#![warn(clippy::manual_range_pattern)]
#![feature(exclusive_range_pattern)]

fn main() {
let f = 6;
Expand All @@ -14,6 +15,11 @@ fn main() {
let _ = matches!(f, 1 | 2147483647);
let _ = matches!(f, 0 | 2147483647);
let _ = matches!(f, -2147483647 | 2147483647);
let _ = matches!(f, 1 | (2..=4));
let _ = matches!(f, 1 | (2..4));
let _ = matches!(f, 1..=48324729);
let _ = matches!(f, 0..=48324730);
let _ = matches!(f, 0..=3);
#[allow(clippy::match_like_matches_macro)]
let _ = match f {
1..=10 => true,
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/manual_range_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![allow(unused)]
#![warn(clippy::manual_range_pattern)]
#![feature(exclusive_range_pattern)]

fn main() {
let f = 6;
Expand All @@ -14,6 +15,11 @@ fn main() {
let _ = matches!(f, 1 | 2147483647);
let _ = matches!(f, 0 | 2147483647);
let _ = matches!(f, -2147483647 | 2147483647);
let _ = matches!(f, 1 | (2..=4));
let _ = matches!(f, 1 | (2..4));
let _ = matches!(f, (1..=10) | (2..=13) | (14..=48324728) | 48324729);
let _ = matches!(f, 0 | (1..=10) | 48324730 | (2..=13) | (14..=48324728) | 48324729);
let _ = matches!(f, 0..=1 | 0..=2 | 0..=3);
#[allow(clippy::match_like_matches_macro)]
let _ = match f {
1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 => true,
Expand Down
26 changes: 22 additions & 4 deletions tests/ui/manual_range_pattern.stderr
Original file line number Diff line number Diff line change
@@ -1,22 +1,40 @@
error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_pattern.rs:9:25
--> $DIR/manual_range_pattern.rs:10:25
|
LL | let _ = matches!(f, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=10`
|
= note: `-D clippy::manual-range-pattern` implied by `-D warnings`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_pattern.rs:10:25
--> $DIR/manual_range_pattern.rs:11:25
|
LL | let _ = matches!(f, 4 | 2 | 3 | 1 | 5 | 6 | 9 | 7 | 8 | 10);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=10`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_pattern.rs:19:9
--> $DIR/manual_range_pattern.rs:20:25
|
LL | let _ = matches!(f, (1..=10) | (2..=13) | (14..=48324728) | 48324729);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=48324729`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_pattern.rs:21:25
|
LL | let _ = matches!(f, 0 | (1..=10) | 48324730 | (2..=13) | (14..=48324728) | 48324729);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `0..=48324730`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_pattern.rs:22:25
|
LL | let _ = matches!(f, 0..=1 | 0..=2 | 0..=3);
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `0..=3`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_pattern.rs:25:9
|
LL | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 => true,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=10`

error: aborting due to 3 previous errors
error: aborting due to 6 previous errors

0 comments on commit ab67745

Please sign in to comment.