Skip to content

Commit

Permalink
Auto merge of #11462 - Alexendoo:manual-range-patterns-preserve-liter…
Browse files Browse the repository at this point in the history 
…als, r=blyxyas

Preserve literals and range kinds in `manual_range_patterns`

Fixes #11461

Also enables linting when there are 3 or fewer alternatives if one of them is already a range pattern

changelog: none
  • Loading branch information
@bors
bors committed Sep 7, 2023
2 parents 415ba21 + bbf67c3 commit 6150bf5
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 52 deletions.
129 changes: 83 additions & 46 deletions clippy_lints/src/manual_range_patterns.rs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_opt;
use rustc_ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, PatKind, RangeEnd, UnOp};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{Span, DUMMY_SP};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -49,78 +51,113 @@ fn expr_as_i128(expr: &Expr<'_>) -> Option<i128> {
}
}

#[derive(Copy, Clone)]
struct Num {
val: i128,
span: Span,
}

impl Num {
fn new(expr: &Expr<'_>) -> Option<Self> {
Some(Self {
val: expr_as_i128(expr)?,
span: expr.span,
})
}

fn dummy(val: i128) -> Self {
Self { val, span: DUMMY_SP }
}

fn min(self, other: Self) -> Self {
if self.val < other.val { self } else { other }
}
}

impl LateLintPass<'_> for ManualRangePatterns {
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &'_ rustc_hir::Pat<'_>) {
if in_external_macro(cx.sess(), pat.span) {
return;
}

// a pattern like 1 | 2 seems fine, lint if there are at least 3 alternatives
// or at least one range
if let PatKind::Or(pats) = pat.kind
&& pats.len() >= 3
&& (pats.len() >= 3 || pats.iter().any(|p| matches!(p.kind, PatKind::Range(..))))
{
let mut min = i128::MAX;
let mut max = i128::MIN;
let mut min = Num::dummy(i128::MAX);
let mut max = Num::dummy(i128::MIN);
let mut range_kind = RangeEnd::Included;
let mut numbers_found = FxHashSet::default();
let mut ranges_found = Vec::new();

for pat in pats {
if let PatKind::Lit(lit) = pat.kind
&& let Some(num) = expr_as_i128(lit)
&& let Some(num) = Num::new(lit)
{
numbers_found.insert(num);
numbers_found.insert(num.val);

min = min.min(num);
max = max.max(num);
if num.val >= max.val {
max = num;
range_kind = RangeEnd::Included;
}
} else if let PatKind::Range(Some(left), Some(right), end) = pat.kind
&& let Some(left) = expr_as_i128(left)
&& let Some(right) = expr_as_i128(right)
&& right >= left
&& let Some(left) = Num::new(left)
&& let Some(mut right) = Num::new(right)
{
if let RangeEnd::Excluded = end {
right.val -= 1;
}

min = min.min(left);
max = max.max(right);
ranges_found.push(left..=match end {
RangeEnd::Included => right,
RangeEnd::Excluded => right - 1,
});
if right.val > max.val {
max = right;
range_kind = end;
}
ranges_found.push(left.val..=right.val);
} else {
return;
}
}

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;
}
let mut num = min.val;
while num <= max.val {
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 {
return;
}
break 'contains true;
};

if contains_whole_range {
span_lint_and_sugg(
cx,
MANUAL_RANGE_PATTERNS,
pat.span,
"this OR pattern can be rewritten using a range",
"try",
format!("{min}..={max}"),
Applicability::MachineApplicable,
);
}

span_lint_and_then(
cx,
MANUAL_RANGE_PATTERNS,
pat.span,
"this OR pattern can be rewritten using a range",
|diag| {
if let Some(min) = snippet_opt(cx, min.span)
&& let Some(max) = snippet_opt(cx, max.span)
{
diag.span_suggestion(
pat.span,
"try",
format!("{min}{range_kind}{max}"),
Applicability::MachineApplicable,
);
}
},
);
}
}
}
17 changes: 14 additions & 3 deletions tests/ui/manual_range_patterns.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ 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..=4);
let _ = matches!(f, 1..4);
let _ = matches!(f, 1..=48324729);
let _ = matches!(f, 0..=48324730);
let _ = matches!(f, 0..=3);
Expand All @@ -25,9 +25,20 @@ fn main() {
};
let _ = matches!(f, -5..=3);
let _ = matches!(f, -1 | -5 | 3 | -2 | -4 | -3 | 0 | 1); // 2 is missing
let _ = matches!(f, -1000001..=1000001);
let _ = matches!(f, -1_000_001..=1_000_001);
let _ = matches!(f, -1_000_000..=1_000_000 | -1_000_001 | 1_000_002);

matches!(f, 0x00..=0x03);
matches!(f, 0x00..=0x07);
matches!(f, -0x09..=0x00);

matches!(f, 0..=5);
matches!(f, 0..5);

matches!(f, 0..10);
matches!(f, 0..=10);
matches!(f, 0..=10);

macro_rules! mac {
($e:expr) => {
matches!($e, 1..=10)
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/manual_range_patterns.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ fn main() {
let _ = matches!(f, -1_000_000..=1_000_000 | -1_000_001 | 1_000_001);
let _ = matches!(f, -1_000_000..=1_000_000 | -1_000_001 | 1_000_002);

matches!(f, 0x00 | 0x01 | 0x02 | 0x03);
matches!(f, 0x00..=0x05 | 0x06 | 0x07);
matches!(f, -0x09 | -0x08 | -0x07..=0x00);

matches!(f, 0..5 | 5);
matches!(f, 0 | 1..5);

matches!(f, 0..=5 | 6..10);
matches!(f, 0..5 | 5..=10);
matches!(f, 5..=10 | 0..5);

macro_rules! mac {
($e:expr) => {
matches!($e, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10)
Expand Down
66 changes: 63 additions & 3 deletions tests/ui/manual_range_patterns.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ error: this OR pattern can be rewritten using a range
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_patterns.rs:16:25
|
LL | let _ = matches!(f, 1 | (2..=4));
| ^^^^^^^^^^^ help: try: `1..=4`

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

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:18:25
|
Expand Down Expand Up @@ -46,10 +58,58 @@ error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:28:25
|
LL | let _ = matches!(f, -1_000_000..=1_000_000 | -1_000_001 | 1_000_001);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `-1000001..=1000001`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `-1_000_001..=1_000_001`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:31:17
|
LL | matches!(f, 0x00 | 0x01 | 0x02 | 0x03);
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `0x00..=0x03`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:32:17
|
LL | matches!(f, 0x00..=0x05 | 0x06 | 0x07);
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `0x00..=0x07`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:33:17
|
LL | matches!(f, -0x09 | -0x08 | -0x07..=0x00);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `-0x09..=0x00`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:35:17
|
LL | matches!(f, 0..5 | 5);
| ^^^^^^^^ help: try: `0..=5`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:36:17
|
LL | matches!(f, 0 | 1..5);
| ^^^^^^^^ help: try: `0..5`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:38:17
|
LL | matches!(f, 0..=5 | 6..10);
| ^^^^^^^^^^^^^ help: try: `0..10`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:39:17
|
LL | matches!(f, 0..5 | 5..=10);
| ^^^^^^^^^^^^^ help: try: `0..=10`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:40:17
|
LL | matches!(f, 5..=10 | 0..5);
| ^^^^^^^^^^^^^ help: try: `0..=10`

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:33:26
--> $DIR/manual_range_patterns.rs:44:26
|
LL | matches!($e, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=10`
Expand All @@ -59,5 +119,5 @@ LL | mac!(f);
|
= note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 9 previous errors
error: aborting due to 19 previous errors

0 comments on commit 6150bf5

Please sign in to comment.