From 06cfeb90c156d70c2318774081b37db809f6d840 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Sat, 2 Apr 2022 15:01:07 -0600 Subject: [PATCH] New lint `is_digit_ascii_radix` --- CHANGELOG.md | 1 + ...se_sensitive_file_extension_comparisons.rs | 4 +- clippy_lints/src/doc.rs | 2 +- clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + .../src/methods/is_digit_ascii_radix.rs | 50 +++++++++++++++++++ clippy_lints/src/methods/mod.rs | 33 ++++++++++++ clippy_lints/src/misc_early/mod.rs | 2 +- clippy_lints/src/non_expressive_names.rs | 2 +- clippy_utils/src/msrvs.rs | 1 + clippy_utils/src/numeric_literal.rs | 2 +- tests/ui/is_digit_ascii_radix.fixed | 18 +++++++ tests/ui/is_digit_ascii_radix.rs | 18 +++++++ tests/ui/is_digit_ascii_radix.stderr | 22 ++++++++ tests/ui/to_digit_is_some.fixed | 4 +- tests/ui/to_digit_is_some.rs | 4 +- tests/ui/to_digit_is_some.stderr | 8 +-- 18 files changed, 160 insertions(+), 14 deletions(-) create mode 100644 clippy_lints/src/methods/is_digit_ascii_radix.rs create mode 100644 tests/ui/is_digit_ascii_radix.fixed create mode 100644 tests/ui/is_digit_ascii_radix.rs create mode 100644 tests/ui/is_digit_ascii_radix.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b4097ea86..c5c93b113 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3356,6 +3356,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 index df780747a..e3e31c586 100644 --- 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 index 28d0c75fd..e72031e48 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -739,7 +739,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 index 14ca93b5f..aa8e3b535 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -165,6 +165,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 index 532590aaa..7436f3112 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -302,6 +302,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 index 3114afac8..2d98802cb 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -61,6 +61,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(methods::CHARS_NEXT_CMP), LintId::of(methods::ERR_EXPECT), 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 new file mode 100644 index 000000000..ad333df2f --- /dev/null +++ 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 index 70d021a16..6a7841a90 100644 --- 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; @@ -2131,6 +2132,36 @@ declare_clippy_lint! { "no-op use of `deref` or `deref_mut` method to `Option`." } +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, @@ -2219,6 +2250,7 @@ impl_lint_pass!(Methods => [ UNNECESSARY_JOIN, ERR_EXPECT, NEEDLESS_OPTION_AS_DEREF, + IS_DIGIT_ASCII_RADIX, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -2516,6 +2548,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 index d955fad7d..6860b60ac 100644 --- a/clippy_lints/src/misc_early/mod.rs +++ b/clippy_lints/src/misc_early/mod.rs @@ -361,7 +361,7 @@ impl MiscEarlyLints { // See 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 index 0d0c88b02..e3bc40c4b 100644 --- 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 index 0424e0672..134fd1ce5 100644 --- 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 index 908ff8227..b92d42e83 100644 --- 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 new file mode 100644 index 000000000..c0ba647d7 --- /dev/null +++ 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 new file mode 100644 index 000000000..68e3f3243 --- /dev/null +++ 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 new file mode 100644 index 000000000..dc5cb2913 --- /dev/null +++ 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 index 19184df0b..3c5e96427 100644 --- 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 index 45a6728eb..4f247c06c 100644 --- 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 index 177d3ccd3..10a1b393a 100644 --- 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