Auto merge of #10968 - y21:manual_range_pat, r=Centri3

new lint: `manual_range_patterns` Fixes #4931 changelog: new lint: [`manual_range_patterns`]
rust-lang · Jun 27, 2023 · 9e57657 · 9e57657
2 parents 6ce656f + b592d39
commit 9e57657
Show file tree

Hide file tree

Showing 13 changed files with 289 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4949,6 +4949,7 @@ Released 2018-09-13
 [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
 [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
+[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
 [`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
 [`manual_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain
 [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -278,6 +278,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::manual_let_else::MANUAL_LET_ELSE_INFO,
     crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
     crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
+    crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO,
     crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
     crate::manual_retain::MANUAL_RETAIN_INFO,
     crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -188,6 +188,7 @@ mod manual_is_ascii_check;
 mod manual_let_else;
 mod manual_main_separator_str;
 mod manual_non_exhaustive;
+mod manual_range_patterns;
 mod manual_rem_euclid;
 mod manual_retain;
 mod manual_slice_size_calculation;
@@ -1068,6 +1069,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
             needless_raw_string_hashes_allow_one,
         })
     });
+    store.register_late_pass(|_| Box::new(manual_range_patterns::ManualRangePatterns));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

diff --git a/clippy_lints/src/manual_range_patterns.rs b/clippy_lints/src/manual_range_patterns.rs
@@ -0,0 +1,123 @@
+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::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! {
+    /// ### What it does
+    /// Looks for combined OR patterns that are all contained in a specific range,
+    /// e.g. `6 | 4 | 5 | 9 | 7 | 8` can be rewritten as `4..=9`.
+    ///
+    /// ### Why is this bad?
+    /// Using an explicit range is more concise and easier to read.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let x = 6;
+    /// let foo = matches!(x, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10);
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let x = 6;
+    /// let foo = matches!(x, 1..=10);
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub MANUAL_RANGE_PATTERNS,
+    complexity,
+    "manually writing range patterns using a combined OR pattern (`|`)"
+}
+declare_lint_pass!(ManualRangePatterns => [MANUAL_RANGE_PATTERNS]);
+
+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 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
+        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();
+            let mut ranges_found = Vec::new();
+
+            for pat in pats {
+                if let PatKind::Lit(lit) = pat.kind
+                    && 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;
+                }
+            }
+
+            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_PATTERNS,
+                    pat.span,
+                    "this OR pattern can be rewritten using a range",
+                    "try",
+                    format!("{min}..={max}"),
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
+    }
+}
diff --git a/tests/ui/manual_range_patterns.fixed b/tests/ui/manual_range_patterns.fixed
@@ -0,0 +1,35 @@
+//@run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::manual_range_patterns)]
+#![feature(exclusive_range_pattern)]
+
+fn main() {
+    let f = 6;
+
+    let _ = matches!(f, 1..=10);
+    let _ = matches!(f, 1..=10);
+    let _ = matches!(f, 4 | 2 | 3 | 1 | 5 | 6 | 9 | 8 | 10); // 7 is missing
+    let _ = matches!(f, | 4);
+    let _ = matches!(f, 4 | 5);
+    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,
+        _ => false,
+    };
+
+    macro_rules! mac {
+        ($e:expr) => {
+            matches!($e, 1..=10)
+        };
+    }
+    mac!(f);
+}
diff --git a/tests/ui/manual_range_patterns.rs b/tests/ui/manual_range_patterns.rs
@@ -0,0 +1,35 @@
+//@run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::manual_range_patterns)]
+#![feature(exclusive_range_pattern)]
+
+fn main() {
+    let f = 6;
+
+    let _ = matches!(f, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10);
+    let _ = matches!(f, 4 | 2 | 3 | 1 | 5 | 6 | 9 | 7 | 8 | 10);
+    let _ = matches!(f, 4 | 2 | 3 | 1 | 5 | 6 | 9 | 8 | 10); // 7 is missing
+    let _ = matches!(f, | 4);
+    let _ = matches!(f, 4 | 5);
+    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,
+        _ => false,
+    };
+
+    macro_rules! mac {
+        ($e:expr) => {
+            matches!($e, 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10)
+        };
+    }
+    mac!(f);
+}
diff --git a/tests/ui/manual_range_patterns.stderr b/tests/ui/manual_range_patterns.stderr
@@ -0,0 +1,51 @@
+error: this OR pattern can be rewritten using a range
+  --> $DIR/manual_range_patterns.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-patterns` implied by `-D warnings`
+
+error: this OR pattern can be rewritten using a range
+  --> $DIR/manual_range_patterns.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_patterns.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_patterns.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_patterns.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_patterns.rs:25:9
+   |
+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
+   |
+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
+
diff --git a/tests/ui/unnested_or_patterns.fixed b/tests/ui/unnested_or_patterns.fixed
@@ -6,7 +6,8 @@
     clippy::cognitive_complexity,
     clippy::match_ref_pats,
     clippy::upper_case_acronyms,
-    clippy::needless_if
+    clippy::needless_if,
+    clippy::manual_range_patterns
 )]
 #![allow(unreachable_patterns, irrefutable_let_patterns, unused)]
 

diff --git a/tests/ui/unnested_or_patterns.rs b/tests/ui/unnested_or_patterns.rs
@@ -6,7 +6,8 @@
     clippy::cognitive_complexity,
     clippy::match_ref_pats,
     clippy::upper_case_acronyms,
-    clippy::needless_if
+    clippy::needless_if,
+    clippy::manual_range_patterns
 )]
 #![allow(unreachable_patterns, irrefutable_let_patterns, unused)]