Auto merge of #10053 - naosense:fix_9933, r=xFrednet

improve `manual_is_ascii_check ` check

Sorry, not familiar the api, i can only check the method name of expression `<expr-1>.contains(<expr-2>)` after read clippy book and hints from #9933 . i dont know how to check
1.  if <expr-1> is a specific range
2. <expr-2> is a character

r? `@xFrednet` could you please provide some more hints? 😝️

---

changelog: Enhancement: [`manual_is_ascii_check`]: Now detects ranges with `.contains()` calls
[#10053](https://github.com/rust-lang/rust-clippy/pull/10053)
<!-- changelog_checked -->
This commit is contained in:
bors 2022-12-14 13:48:53 +00:00
commit be15e60d00
4 changed files with 141 additions and 40 deletions

View file

@ -1,11 +1,12 @@
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::{diagnostics::span_lint_and_sugg, in_constant, macros::root_macro_call, source::snippet};
use clippy_utils::{diagnostics::span_lint_and_sugg, higher, in_constant, macros::root_macro_call, source::snippet};
use rustc_ast::ast::RangeLimits;
use rustc_ast::LitKind::{Byte, Char};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, PatKind, RangeEnd};
use rustc_hir::{BorrowKind, Expr, ExprKind, PatKind, RangeEnd};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{def_id::DefId, sym};
use rustc_span::{def_id::DefId, sym, Span};
declare_clippy_lint! {
/// ### What it does
@ -23,6 +24,10 @@ declare_clippy_lint! {
/// assert!(matches!(b'X', b'A'..=b'Z'));
/// assert!(matches!('2', '0'..='9'));
/// assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
///
/// ('0'..='9').contains(&'0');
/// ('a'..='z').contains(&'a');
/// ('A'..='Z').contains(&'A');
/// }
/// ```
/// Use instead:
@ -32,6 +37,10 @@ declare_clippy_lint! {
/// assert!(b'X'.is_ascii_uppercase());
/// assert!('2'.is_ascii_digit());
/// assert!('x'.is_ascii_alphabetic());
///
/// '0'.is_ascii_digit();
/// 'a'.is_ascii_lowercase();
/// 'A'.is_ascii_uppercase();
/// }
/// ```
#[clippy::version = "1.66.0"]
@ -75,40 +84,21 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
return;
}
let Some(macro_call) = root_macro_call(expr.span) else { return };
if is_matches_macro(cx, macro_call.def_id) {
if let Some(macro_call) = root_macro_call(expr.span)
&& 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 default_snip = "..";
// `snippet_with_applicability` may set applicability to `MaybeIncorrect` for
// macro span, so we check applicability manually by comparing `recv` is not default.
let recv = snippet(cx, recv.span, default_snip);
let applicability = if recv == default_snip {
Applicability::HasPlaceholders
} else {
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,
);
}
check_is_ascii(cx, macro_call.span, recv, &range);
}
} else if let ExprKind::MethodCall(path, receiver, [arg], ..) = expr.kind
&& path.ident.name == sym!(contains)
&& let Some(higher::Range { start: Some(start), end: Some(end), limits: RangeLimits::Closed })
= higher::Range::hir(receiver) {
let range = check_range(start, end);
if let ExprKind::AddrOf(BorrowKind::Ref, _, e) = arg.kind {
check_is_ascii(cx, expr.span, e, &range);
} else {
check_is_ascii(cx, expr.span, arg, &range);
}
}
}
@ -116,6 +106,37 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
extract_msrv_attr!(LateContext);
}
fn check_is_ascii(cx: &LateContext<'_>, span: Span, recv: &Expr<'_>, range: &CharRange) {
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 default_snip = "..";
// `snippet_with_applicability` may set applicability to `MaybeIncorrect` for
// macro span, so we check applicability manually by comparing `recv` is not default.
let recv = snippet(cx, recv.span, default_snip);
let applicability = if recv == default_snip {
Applicability::HasPlaceholders
} else {
Applicability::MachineApplicable
};
span_lint_and_sugg(
cx,
MANUAL_IS_ASCII_CHECK,
span,
"manual check for common ascii range",
"try",
format!("{recv}.{sugg}()"),
applicability,
);
}
}
fn check_pat(pat_kind: &PatKind<'_>) -> CharRange {
match pat_kind {
PatKind::Or(pats) => {

View file

@ -15,6 +15,19 @@ fn main() {
assert!('x'.is_ascii_alphabetic());
assert!(matches!('x', 'A'..='Z' | 'a'..='z' | '_'));
b'0'.is_ascii_digit();
b'a'.is_ascii_lowercase();
b'A'.is_ascii_uppercase();
'0'.is_ascii_digit();
'a'.is_ascii_lowercase();
'A'.is_ascii_uppercase();
let cool_letter = &'g';
cool_letter.is_ascii_digit();
cool_letter.is_ascii_lowercase();
cool_letter.is_ascii_uppercase();
}
#[clippy::msrv = "1.23"]

View file

@ -15,6 +15,19 @@ fn main() {
assert!(matches!('x', 'A'..='Z' | 'a'..='z'));
assert!(matches!('x', 'A'..='Z' | 'a'..='z' | '_'));
(b'0'..=b'9').contains(&b'0');
(b'a'..=b'z').contains(&b'a');
(b'A'..=b'Z').contains(&b'A');
('0'..='9').contains(&'0');
('a'..='z').contains(&'a');
('A'..='Z').contains(&'A');
let cool_letter = &'g';
('0'..='9').contains(cool_letter);
('a'..='z').contains(cool_letter);
('A'..='Z').contains(cool_letter);
}
#[clippy::msrv = "1.23"]

View file

@ -43,28 +43,82 @@ 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:29:13
--> $DIR/manual_is_ascii_check.rs:19:5
|
LL | (b'0'..=b'9').contains(&b'0');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'0'.is_ascii_digit()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:20:5
|
LL | (b'a'..=b'z').contains(&b'a');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'a'.is_ascii_lowercase()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:21:5
|
LL | (b'A'..=b'Z').contains(&b'A');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'A'.is_ascii_uppercase()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:23:5
|
LL | ('0'..='9').contains(&'0');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'0'.is_ascii_digit()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:24:5
|
LL | ('a'..='z').contains(&'a');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'a'.is_ascii_lowercase()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:25:5
|
LL | ('A'..='Z').contains(&'A');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'A'.is_ascii_uppercase()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:28:5
|
LL | ('0'..='9').contains(cool_letter);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cool_letter.is_ascii_digit()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:29:5
|
LL | ('a'..='z').contains(cool_letter);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cool_letter.is_ascii_lowercase()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:30:5
|
LL | ('A'..='Z').contains(cool_letter);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cool_letter.is_ascii_uppercase()`
error: manual check for common ascii range
--> $DIR/manual_is_ascii_check.rs:42: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:30:13
--> $DIR/manual_is_ascii_check.rs:43: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:31:13
--> $DIR/manual_is_ascii_check.rs:44: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:41:23
--> $DIR/manual_is_ascii_check.rs:54:23
|
LL | const FOO: bool = matches!('x', '0'..='9');
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_digit()`
error: aborting due to 11 previous errors
error: aborting due to 20 previous errors