Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint is_digit_ascii_radix #8624

Merged
merged 1 commit into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/case_sensitive_file_extension_comparisons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
50 changes: 50 additions & 0 deletions clippy_lints/src/methods/is_digit_ascii_radix.rs
Original file line number Diff line number Diff line change
@@ -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,
);
}
}
33 changes: 33 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<RustcVersion>,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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]) => {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/misc_early/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/non_expressive_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
2 changes: 1 addition & 1 deletion clippy_utils/src/numeric_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(..));
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/is_digit_ascii_radix.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
18 changes: 18 additions & 0 deletions tests/ui/is_digit_ascii_radix.rs
Original file line number Diff line number Diff line change
@@ -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);
}
22 changes: 22 additions & 0 deletions tests/ui/is_digit_ascii_radix.stderr
Original file line number Diff line number Diff line change
@@ -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

4 changes: 2 additions & 2 deletions tests/ui/to_digit_is_some.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
4 changes: 2 additions & 2 deletions tests/ui/to_digit_is_some.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
8 changes: 4 additions & 4 deletions tests/ui/to_digit_is_some.stderr
Original file line number Diff line number Diff line change
@@ -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