Skip to content

Commit

Permalink
Auto merge of #11096 - y21:issue11091, r=giraffate
Browse files Browse the repository at this point in the history
[`manual_range_patterns`]: lint negative values

Fixes #11091.

Now also lints negative values in patterns (`-1 | -2 | -3`)

changelog: [`manual_range_patterns`]: lint negative values
  • Loading branch information
bors committed Jul 10, 2023
2 parents 507d1c2 + c927912 commit 9058b04
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 10 deletions.
24 changes: 16 additions & 8 deletions clippy_lints/src/manual_range_patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_hir::Expr;
use rustc_hir::ExprKind;
use rustc_hir::PatKind;
use rustc_hir::RangeEnd;
use rustc_hir::UnOp;
use rustc_lint::LintContext;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
Expand All @@ -19,6 +20,10 @@ declare_clippy_lint! {
/// ### Why is this bad?
/// Using an explicit range is more concise and easier to read.
///
/// ### Known issues
/// This lint intentionally does not handle numbers greater than `i128::MAX` for `u128` literals
/// in order to support negative numbers.
///
/// ### Example
/// ```rust
/// let x = 6;
Expand All @@ -36,11 +41,14 @@ declare_clippy_lint! {
}
declare_lint_pass!(ManualRangePatterns => [MANUAL_RANGE_PATTERNS]);

fn expr_as_u128(expr: &Expr<'_>) -> Option<u128> {
if let ExprKind::Lit(lit) = expr.kind
fn expr_as_i128(expr: &Expr<'_>) -> Option<i128> {
if let ExprKind::Unary(UnOp::Neg, expr) = expr.kind {
expr_as_i128(expr).map(|num| -num)
} else if let ExprKind::Lit(lit) = expr.kind
&& let LitKind::Int(num, _) = lit.node
{
Some(num)
// Intentionally not handling numbers greater than i128::MAX (for u128 literals) for now.
num.try_into().ok()
} else {
None
}
Expand All @@ -56,22 +64,22 @@ impl LateLintPass<'_> for ManualRangePatterns {
if let PatKind::Or(pats) = pat.kind
&& pats.len() >= 3
{
let mut min = u128::MAX;
let mut max = 0;
let mut min = i128::MAX;
let mut max = i128::MIN;
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_u128(lit)
&& let Some(num) = expr_as_i128(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)
&& let Some(left) = expr_as_i128(left)
&& let Some(right) = expr_as_i128(right)
&& right >= left
{
min = min.min(left);
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/manual_range_patterns.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ fn main() {
1..=10 => true,
_ => false,
};
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_000..=1_000_000 | -1_000_001 | 1_000_002);

macro_rules! mac {
($e:expr) => {
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/manual_range_patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ fn main() {
1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 => true,
_ => false,
};
let _ = matches!(f, -1 | -5 | 3 | -2 | -4 | -3 | 0 | 1 | 2);
let _ = matches!(f, -1 | -5 | 3 | -2 | -4 | -3 | 0 | 1); // 2 is missing
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);

macro_rules! mac {
($e:expr) => {
Expand Down
16 changes: 14 additions & 2 deletions tests/ui/manual_range_patterns.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,19 @@ LL | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 => true,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=10`

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

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

error: this OR pattern can be rewritten using a range
--> $DIR/manual_range_patterns.rs:35:26
|
LL | matches!($e, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `1..=10`
Expand All @@ -47,5 +59,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 7 previous errors
error: aborting due to 9 previous errors

0 comments on commit 9058b04

Please sign in to comment.