New lint is_digit_ascii_radix

pitaj · pitaj · commit 77c3ea05e9f2 · 2022-04-06T19:34:15.000-06:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3354,6 +3354,7 @@ Released 2018-09-13
 [`invalid_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_regex
 [`invalid_upcast_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_upcast_comparisons
 [`invisible_characters`]: https://rust-lang.github.io/rust-clippy/master/index.html#invisible_characters
+[`is_digit_ascii_radix`]: https://rust-lang.github.io/rust-clippy/master/index.html#is_digit_ascii_radix
 [`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements
 [`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect
 [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count
diff --git a/clippy_lints/src/case_sensitive_file_extension_comparisons.rs b/clippy_lints/src/case_sensitive_file_extension_comparisons.rs
@@ -43,8 +43,8 @@ fn check_case_sensitive_file_extension_comparison(ctx: &LateContext<'_>, expr: &
         if let ExprKind::Lit(Spanned { node: LitKind::Str(ext_literal, ..), ..}) = extension.kind;
         if (2..=6).contains(&ext_literal.as_str().len());
         if ext_literal.as_str().starts_with('.');
-        if ext_literal.as_str().chars().skip(1).all(|c| c.is_uppercase() || c.is_digit(10))
-            || ext_literal.as_str().chars().skip(1).all(|c| c.is_lowercase() || c.is_digit(10));
+        if ext_literal.as_str().chars().skip(1).all(|c| c.is_uppercase() || c.is_ascii_digit())
+            || ext_literal.as_str().chars().skip(1).all(|c| c.is_lowercase() || c.is_ascii_digit());
         then {
             let mut ty = ctx.typeck_results().expr_ty(obj);
             ty = match ty.kind() {
diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs
@@ -727,7 +727,7 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span) {
     /// letters (`Clippy` is ok) and one lower-case letter (`NASA` is ok).
     /// Plurals are also excluded (`IDs` is ok).
     fn is_camel_case(s: &str) -> bool {
-        if s.starts_with(|c: char| c.is_digit(10)) {
+        if s.starts_with(|c: char| c.is_ascii_digit()) {
             return false;
         }
 
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
@@ -163,6 +163,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(methods::FLAT_MAP_IDENTITY),
     LintId::of(methods::INSPECT_FOR_EACH),
     LintId::of(methods::INTO_ITER_ON_REF),
+    LintId::of(methods::IS_DIGIT_ASCII_RADIX),
     LintId::of(methods::ITERATOR_STEP_BY_ZERO),
     LintId::of(methods::ITER_CLONED_COLLECT),
     LintId::of(methods::ITER_COUNT),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
@@ -300,6 +300,7 @@ store.register_lints(&[
     methods::INEFFICIENT_TO_STRING,
     methods::INSPECT_FOR_EACH,
     methods::INTO_ITER_ON_REF,
+    methods::IS_DIGIT_ASCII_RADIX,
     methods::ITERATOR_STEP_BY_ZERO,
     methods::ITER_CLONED_COLLECT,
     methods::ITER_COUNT,
diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs
@@ -60,6 +60,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(methods::CHARS_LAST_CMP),
     LintId::of(methods::CHARS_NEXT_CMP),
     LintId::of(methods::INTO_ITER_ON_REF),
+    LintId::of(methods::IS_DIGIT_ASCII_RADIX),
     LintId::of(methods::ITER_CLONED_COLLECT),
     LintId::of(methods::ITER_NEXT_SLICE),
     LintId::of(methods::ITER_NTH_ZERO),
diff --git a/clippy_lints/src/methods/is_digit_ascii_radix.rs b/clippy_lints/src/methods/is_digit_ascii_radix.rs
@@ -0,0 +1,50 @@
+//! Lint for `c.is_digit(10)`
+
+use super::IS_DIGIT_ASCII_RADIX;
+use clippy_utils::{
+    consts::constant_full_int, consts::FullInt, diagnostics::span_lint_and_sugg, meets_msrv, msrvs,
+    source::snippet_with_applicability,
+};
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_semver::RustcVersion;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    self_arg: &'tcx Expr<'_>,
+    radix: &'tcx Expr<'_>,
+    msrv: Option<&RustcVersion>,
+) {
+    if !meets_msrv(msrv, &msrvs::IS_ASCII_DIGIT) {
+        return;
+    }
+
+    if !cx.typeck_results().expr_ty_adjusted(self_arg).peel_refs().is_char() {
+        return;
+    }
+
+    if let Some(radix_val) = constant_full_int(cx, cx.typeck_results(), radix) {
+        let (num, replacement) = match radix_val {
+            FullInt::S(10) | FullInt::U(10) => (10, "is_ascii_digit"),
+            FullInt::S(16) | FullInt::U(16) => (16, "is_ascii_hexdigit"),
+            _ => return,
+        };
+        let mut applicability = Applicability::MachineApplicable;
+
+        span_lint_and_sugg(
+            cx,
+            IS_DIGIT_ASCII_RADIX,
+            expr.span,
+            &format!("use of `char::is_digit` with literal radix of {}", num),
+            "try",
+            format!(
+                "{}.{}()",
+                snippet_with_applicability(cx, self_arg.span, "..", &mut applicability),
+                replacement
+            ),
+            applicability,
+        );
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -26,6 +26,7 @@ mod implicit_clone;
 mod inefficient_to_string;
 mod inspect_for_each;
 mod into_iter_on_ref;
+mod is_digit_ascii_radix;
 mod iter_cloned_collect;
 mod iter_count;
 mod iter_next_slice;
@@ -2106,6 +2107,36 @@ declare_clippy_lint! {
     "using `.collect::<Vec<String>>().join(\"\")` on an iterator"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Finds usages of [`char::is_digit`]
+    /// (https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_digit) that
+    /// can be replaced with [`is_ascii_digit`]
+    /// (https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_ascii_digit) or
+    /// [`is_ascii_hexdigit`]
+    /// (https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_ascii_hexdigit).
+    ///
+    /// ### Why is this bad?
+    /// `is_digit(..)` is slower and requires specifying the radix.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let c: char = '6';
+    /// c.is_digit(10);
+    /// c.is_digit(16);
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let c: char = '6';
+    /// c.is_ascii_digit();
+    /// c.is_ascii_hexdigit();
+    /// ```
+    #[clippy::version = "1.61.0"]
+    pub IS_DIGIT_ASCII_RADIX,
+    style,
+    "use of `char::is_digit(..)` with literal radix of 10 or 16"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -2193,6 +2224,7 @@ impl_lint_pass!(Methods => [
     UNNECESSARY_TO_OWNED,
     UNNECESSARY_JOIN,
     ERR_EXPECT,
+    IS_DIGIT_ASCII_RADIX,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -2488,6 +2520,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             },
             ("get_or_insert_with", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert"),
             ("is_file", []) => filetype_is_file::check(cx, expr, recv),
+            ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, msrv),
             ("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
             ("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
             ("join", [join_arg]) => {
diff --git a/clippy_lints/src/misc_early/mod.rs b/clippy_lints/src/misc_early/mod.rs
@@ -361,7 +361,7 @@ impl MiscEarlyLints {
         // See <https://github.com/rust-lang/rust-clippy/issues/4507> for a regression.
         // FIXME: Find a better way to detect those cases.
         let lit_snip = match snippet_opt(cx, lit.span) {
-            Some(snip) if snip.chars().next().map_or(false, |c| c.is_digit(10)) => snip,
+            Some(snip) if snip.chars().next().map_or(false, |c| c.is_ascii_digit()) => snip,
             _ => return,
         };
 
diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs
@@ -197,7 +197,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
         if interned_name.chars().any(char::is_uppercase) {
             return;
         }
-        if interned_name.chars().all(|c| c.is_digit(10) || c == '_') {
+        if interned_name.chars().all(|c| c.is_ascii_digit() || c == '_') {
             span_lint(
                 self.0.cx,
                 JUST_UNDERSCORES_AND_DIGITS,
diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs
@@ -32,4 +32,5 @@ msrv_aliases! {
     1,28,0 { FROM_BOOL }
     1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR }
     1,16,0 { STR_REPEAT }
+    1,24,0 { IS_ASCII_DIGIT }
 }
diff --git a/clippy_utils/src/numeric_literal.rs b/clippy_utils/src/numeric_literal.rs
@@ -57,7 +57,7 @@ impl<'a> NumericLiteral<'a> {
                 .trim_start()
                 .chars()
                 .next()
-                .map_or(false, |c| c.is_digit(10))
+                .map_or(false, |c| c.is_ascii_digit())
         {
             let (unsuffixed, suffix) = split_suffix(src, lit_kind);
             let float = matches!(lit_kind, LitKind::Float(..));
diff --git a/tests/ui/is_digit_ascii_radix.fixed b/tests/ui/is_digit_ascii_radix.fixed
@@ -0,0 +1,18 @@
+// run-rustfix
+
+#![warn(clippy::is_digit_ascii_radix)]
+
+const TEN: u32 = 10;
+
+fn main() {
+    let c: char = '6';
+
+    // Should trigger the lint.
+    let _ = c.is_ascii_digit();
+    let _ = c.is_ascii_hexdigit();
+    let _ = c.is_ascii_hexdigit();
+
+    // Should not trigger the lint.
+    let _ = c.is_digit(11);
+    let _ = c.is_digit(TEN);
+}
diff --git a/tests/ui/is_digit_ascii_radix.rs b/tests/ui/is_digit_ascii_radix.rs
@@ -0,0 +1,18 @@
+// run-rustfix
+
+#![warn(clippy::is_digit_ascii_radix)]
+
+const TEN: u32 = 10;
+
+fn main() {
+    let c: char = '6';
+
+    // Should trigger the lint.
+    let _ = c.is_digit(10);
+    let _ = c.is_digit(16);
+    let _ = c.is_digit(0x10);
+
+    // Should not trigger the lint.
+    let _ = c.is_digit(11);
+    let _ = c.is_digit(TEN);
+}
diff --git a/tests/ui/is_digit_ascii_radix.stderr b/tests/ui/is_digit_ascii_radix.stderr
@@ -0,0 +1,22 @@
+error: use of `char::is_digit` with literal radix of 10
+  --> $DIR/is_digit_ascii_radix.rs:11:13
+   |
+LL |     let _ = c.is_digit(10);
+   |             ^^^^^^^^^^^^^^ help: try: `c.is_ascii_digit()`
+   |
+   = note: `-D clippy::is-digit-ascii-radix` implied by `-D warnings`
+
+error: use of `char::is_digit` with literal radix of 16
+  --> $DIR/is_digit_ascii_radix.rs:12:13
+   |
+LL |     let _ = c.is_digit(16);
+   |             ^^^^^^^^^^^^^^ help: try: `c.is_ascii_hexdigit()`
+
+error: use of `char::is_digit` with literal radix of 16
+  --> $DIR/is_digit_ascii_radix.rs:13:13
+   |
+LL |     let _ = c.is_digit(0x10);
+   |             ^^^^^^^^^^^^^^^^ help: try: `c.is_ascii_hexdigit()`
+
+error: aborting due to 3 previous errors
+
diff --git a/tests/ui/to_digit_is_some.fixed b/tests/ui/to_digit_is_some.fixed
@@ -6,6 +6,6 @@ fn main() {
     let c = 'x';
     let d = &c;
 
-    let _ = d.is_digit(10);
-    let _ = char::is_digit(c, 10);
+    let _ = d.is_digit(8);
+    let _ = char::is_digit(c, 8);
 }
diff --git a/tests/ui/to_digit_is_some.rs b/tests/ui/to_digit_is_some.rs
@@ -6,6 +6,6 @@ fn main() {
     let c = 'x';
     let d = &c;
 
-    let _ = d.to_digit(10).is_some();
-    let _ = char::to_digit(c, 10).is_some();
+    let _ = d.to_digit(8).is_some();
+    let _ = char::to_digit(c, 8).is_some();
 }
diff --git a/tests/ui/to_digit_is_some.stderr b/tests/ui/to_digit_is_some.stderr
@@ -1,16 +1,16 @@
 error: use of `.to_digit(..).is_some()`
   --> $DIR/to_digit_is_some.rs:9:13
    |
-LL |     let _ = d.to_digit(10).is_some();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `d.is_digit(10)`
+LL |     let _ = d.to_digit(8).is_some();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `d.is_digit(8)`
    |
    = note: `-D clippy::to-digit-is-some` implied by `-D warnings`
 
 error: use of `.to_digit(..).is_some()`
   --> $DIR/to_digit_is_some.rs:10:13
    |
-LL |     let _ = char::to_digit(c, 10).is_some();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `char::is_digit(c, 10)`
+LL |     let _ = char::to_digit(c, 8).is_some();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `char::is_digit(c, 8)`
 
 error: aborting due to 2 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -727,7 +727,7 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span) {`
`727`	`727`	/// letters (`Clippy` is ok) and one lower-case letter (`NASA` is ok).
`728`	`728`	/// Plurals are also excluded (`IDs` is ok).
`729`	`729`	`fn is_camel_case(s: &str) -> bool {`
`730`		`- if s.starts_with(\|c: char\| c.is_digit(10)) {`
	`730`	`+ if s.starts_with(\|c: char\| c.is_ascii_digit()) {`
`731`	`731`	`return false;`
`732`	`732`	`}`
`733`	`733`
Original file line number	Diff line number	Diff line change
`@@ -197,7 +197,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {`
`197`	`197`	`if interned_name.chars().any(char::is_uppercase) {`
`198`	`198`	`return;`
`199`	`199`	`}`
`200`		`- if interned_name.chars().all(\|c\| c.is_digit(10) \|\| c == '_') {`
	`200`	`+ if interned_name.chars().all(\|c\| c.is_ascii_digit() \|\| c == '_') {`
`201`	`201`	`span_lint(`
`202`	`202`	`self.0.cx,`
`203`	`203`	`JUST_UNDERSCORES_AND_DIGITS,`
Original file line number	Diff line number	Diff line change
`@@ -32,4 +32,5 @@ msrv_aliases! {`
`32`	`32`	`1,28,0 { FROM_BOOL }`
`33`	`33`	`1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR }`
`34`	`34`	`1,16,0 { STR_REPEAT }`
	`35`	`+ 1,24,0 { IS_ASCII_DIGIT }`
`35`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ impl<'a> NumericLiteral<'a> {`
`57`	`57`	`.trim_start()`
`58`	`58`	`.chars()`
`59`	`59`	`.next()`
`60`		`- .map_or(false, \|c\| c.is_digit(10))`
	`60`	`+ .map_or(false, \|c\| c.is_ascii_digit())`
`61`	`61`	`{`
`62`	`62`	`let (unsuffixed, suffix) = split_suffix(src, lit_kind);`
`63`	`63`	`let float = matches!(lit_kind, LitKind::Float(..));`
Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,6 @@ fn main() {`
`6`	`6`	`let c = 'x';`
`7`	`7`	`let d = &c;`
`8`	`8`
`9`		`- let _ = d.is_digit(10);`
`10`		`- let _ = char::is_digit(c, 10);`
	`9`	`+ let _ = d.is_digit(8);`
	`10`	`+ let _ = char::is_digit(c, 8);`
`11`	`11`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,16 +1,16 @@`
`1`	`1`	error: use of `.to_digit(..).is_some()`
`2`	`2`	`--> $DIR/to_digit_is_some.rs:9:13`
`3`	`3`	`\|`
`4`		`-LL \| let _ = d.to_digit(10).is_some();`
`5`		- \| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `d.is_digit(10)`
	`4`	`+LL \| let _ = d.to_digit(8).is_some();`
	`5`	+ \| ^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `d.is_digit(8)`
`6`	`6`	`\|`
`7`	`7`	= note: `-D clippy::to-digit-is-some` implied by `-D warnings`
`8`	`8`
`9`	`9`	error: use of `.to_digit(..).is_some()`
`10`	`10`	`--> $DIR/to_digit_is_some.rs:10:13`
`11`	`11`	`\|`
`12`		`-LL \| let _ = char::to_digit(c, 10).is_some();`
`13`		- \| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `char::is_digit(c, 10)`
	`12`	`+LL \| let _ = char::to_digit(c, 8).is_some();`
	`13`	+ \| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `char::is_digit(c, 8)`
`14`	`14`
`15`	`15`	`error: aborting due to 2 previous errors`
`16`	`16`