Auto merge of #9765 - koka831:feat/manual_is_ascii_check, r=xFrednet

Add `manual_is_ascii_check` lint

Addresses https://github.com/rust-lang/rust-clippy/issues/9290

This PR adds new lint `manual_is_ascii_check`, which detects comparison with ascii ranges using `matches!` macros.

As I mentioned as following in the Issue;
> Yes, that's true. we'll start small and then grow it.
> So I'll try to handle matches! macro with single range as suggested above.

However during writing first version, I was thinking that the changes to support alphabetic and digits will be small patch, so I made a single PR in hope review cost can be reduced.

changelog: add new lint [`manual_is_ascii_check`]

r? `@xFrednet`

CHANGELOG.md

@@ -4142,6 +4142,7 @@ Released 2018-09-13
 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
 [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
 [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
+[`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
 [`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
 [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map
 [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy

clippy_lints/src/declared_lints.rs

@@ -251,6 +251,7 @@
     crate::manual_bits::MANUAL_BITS_INFO,
     crate::manual_clamp::MANUAL_CLAMP_INFO,
     crate::manual_instant_elapsed::MANUAL_INSTANT_ELAPSED_INFO,
+    crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
     crate::manual_let_else::MANUAL_LET_ELSE_INFO,
     crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
     crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,

clippy_lints/src/lib.rs

@@ -173,6 +173,7 @@
 mod manual_bits;
 mod manual_clamp;
 mod manual_instant_elapsed;
+mod manual_is_ascii_check;
 mod manual_let_else;
 mod manual_non_exhaustive;
 mod manual_rem_euclid;
@@ -919,6 +920,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(missing_trait_methods::MissingTraitMethods));
     store.register_late_pass(|_| Box::new(from_raw_with_void_ptr::FromRawWithVoidPtr));
     store.register_late_pass(|_| Box::new(suspicious_xor_used_as_pow::ConfusingXorAndPow));
+    store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/manual_is_ascii_check.rs

@@ -0,0 +1,157 @@
+use rustc_ast::LitKind::{Byte, Char};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, PatKind, RangeEnd};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_semver::RustcVersion;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{def_id::DefId, sym};
+
+use clippy_utils::{
+    diagnostics::span_lint_and_sugg, in_constant, macros::root_macro_call, meets_msrv, msrvs, source::snippet,
+};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Suggests to use dedicated built-in methods,
+    /// `is_ascii_(lowercase|uppercase|digit)` for checking on corresponding ascii range
+    ///
+    /// ### Why is this bad?
+    /// Using the built-in functions is more readable and makes it
+    /// clear that it's not a specific subset of characters, but all
+    /// ASCII (lowercase|uppercase|digit) characters.
+    /// ### Example
+    /// ```rust
+    /// fn main() {
+    ///     assert!(matches!('x', 'a'..='z'));
+    ///     assert!(matches!(b'X', b'A'..=b'Z'));
+    ///     assert!(matches!('2', '0'..='9'));
+    ///     assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn main() {
+    ///     assert!('x'.is_ascii_lowercase());
+    ///     assert!(b'X'.is_ascii_uppercase());
+    ///     assert!('2'.is_ascii_digit());
+    ///     assert!('x'.is_ascii_alphabetic());
+    /// }
+    /// ```
+    #[clippy::version = "1.66.0"]
+    pub MANUAL_IS_ASCII_CHECK,
+    style,
+    "use dedicated method to check ascii range"
+}
+impl_lint_pass!(ManualIsAsciiCheck => [MANUAL_IS_ASCII_CHECK]);
+
+pub struct ManualIsAsciiCheck {
+    msrv: Option<RustcVersion>,
+}
+
+impl ManualIsAsciiCheck {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self { msrv }
+    }
+}
+
+#[derive(Debug, PartialEq)]
+enum CharRange {
+    /// 'a'..='z' | b'a'..=b'z'
+    LowerChar,
+    /// 'A'..='Z' | b'A'..=b'Z'
+    UpperChar,
+    /// AsciiLower | AsciiUpper
+    FullChar,
+    /// '0..=9'
+    Digit,
+    Otherwise,
+}
+
+impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if !meets_msrv(self.msrv, msrvs::IS_ASCII_DIGIT) {
+            return;
+        }
+
+        if in_constant(cx, expr.hir_id) && !meets_msrv(self.msrv, msrvs::IS_ASCII_DIGIT_CONST) {
+            return;
+        }
+
+        let Some(macro_call) = root_macro_call(expr.span) else { return };
+
+        if is_matches_macro(cx, macro_call.def_id) {
+            if let ExprKind::Match(recv, [arm, ..], _) = expr.kind {
+                let range = check_pat(&arm.pat.kind);
+
+                if let Some(sugg) = match range {
+                    CharRange::UpperChar => Some("is_ascii_uppercase"),
+                    CharRange::LowerChar => Some("is_ascii_lowercase"),
+                    CharRange::FullChar => Some("is_ascii_alphabetic"),
+                    CharRange::Digit => Some("is_ascii_digit"),
+                    CharRange::Otherwise => None,
+                } {
+                    let mut applicability = Applicability::MaybeIncorrect;
+                    let default_snip = "..";
+                    // `snippet_with_applicability` may set applicability to `MaybeIncorrect` for
+                    // macro span, so we check applicability manually by comaring `recv` is not default.
+                    let recv = snippet(cx, recv.span, default_snip);
+
+                    if recv != default_snip {
+                        applicability = Applicability::MachineApplicable;
+                    }
+
+                    span_lint_and_sugg(
+                        cx,
+                        MANUAL_IS_ASCII_CHECK,
+                        macro_call.span,
+                        "manual check for common ascii range",
+                        "try",
+                        format!("{recv}.{sugg}()"),
+                        applicability,
+                    );
+                }
+            }
+        }
+    }
+
+    extract_msrv_attr!(LateContext);
+}
+
+fn check_pat(pat_kind: &PatKind<'_>) -> CharRange {
+    match pat_kind {
+        PatKind::Or(pats) => {
+            let ranges = pats.iter().map(|p| check_pat(&p.kind)).collect::<Vec<_>>();
+
+            if ranges.len() == 2 && ranges.contains(&CharRange::UpperChar) && ranges.contains(&CharRange::LowerChar) {
+                CharRange::FullChar
+            } else {
+                CharRange::Otherwise
+            }
+        },
+        PatKind::Range(Some(start), Some(end), kind) if *kind == RangeEnd::Included => check_range(start, end),
+        _ => CharRange::Otherwise,
+    }
+}
+
+fn check_range(start: &Expr<'_>, end: &Expr<'_>) -> CharRange {
+    if let ExprKind::Lit(start_lit) = &start.kind
+        && let ExprKind::Lit(end_lit) = &end.kind {
+        match (&start_lit.node, &end_lit.node) {
+            (Char('a'), Char('z')) | (Byte(b'a'), Byte(b'z')) => CharRange::LowerChar,
+            (Char('A'), Char('Z')) | (Byte(b'A'), Byte(b'Z')) => CharRange::UpperChar,
+            (Char('0'), Char('9')) | (Byte(b'0'), Byte(b'9')) => CharRange::Digit,
+            _ => CharRange::Otherwise,
+        }
+    } else {
+        CharRange::Otherwise
+    }
+}
+
+fn is_matches_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool {
+    if let Some(name) = cx.tcx.get_diagnostic_name(macro_def_id) {
+        return sym::matches_macro == name;
+    }
+
+    false
+}

clippy_utils/src/msrvs.rs

@@ -19,7 +19,7 @@ macro_rules! msrv_aliases {
     1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
     1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS }
     1,50,0 { BOOL_THEN, CLAMP }
-    1,47,0 { TAU }
+    1,47,0 { TAU, IS_ASCII_DIGIT_CONST }
     1,46,0 { CONST_IF_MATCH }
     1,45,0 { STR_STRIP_PREFIX }
     1,43,0 { LOG2_10, LOG10_2 }

tests/ui/manual_is_ascii_check.fixed

@@ -0,0 +1,45 @@
+// run-rustfix
+
+#![feature(custom_inner_attributes)]
+#![allow(unused, dead_code)]
+#![warn(clippy::manual_is_ascii_check)]
+
+fn main() {
+    assert!('x'.is_ascii_lowercase());
+    assert!('X'.is_ascii_uppercase());
+    assert!(b'x'.is_ascii_lowercase());
+    assert!(b'X'.is_ascii_uppercase());
+
+    let num = '2';
+    assert!(num.is_ascii_digit());
+    assert!(b'1'.is_ascii_digit());
+    assert!('x'.is_ascii_alphabetic());
+
+    assert!(matches!('x', 'A'..='Z' | 'a'..='z' | '_'));
+}
+
+fn msrv_1_23() {
+    #![clippy::msrv = "1.23"]
+
+    assert!(matches!(b'1', b'0'..=b'9'));
+    assert!(matches!('X', 'A'..='Z'));
+    assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
+}
+
+fn msrv_1_24() {
+    #![clippy::msrv = "1.24"]
+
+    assert!(b'1'.is_ascii_digit());
+    assert!('X'.is_ascii_uppercase());
+    assert!('x'.is_ascii_alphabetic());
+}
+
+fn msrv_1_46() {
+    #![clippy::msrv = "1.46"]
+    const FOO: bool = matches!('x', '0'..='9');
+}
+
+fn msrv_1_47() {
+    #![clippy::msrv = "1.47"]
+    const FOO: bool = 'x'.is_ascii_digit();
+}

tests/ui/manual_is_ascii_check.rs

@@ -0,0 +1,45 @@
+// run-rustfix
+
+#![feature(custom_inner_attributes)]
+#![allow(unused, dead_code)]
+#![warn(clippy::manual_is_ascii_check)]
+
+fn main() {
+    assert!(matches!('x', 'a'..='z'));
+    assert!(matches!('X', 'A'..='Z'));
+    assert!(matches!(b'x', b'a'..=b'z'));
+    assert!(matches!(b'X', b'A'..=b'Z'));
+
+    let num = '2';
+    assert!(matches!(num, '0'..='9'));
+    assert!(matches!(b'1', b'0'..=b'9'));
+    assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
+
+    assert!(matches!('x', 'A'..='Z' | 'a'..='z' | '_'));
+}
+
+fn msrv_1_23() {
+    #![clippy::msrv = "1.23"]
+
+    assert!(matches!(b'1', b'0'..=b'9'));
+    assert!(matches!('X', 'A'..='Z'));
+    assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
+}
+
+fn msrv_1_24() {
+    #![clippy::msrv = "1.24"]
+
+    assert!(matches!(b'1', b'0'..=b'9'));
+    assert!(matches!('X', 'A'..='Z'));
+    assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
+}
+
+fn msrv_1_46() {
+    #![clippy::msrv = "1.46"]
+    const FOO: bool = matches!('x', '0'..='9');
+}
+
+fn msrv_1_47() {
+    #![clippy::msrv = "1.47"]
+    const FOO: bool = matches!('x', '0'..='9');
+}

tests/ui/manual_is_ascii_check.stderr

@@ -0,0 +1,70 @@
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:8:13
+   |
+LL |     assert!(matches!('x', 'a'..='z'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_lowercase()`
+   |
+   = note: `-D clippy::manual-is-ascii-check` implied by `-D warnings`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:9:13
+   |
+LL |     assert!(matches!('X', 'A'..='Z'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'X'.is_ascii_uppercase()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:10:13
+   |
+LL |     assert!(matches!(b'x', b'a'..=b'z'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'x'.is_ascii_lowercase()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:11:13
+   |
+LL |     assert!(matches!(b'X', b'A'..=b'Z'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'X'.is_ascii_uppercase()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:14:13
+   |
+LL |     assert!(matches!(num, '0'..='9'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.is_ascii_digit()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:15:13
+   |
+LL |     assert!(matches!(b'1', b'0'..=b'9'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'1'.is_ascii_digit()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:16:13
+   |
+LL |     assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_alphabetic()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:32:13
+   |
+LL |     assert!(matches!(b'1', b'0'..=b'9'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'1'.is_ascii_digit()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:33:13
+   |
+LL |     assert!(matches!('X', 'A'..='Z'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'X'.is_ascii_uppercase()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:34:13
+   |
+LL |     assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_alphabetic()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:44:23
+   |
+LL |     const FOO: bool = matches!('x', '0'..='9');
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_digit()`
+
+error: aborting due to 11 previous errors
+