From e4540ad65fa14ceebd5145ab771fe4918d170bf1 Mon Sep 17 00:00:00 2001 From: koka Date: Mon, 24 Oct 2022 23:49:59 +0900 Subject: [PATCH] feat: implement manual_is_ascii_check lint modify fix: allow unused in test code fix: types in doc comment Update clippy_lints/src/manual_is_ascii_check.rs Co-authored-by: Fridtjof Stoldt Update clippy_lints/src/manual_is_ascii_check.rs Co-authored-by: Fridtjof Stoldt Update clippy_lints/src/manual_is_ascii_check.rs Co-authored-by: Fridtjof Stoldt fix ui test result fix: unnecessary format! chore: apply feedbacks * check msrvs also for const fn * check applicability manually * modify documents --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/manual_is_ascii_check.rs | 157 ++++++++++++++++++++++ clippy_utils/src/msrvs.rs | 2 +- tests/ui/manual_is_ascii_check.fixed | 45 +++++++ tests/ui/manual_is_ascii_check.rs | 45 +++++++ tests/ui/manual_is_ascii_check.stderr | 70 ++++++++++ 8 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/manual_is_ascii_check.rs create mode 100644 tests/ui/manual_is_ascii_check.fixed create mode 100644 tests/ui/manual_is_ascii_check.rs create mode 100644 tests/ui/manual_is_ascii_check.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 35864e856..d75ef52e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3997,6 +3997,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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index f4fc2bd33..448b12f1b 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -251,6 +251,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ 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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 197d8f2dc..fe05f2a13 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -173,6 +173,7 @@ mod manual_async_fn; 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; @@ -918,6 +919,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` } diff --git a/clippy_lints/src/manual_is_ascii_check.rs b/clippy_lints/src/manual_is_ascii_check.rs new file mode 100644 index 000000000..3a6b693f7 --- /dev/null +++ b/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, +} + +impl ManualIsAsciiCheck { + #[must_use] + pub fn new(msrv: Option) -> 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::>(); + + 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 +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 9a672e2dd..79b19e6fb 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -19,7 +19,7 @@ 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 } diff --git a/tests/ui/manual_is_ascii_check.fixed b/tests/ui/manual_is_ascii_check.fixed new file mode 100644 index 000000000..765bb7859 --- /dev/null +++ b/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(); +} diff --git a/tests/ui/manual_is_ascii_check.rs b/tests/ui/manual_is_ascii_check.rs new file mode 100644 index 000000000..be1331610 --- /dev/null +++ b/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'); +} diff --git a/tests/ui/manual_is_ascii_check.stderr b/tests/ui/manual_is_ascii_check.stderr new file mode 100644 index 000000000..c0a9d4db1 --- /dev/null +++ b/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 +