Auto merge of #8624 - pitaj:is_digit_ascii_radix, r=xFrednet

New lint `is_digit_ascii_radix`

Closes #6399

changelog: Added [`is_digit_ascii_radix`]: recommend `is_ascii_digit()` or `is_ascii_hexdigit()` in place of `is_digit(10)` and `is_digit(16)`
This commit is contained in:
bors 2022-04-11 18:56:21 +00:00
commit dbcd82885f
18 changed files with 160 additions and 14 deletions

View File

@ -3356,6 +3356,7 @@ Released 2018-09-13
[`invalid_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_regex [`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 [`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 [`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 [`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_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 [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count

View File

@ -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 let ExprKind::Lit(Spanned { node: LitKind::Str(ext_literal, ..), ..}) = extension.kind;
if (2..=6).contains(&ext_literal.as_str().len()); if (2..=6).contains(&ext_literal.as_str().len());
if ext_literal.as_str().starts_with('.'); if ext_literal.as_str().starts_with('.');
if ext_literal.as_str().chars().skip(1).all(|c| c.is_uppercase() || 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_digit(10)); || ext_literal.as_str().chars().skip(1).all(|c| c.is_lowercase() || c.is_ascii_digit());
then { then {
let mut ty = ctx.typeck_results().expr_ty(obj); let mut ty = ctx.typeck_results().expr_ty(obj);
ty = match ty.kind() { ty = match ty.kind() {

View File

@ -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). /// letters (`Clippy` is ok) and one lower-case letter (`NASA` is ok).
/// Plurals are also excluded (`IDs` is ok). /// Plurals are also excluded (`IDs` is ok).
fn is_camel_case(s: &str) -> bool { 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; return false;
} }

View File

@ -165,6 +165,7 @@
LintId::of(methods::FLAT_MAP_IDENTITY), LintId::of(methods::FLAT_MAP_IDENTITY),
LintId::of(methods::INSPECT_FOR_EACH), LintId::of(methods::INSPECT_FOR_EACH),
LintId::of(methods::INTO_ITER_ON_REF), LintId::of(methods::INTO_ITER_ON_REF),
LintId::of(methods::IS_DIGIT_ASCII_RADIX),
LintId::of(methods::ITERATOR_STEP_BY_ZERO), LintId::of(methods::ITERATOR_STEP_BY_ZERO),
LintId::of(methods::ITER_CLONED_COLLECT), LintId::of(methods::ITER_CLONED_COLLECT),
LintId::of(methods::ITER_COUNT), LintId::of(methods::ITER_COUNT),

View File

@ -302,6 +302,7 @@
methods::INEFFICIENT_TO_STRING, methods::INEFFICIENT_TO_STRING,
methods::INSPECT_FOR_EACH, methods::INSPECT_FOR_EACH,
methods::INTO_ITER_ON_REF, methods::INTO_ITER_ON_REF,
methods::IS_DIGIT_ASCII_RADIX,
methods::ITERATOR_STEP_BY_ZERO, methods::ITERATOR_STEP_BY_ZERO,
methods::ITER_CLONED_COLLECT, methods::ITER_CLONED_COLLECT,
methods::ITER_COUNT, methods::ITER_COUNT,

View File

@ -61,6 +61,7 @@
LintId::of(methods::CHARS_NEXT_CMP), LintId::of(methods::CHARS_NEXT_CMP),
LintId::of(methods::ERR_EXPECT), LintId::of(methods::ERR_EXPECT),
LintId::of(methods::INTO_ITER_ON_REF), LintId::of(methods::INTO_ITER_ON_REF),
LintId::of(methods::IS_DIGIT_ASCII_RADIX),
LintId::of(methods::ITER_CLONED_COLLECT), LintId::of(methods::ITER_CLONED_COLLECT),
LintId::of(methods::ITER_NEXT_SLICE), LintId::of(methods::ITER_NEXT_SLICE),
LintId::of(methods::ITER_NTH_ZERO), LintId::of(methods::ITER_NTH_ZERO),

View File

@ -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,
);
}
}

View File

@ -26,6 +26,7 @@
mod inefficient_to_string; mod inefficient_to_string;
mod inspect_for_each; mod inspect_for_each;
mod into_iter_on_ref; mod into_iter_on_ref;
mod is_digit_ascii_radix;
mod iter_cloned_collect; mod iter_cloned_collect;
mod iter_count; mod iter_count;
mod iter_next_slice; mod iter_next_slice;
@ -2131,6 +2132,36 @@
"no-op use of `deref` or `deref_mut` method to `Option`." "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 { pub struct Methods {
avoid_breaking_exported_api: bool, avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>, msrv: Option<RustcVersion>,
@ -2219,6 +2250,7 @@ pub fn new(avoid_breaking_exported_api: bool, msrv: Option<RustcVersion>) -> Sel
UNNECESSARY_JOIN, UNNECESSARY_JOIN,
ERR_EXPECT, ERR_EXPECT,
NEEDLESS_OPTION_AS_DEREF, NEEDLESS_OPTION_AS_DEREF,
IS_DIGIT_ASCII_RADIX,
]); ]);
/// Extracts a method call name, args, and `Span` of the method name. /// 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"), ("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_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_none", []) => check_is_some_is_none(cx, expr, recv, false),
("is_some", []) => check_is_some_is_none(cx, expr, recv, true), ("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
("join", [join_arg]) => { ("join", [join_arg]) => {

View File

@ -361,7 +361,7 @@ fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) {
// See <https://github.com/rust-lang/rust-clippy/issues/4507> for a regression. // See <https://github.com/rust-lang/rust-clippy/issues/4507> for a regression.
// FIXME: Find a better way to detect those cases. // FIXME: Find a better way to detect those cases.
let lit_snip = match snippet_opt(cx, lit.span) { 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, _ => return,
}; };

View File

@ -197,7 +197,7 @@ fn check_ident(&mut self, ident: Ident) {
if interned_name.chars().any(char::is_uppercase) { if interned_name.chars().any(char::is_uppercase) {
return; 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( span_lint(
self.0.cx, self.0.cx,
JUST_UNDERSCORES_AND_DIGITS, JUST_UNDERSCORES_AND_DIGITS,

View File

@ -32,4 +32,5 @@ macro_rules! msrv_aliases {
1,28,0 { FROM_BOOL } 1,28,0 { FROM_BOOL }
1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR } 1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR }
1,16,0 { STR_REPEAT } 1,16,0 { STR_REPEAT }
1,24,0 { IS_ASCII_DIGIT }
} }

View File

@ -57,7 +57,7 @@ pub fn from_lit_kind(src: &'a str, lit_kind: &LitKind) -> Option<NumericLiteral<
.trim_start() .trim_start()
.chars() .chars()
.next() .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 (unsuffixed, suffix) = split_suffix(src, lit_kind);
let float = matches!(lit_kind, LitKind::Float(..)); let float = matches!(lit_kind, LitKind::Float(..));

View File

@ -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);
}

View File

@ -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);
}

View File

@ -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

View File

@ -6,6 +6,6 @@ fn main() {
let c = 'x'; let c = 'x';
let d = &c; let d = &c;
let _ = d.is_digit(10); let _ = d.is_digit(8);
let _ = char::is_digit(c, 10); let _ = char::is_digit(c, 8);
} }

View File

@ -6,6 +6,6 @@ fn main() {
let c = 'x'; let c = 'x';
let d = &c; let d = &c;
let _ = d.to_digit(10).is_some(); let _ = d.to_digit(8).is_some();
let _ = char::to_digit(c, 10).is_some(); let _ = char::to_digit(c, 8).is_some();
} }

View File

@ -1,16 +1,16 @@
error: use of `.to_digit(..).is_some()` error: use of `.to_digit(..).is_some()`
--> $DIR/to_digit_is_some.rs:9:13 --> $DIR/to_digit_is_some.rs:9:13
| |
LL | let _ = d.to_digit(10).is_some(); LL | let _ = d.to_digit(8).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `d.is_digit(10)` | ^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `d.is_digit(8)`
| |
= note: `-D clippy::to-digit-is-some` implied by `-D warnings` = note: `-D clippy::to-digit-is-some` implied by `-D warnings`
error: use of `.to_digit(..).is_some()` error: use of `.to_digit(..).is_some()`
--> $DIR/to_digit_is_some.rs:10:13 --> $DIR/to_digit_is_some.rs:10:13
| |
LL | let _ = char::to_digit(c, 10).is_some(); LL | let _ = char::to_digit(c, 8).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `char::is_digit(c, 10)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `char::is_digit(c, 8)`
error: aborting due to 2 previous errors error: aborting due to 2 previous errors