about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-09-07 13:50:16 +0000
committerbors <bors@rust-lang.org>2023-09-07 13:50:16 +0000
commit6150bf5b9219f2cd3e5494fbe4d3c87baf0f036e (patch)
treeb713cd78dd22e7b307363afa4dd0de7275a3fbdf
parent415ba21c3b78125cf583ca0ef7a318a8f347a282 (diff)
parentbbf67c3424cb76ce7ae92e012f1bf23c1161eea7 (diff)
downloadrust-6150bf5b9219f2cd3e5494fbe4d3c87baf0f036e.tar.gz
rust-6150bf5b9219f2cd3e5494fbe4d3c87baf0f036e.zip
Auto merge of #11462 - Alexendoo:manual-range-patterns-preserve-literals, 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
-rw-r--r--clippy_lints/src/manual_range_patterns.rs129
-rw-r--r--tests/ui/manual_range_patterns.fixed17
-rw-r--r--tests/ui/manual_range_patterns.rs11
-rw-r--r--tests/ui/manual_range_patterns.stderr66
4 files changed, 171 insertions, 52 deletions
diff --git a/clippy_lints/src/manual_range_patterns.rs b/clippy_lints/src/manual_range_patterns.rs
index 39d8b20d38d..90557b55560 100644
--- a/clippy_lints/src/manual_range_patterns.rs
+++ b/clippy_lints/src/manual_range_patterns.rs
@@ -1,4 +1,5 @@
-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;
@@ -6,6 +7,7 @@ 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
@@ -49,6 +51,29 @@ 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) {
@@ -56,71 +81,83 @@ impl LateLintPass<'_> for ManualRangePatterns {
         }
 
         // 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,
+                        );
+                    }
+                },
+            );
         }
     }
 }
diff --git a/tests/ui/manual_range_patterns.fixed b/tests/ui/manual_range_patterns.fixed
index ea06e27d153..b348d7071f6 100644
--- a/tests/ui/manual_range_patterns.fixed
+++ b/tests/ui/manual_range_patterns.fixed
@@ -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);
@@ -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)
diff --git a/tests/ui/manual_range_patterns.rs b/tests/ui/manual_range_patterns.rs
index eb29987b89f..a0750f54b73 100644
--- a/tests/ui/manual_range_patterns.rs
+++ b/tests/ui/manual_range_patterns.rs
@@ -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)
diff --git a/tests/ui/manual_range_patterns.stderr b/tests/ui/manual_range_patterns.stderr
index 1cf58d7df6a..60ee75d1b8e 100644
--- a/tests/ui/manual_range_patterns.stderr
+++ b/tests/ui/manual_range_patterns.stderr
@@ -13,6 +13,18 @@ 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
    |
 LL |     let _ = matches!(f, (1..=10) | (2..=13) | (14..=48324728) | 48324729);
@@ -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`
@@ -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