Skip to content

Commit

Permalink
move check, also lint in macros defined in same crate
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Jun 20, 2023
1 parent ab67745 commit f5a1514
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 37 deletions.
72 changes: 37 additions & 35 deletions clippy_lints/src/manual_range_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use rustc_hir::Expr;
use rustc_hir::ExprKind;
use rustc_hir::PatKind;
use rustc_hir::RangeEnd;
use rustc_lint::LintContext;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
Expand Down Expand Up @@ -46,11 +48,14 @@ fn expr_as_u128(expr: &Expr<'_>) -> Option<u128> {

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

if let PatKind::Or(pats) = pat.kind {
// a pattern like 1 | 2 seems fine, lint if there are at least 3 alternatives
if let PatKind::Or(pats) = pat.kind
&& pats.len() >= 3
{
let mut min = u128::MAX;
let mut max = 0;
let mut numbers_found = FxHashSet::default();
Expand Down Expand Up @@ -80,41 +85,38 @@ impl LateLintPass<'_> for ManualRangePattern {
}
}

// a pattern like 1 | 2 seems fine, lint if there are at least 3 alternatives
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;
}
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(
cx,
MANUAL_RANGE_PATTERN,
pat.span,
"this OR pattern can be rewritten using a range",
"try",
format!("{min}..={max}"),
Applicability::MachineApplicable,
);
}
break 'contains true;
};

if contains_whole_range {
span_lint_and_sugg(
cx,
MANUAL_RANGE_PATTERN,
pat.span,
"this OR pattern can be rewritten using a range",
"try",
format!("{min}..={max}"),
Applicability::MachineApplicable,
);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/manual_range_pattern.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn main() {

macro_rules! mac {
($e:expr) => {
matches!($e, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10)
matches!($e, 1..=10)
};
}
mac!(f);
Expand Down
13 changes: 12 additions & 1 deletion tests/ui/manual_range_pattern.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,16 @@ error: this OR pattern can be rewritten using a range
LL | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 => true,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=10`

error: aborting due to 6 previous errors
error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_pattern.rs:31:26
|
LL | matches!($e, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=10`
...
LL | mac!(f);
| ------- in this macro invocation
|
= note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 7 previous errors

0 comments on commit f5a1514

Please sign in to comment.