Skip to content

Commit e4540ad

Browse files
committed
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 <[email protected]> Update clippy_lints/src/manual_is_ascii_check.rs Co-authored-by: Fridtjof Stoldt <[email protected]> Update clippy_lints/src/manual_is_ascii_check.rs Co-authored-by: Fridtjof Stoldt <[email protected]> fix ui test result fix: unnecessary format! chore: apply feedbacks * check msrvs also for const fn * check applicability manually * modify documents
1 parent 7600535 commit e4540ad

File tree

8 files changed

+322
-1
lines changed

8 files changed

+322
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3997,6 +3997,7 @@ Released 2018-09-13
39973997
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
39983998
[`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
39993999
[`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
4000+
[`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
40004001
[`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
40014002
[`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map
40024003
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
251251
crate::manual_bits::MANUAL_BITS_INFO,
252252
crate::manual_clamp::MANUAL_CLAMP_INFO,
253253
crate::manual_instant_elapsed::MANUAL_INSTANT_ELAPSED_INFO,
254+
crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
254255
crate::manual_let_else::MANUAL_LET_ELSE_INFO,
255256
crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
256257
crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ mod manual_async_fn;
173173
mod manual_bits;
174174
mod manual_clamp;
175175
mod manual_instant_elapsed;
176+
mod manual_is_ascii_check;
176177
mod manual_let_else;
177178
mod manual_non_exhaustive;
178179
mod manual_rem_euclid;
@@ -918,6 +919,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
918919
store.register_late_pass(|_| Box::new(missing_trait_methods::MissingTraitMethods));
919920
store.register_late_pass(|_| Box::new(from_raw_with_void_ptr::FromRawWithVoidPtr));
920921
store.register_late_pass(|_| Box::new(suspicious_xor_used_as_pow::ConfusingXorAndPow));
922+
store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv)));
921923
// add lints here, do not remove this comment, it's used in `new_lint`
922924
}
923925

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
use rustc_ast::LitKind::{Byte, Char};
2+
use rustc_errors::Applicability;
3+
use rustc_hir::{Expr, ExprKind, PatKind, RangeEnd};
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_semver::RustcVersion;
6+
use rustc_session::{declare_tool_lint, impl_lint_pass};
7+
use rustc_span::{def_id::DefId, sym};
8+
9+
use clippy_utils::{
10+
diagnostics::span_lint_and_sugg, in_constant, macros::root_macro_call, meets_msrv, msrvs, source::snippet,
11+
};
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Suggests to use dedicated built-in methods,
16+
/// `is_ascii_(lowercase|uppercase|digit)` for checking on corresponding ascii range
17+
///
18+
/// ### Why is this bad?
19+
/// Using the built-in functions is more readable and makes it
20+
/// clear that it's not a specific subset of characters, but all
21+
/// ASCII (lowercase|uppercase|digit) characters.
22+
/// ### Example
23+
/// ```rust
24+
/// fn main() {
25+
/// assert!(matches!('x', 'a'..='z'));
26+
/// assert!(matches!(b'X', b'A'..=b'Z'));
27+
/// assert!(matches!('2', '0'..='9'));
28+
/// assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
29+
/// }
30+
/// ```
31+
/// Use instead:
32+
/// ```rust
33+
/// fn main() {
34+
/// assert!('x'.is_ascii_lowercase());
35+
/// assert!(b'X'.is_ascii_uppercase());
36+
/// assert!('2'.is_ascii_digit());
37+
/// assert!('x'.is_ascii_alphabetic());
38+
/// }
39+
/// ```
40+
#[clippy::version = "1.66.0"]
41+
pub MANUAL_IS_ASCII_CHECK,
42+
style,
43+
"use dedicated method to check ascii range"
44+
}
45+
impl_lint_pass!(ManualIsAsciiCheck => [MANUAL_IS_ASCII_CHECK]);
46+
47+
pub struct ManualIsAsciiCheck {
48+
msrv: Option<RustcVersion>,
49+
}
50+
51+
impl ManualIsAsciiCheck {
52+
#[must_use]
53+
pub fn new(msrv: Option<RustcVersion>) -> Self {
54+
Self { msrv }
55+
}
56+
}
57+
58+
#[derive(Debug, PartialEq)]
59+
enum CharRange {
60+
/// 'a'..='z' | b'a'..=b'z'
61+
LowerChar,
62+
/// 'A'..='Z' | b'A'..=b'Z'
63+
UpperChar,
64+
/// AsciiLower | AsciiUpper
65+
FullChar,
66+
/// '0..=9'
67+
Digit,
68+
Otherwise,
69+
}
70+
71+
impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
72+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
73+
if !meets_msrv(self.msrv, msrvs::IS_ASCII_DIGIT) {
74+
return;
75+
}
76+
77+
if in_constant(cx, expr.hir_id) && !meets_msrv(self.msrv, msrvs::IS_ASCII_DIGIT_CONST) {
78+
return;
79+
}
80+
81+
let Some(macro_call) = root_macro_call(expr.span) else { return };
82+
83+
if is_matches_macro(cx, macro_call.def_id) {
84+
if let ExprKind::Match(recv, [arm, ..], _) = expr.kind {
85+
let range = check_pat(&arm.pat.kind);
86+
87+
if let Some(sugg) = match range {
88+
CharRange::UpperChar => Some("is_ascii_uppercase"),
89+
CharRange::LowerChar => Some("is_ascii_lowercase"),
90+
CharRange::FullChar => Some("is_ascii_alphabetic"),
91+
CharRange::Digit => Some("is_ascii_digit"),
92+
CharRange::Otherwise => None,
93+
} {
94+
let mut applicability = Applicability::MaybeIncorrect;
95+
let default_snip = "..";
96+
// `snippet_with_applicability` may set applicability to `MaybeIncorrect` for
97+
// macro span, so we check applicability manually by comaring `recv` is not default.
98+
let recv = snippet(cx, recv.span, default_snip);
99+
100+
if recv != default_snip {
101+
applicability = Applicability::MachineApplicable;
102+
}
103+
104+
span_lint_and_sugg(
105+
cx,
106+
MANUAL_IS_ASCII_CHECK,
107+
macro_call.span,
108+
"manual check for common ascii range",
109+
"try",
110+
format!("{recv}.{sugg}()"),
111+
applicability,
112+
);
113+
}
114+
}
115+
}
116+
}
117+
118+
extract_msrv_attr!(LateContext);
119+
}
120+
121+
fn check_pat(pat_kind: &PatKind<'_>) -> CharRange {
122+
match pat_kind {
123+
PatKind::Or(pats) => {
124+
let ranges = pats.iter().map(|p| check_pat(&p.kind)).collect::<Vec<_>>();
125+
126+
if ranges.len() == 2 && ranges.contains(&CharRange::UpperChar) && ranges.contains(&CharRange::LowerChar) {
127+
CharRange::FullChar
128+
} else {
129+
CharRange::Otherwise
130+
}
131+
},
132+
PatKind::Range(Some(start), Some(end), kind) if *kind == RangeEnd::Included => check_range(start, end),
133+
_ => CharRange::Otherwise,
134+
}
135+
}
136+
137+
fn check_range(start: &Expr<'_>, end: &Expr<'_>) -> CharRange {
138+
if let ExprKind::Lit(start_lit) = &start.kind
139+
&& let ExprKind::Lit(end_lit) = &end.kind {
140+
match (&start_lit.node, &end_lit.node) {
141+
(Char('a'), Char('z')) | (Byte(b'a'), Byte(b'z')) => CharRange::LowerChar,
142+
(Char('A'), Char('Z')) | (Byte(b'A'), Byte(b'Z')) => CharRange::UpperChar,
143+
(Char('0'), Char('9')) | (Byte(b'0'), Byte(b'9')) => CharRange::Digit,
144+
_ => CharRange::Otherwise,
145+
}
146+
} else {
147+
CharRange::Otherwise
148+
}
149+
}
150+
151+
fn is_matches_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool {
152+
if let Some(name) = cx.tcx.get_diagnostic_name(macro_def_id) {
153+
return sym::matches_macro == name;
154+
}
155+
156+
false
157+
}

clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ msrv_aliases! {
1919
1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
2020
1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS }
2121
1,50,0 { BOOL_THEN, CLAMP }
22-
1,47,0 { TAU }
22+
1,47,0 { TAU, IS_ASCII_DIGIT_CONST }
2323
1,46,0 { CONST_IF_MATCH }
2424
1,45,0 { STR_STRIP_PREFIX }
2525
1,43,0 { LOG2_10, LOG10_2 }

tests/ui/manual_is_ascii_check.fixed

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// run-rustfix
2+
3+
#![feature(custom_inner_attributes)]
4+
#![allow(unused, dead_code)]
5+
#![warn(clippy::manual_is_ascii_check)]
6+
7+
fn main() {
8+
assert!('x'.is_ascii_lowercase());
9+
assert!('X'.is_ascii_uppercase());
10+
assert!(b'x'.is_ascii_lowercase());
11+
assert!(b'X'.is_ascii_uppercase());
12+
13+
let num = '2';
14+
assert!(num.is_ascii_digit());
15+
assert!(b'1'.is_ascii_digit());
16+
assert!('x'.is_ascii_alphabetic());
17+
18+
assert!(matches!('x', 'A'..='Z' | 'a'..='z' | '_'));
19+
}
20+
21+
fn msrv_1_23() {
22+
#![clippy::msrv = "1.23"]
23+
24+
assert!(matches!(b'1', b'0'..=b'9'));
25+
assert!(matches!('X', 'A'..='Z'));
26+
assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
27+
}
28+
29+
fn msrv_1_24() {
30+
#![clippy::msrv = "1.24"]
31+
32+
assert!(b'1'.is_ascii_digit());
33+
assert!('X'.is_ascii_uppercase());
34+
assert!('x'.is_ascii_alphabetic());
35+
}
36+
37+
fn msrv_1_46() {
38+
#![clippy::msrv = "1.46"]
39+
const FOO: bool = matches!('x', '0'..='9');
40+
}
41+
42+
fn msrv_1_47() {
43+
#![clippy::msrv = "1.47"]
44+
const FOO: bool = 'x'.is_ascii_digit();
45+
}

tests/ui/manual_is_ascii_check.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// run-rustfix
2+
3+
#![feature(custom_inner_attributes)]
4+
#![allow(unused, dead_code)]
5+
#![warn(clippy::manual_is_ascii_check)]
6+
7+
fn main() {
8+
assert!(matches!('x', 'a'..='z'));
9+
assert!(matches!('X', 'A'..='Z'));
10+
assert!(matches!(b'x', b'a'..=b'z'));
11+
assert!(matches!(b'X', b'A'..=b'Z'));
12+
13+
let num = '2';
14+
assert!(matches!(num, '0'..='9'));
15+
assert!(matches!(b'1', b'0'..=b'9'));
16+
assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
17+
18+
assert!(matches!('x', 'A'..='Z' | 'a'..='z' | '_'));
19+
}
20+
21+
fn msrv_1_23() {
22+
#![clippy::msrv = "1.23"]
23+
24+
assert!(matches!(b'1', b'0'..=b'9'));
25+
assert!(matches!('X', 'A'..='Z'));
26+
assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
27+
}
28+
29+
fn msrv_1_24() {
30+
#![clippy::msrv = "1.24"]
31+
32+
assert!(matches!(b'1', b'0'..=b'9'));
33+
assert!(matches!('X', 'A'..='Z'));
34+
assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
35+
}
36+
37+
fn msrv_1_46() {
38+
#![clippy::msrv = "1.46"]
39+
const FOO: bool = matches!('x', '0'..='9');
40+
}
41+
42+
fn msrv_1_47() {
43+
#![clippy::msrv = "1.47"]
44+
const FOO: bool = matches!('x', '0'..='9');
45+
}

tests/ui/manual_is_ascii_check.stderr

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
error: manual check for common ascii range
2+
--> $DIR/manual_is_ascii_check.rs:8:13
3+
|
4+
LL | assert!(matches!('x', 'a'..='z'));
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_lowercase()`
6+
|
7+
= note: `-D clippy::manual-is-ascii-check` implied by `-D warnings`
8+
9+
error: manual check for common ascii range
10+
--> $DIR/manual_is_ascii_check.rs:9:13
11+
|
12+
LL | assert!(matches!('X', 'A'..='Z'));
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'X'.is_ascii_uppercase()`
14+
15+
error: manual check for common ascii range
16+
--> $DIR/manual_is_ascii_check.rs:10:13
17+
|
18+
LL | assert!(matches!(b'x', b'a'..=b'z'));
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'x'.is_ascii_lowercase()`
20+
21+
error: manual check for common ascii range
22+
--> $DIR/manual_is_ascii_check.rs:11:13
23+
|
24+
LL | assert!(matches!(b'X', b'A'..=b'Z'));
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'X'.is_ascii_uppercase()`
26+
27+
error: manual check for common ascii range
28+
--> $DIR/manual_is_ascii_check.rs:14:13
29+
|
30+
LL | assert!(matches!(num, '0'..='9'));
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.is_ascii_digit()`
32+
33+
error: manual check for common ascii range
34+
--> $DIR/manual_is_ascii_check.rs:15:13
35+
|
36+
LL | assert!(matches!(b'1', b'0'..=b'9'));
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'1'.is_ascii_digit()`
38+
39+
error: manual check for common ascii range
40+
--> $DIR/manual_is_ascii_check.rs:16:13
41+
|
42+
LL | assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_alphabetic()`
44+
45+
error: manual check for common ascii range
46+
--> $DIR/manual_is_ascii_check.rs:32:13
47+
|
48+
LL | assert!(matches!(b'1', b'0'..=b'9'));
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'1'.is_ascii_digit()`
50+
51+
error: manual check for common ascii range
52+
--> $DIR/manual_is_ascii_check.rs:33:13
53+
|
54+
LL | assert!(matches!('X', 'A'..='Z'));
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'X'.is_ascii_uppercase()`
56+
57+
error: manual check for common ascii range
58+
--> $DIR/manual_is_ascii_check.rs:34:13
59+
|
60+
LL | assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_alphabetic()`
62+
63+
error: manual check for common ascii range
64+
--> $DIR/manual_is_ascii_check.rs:44:23
65+
|
66+
LL | const FOO: bool = matches!('x', '0'..='9');
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_digit()`
68+
69+
error: aborting due to 11 previous errors
70+

0 commit comments

Comments
 (0)