Skip to content

Commit c71f0be

Browse files
committed
Auto merge of #13334 - nyurik:ascii-str-eq, r=xFrednet
Add manual_ignore_cast_cmp lint ```rust // bad fn compare(a: &str, b: &str) -> bool { a.to_ascii_lowercase() == b.to_ascii_lowercase() || a.to_ascii_lowercase() == "abc" } // good fn compare(a: &str, b: &str) -> bool { a.eq_ignore_ascii_case(b) || a.eq_ignore_ascii_case("abc") } ``` - [x] Followed [lint naming conventions][lint_naming] - [x] Added passing UI tests (including committed `.stderr` file) - [x] `cargo test` passes locally - [x] Executed `cargo dev update_lints` - [x] Added lint documentation - [x] Run `cargo dev fmt` changelog: New lint: [`manual_ignore_case_cmp`] `perf` [#13334](rust-lang/rust-clippy#13334) Closes#13204
2 parents 55f6029 + d1dec19 commit c71f0be

7 files changed

Lines changed: 891 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5621,6 +5621,7 @@ Released 2018-09-13
56215621
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
56225622
[`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
56235623
[`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one
5624+
[`manual_ignore_case_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ignore_case_cmp
56245625
[`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
56255626
[`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
56265627
[`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
306306
crate::manual_float_methods::MANUAL_IS_FINITE_INFO,
307307
crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
308308
crate::manual_hash_one::MANUAL_HASH_ONE_INFO,
309+
crate::manual_ignore_case_cmp::MANUAL_IGNORE_CASE_CMP_INFO,
309310
crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
310311
crate::manual_is_power_of_two::MANUAL_IS_POWER_OF_TWO_INFO,
311312
crate::manual_let_else::MANUAL_LET_ELSE_INFO,

‎clippy_lints/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ mod manual_clamp;
208208
mod manual_div_ceil;
209209
mod manual_float_methods;
210210
mod manual_hash_one;
211+
mod manual_ignore_case_cmp;
211212
mod manual_is_ascii_check;
212213
mod manual_is_power_of_two;
213214
mod manual_let_else;
@@ -947,5 +948,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
947948
store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
948949
store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
949950
store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
951+
store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
950952
// add lints here, do not remove this comment, it's used in `new_lint`
951953
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
usecrate::manual_ignore_case_cmp::MatchType::{Literal,ToAscii};
2+
use clippy_utils::diagnostics::span_lint_and_then;
3+
use clippy_utils::source::snippet_with_applicability;
4+
use clippy_utils::ty::{get_type_diagnostic_name, is_type_diagnostic_item, is_type_lang_item};
5+
use rustc_ast::LitKind;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::ExprKind::{Binary,Lit,MethodCall};
8+
use rustc_hir::{BinOpKind,Expr,LangItem};
9+
use rustc_lint::{LateContext,LateLintPass};
10+
use rustc_middle::ty;
11+
use rustc_middle::ty::{Ty,UintTy};
12+
use rustc_session::declare_lint_pass;
13+
use rustc_span::{Span, sym};
14+
15+
declare_clippy_lint!{
16+
/// ### What it does
17+
/// Checks for manual case-insensitive ASCII comparison.
18+
///
19+
/// ### Why is this bad?
20+
/// The `eq_ignore_ascii_case` method is faster because it does not allocate
21+
/// memory for the new strings, and it is more readable.
22+
///
23+
/// ### Example
24+
/// ```no_run
25+
/// fn compare(a: &str, b: &str) -> bool {
26+
/// a.to_ascii_lowercase() == b.to_ascii_lowercase() || a.to_ascii_lowercase() == "abc"
27+
/// }
28+
/// ```
29+
/// Use instead:
30+
/// ```no_run
31+
/// fn compare(a: &str, b: &str) -> bool {
32+
/// a.eq_ignore_ascii_case(b) || a.eq_ignore_ascii_case("abc")
33+
/// }
34+
/// ```
35+
#[clippy::version = "1.82.0"]
36+
pubMANUAL_IGNORE_CASE_CMP,
37+
perf,
38+
"manual case-insensitive ASCII comparison"
39+
}
40+
41+
declare_lint_pass!(ManualIgnoreCaseCmp => [MANUAL_IGNORE_CASE_CMP]);
42+
43+
enumMatchType<'a,'b>{
44+
ToAscii(bool,Ty<'a>),
45+
Literal(&'bLitKind),
46+
}
47+
48+
fnget_ascii_type<'a,'b>(cx:&LateContext<'a>,kind: rustc_hir::ExprKind<'b>) -> Option<(Span,MatchType<'a,'b>)>{
49+
ifletMethodCall(path, expr, _, _) = kind {
50+
let is_lower = match path.ident.name.as_str(){
51+
"to_ascii_lowercase" => true,
52+
"to_ascii_uppercase" => false,
53+
_ => returnNone,
54+
};
55+
let ty_raw = cx.typeck_results().expr_ty(expr);
56+
let ty = ty_raw.peel_refs();
57+
ifneeds_ref_to_cmp(cx, ty)
58+
|| ty.is_str()
59+
|| ty.is_slice()
60+
|| matches!(get_type_diagnostic_name(cx, ty),Some(sym::OsStr | sym::OsString))
61+
{
62+
returnSome((expr.span,ToAscii(is_lower, ty_raw)));
63+
}
64+
}elseifletLit(expr) = kind {
65+
returnSome((expr.span,Literal(&expr.node)));
66+
}
67+
None
68+
}
69+
70+
/// Returns true if the type needs to be dereferenced to be compared
71+
fnneeds_ref_to_cmp(cx:&LateContext<'_>,ty:Ty<'_>) -> bool{
72+
ty.is_char()
73+
|| *ty.kind() == ty::Uint(UintTy::U8)
74+
|| is_type_diagnostic_item(cx, ty, sym::Vec)
75+
|| is_type_lang_item(cx, ty,LangItem::String)
76+
}
77+
78+
implLateLintPass<'_>forManualIgnoreCaseCmp{
79+
fncheck_expr(&mutself,cx:&LateContext<'_>,expr:&'_Expr<'_>){
80+
// check if expression represents a comparison of two strings
81+
// using .to_ascii_lowercase() or .to_ascii_uppercase() methods,
82+
// or one of the sides is a literal
83+
// Offer to replace it with .eq_ignore_ascii_case() method
84+
ifletBinary(op, left, right) = &expr.kind
85+
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
86+
&& letSome((left_span, left_val)) = get_ascii_type(cx, left.kind)
87+
&& letSome((right_span, right_val)) = get_ascii_type(cx, right.kind)
88+
&& match(&left_val,&right_val){
89+
(ToAscii(l_lower, ..),ToAscii(r_lower, ..))if l_lower == r_lower => true,
90+
(ToAscii(..),Literal(..)) | (Literal(..),ToAscii(..)) => true,
91+
_ => false,
92+
}
93+
{
94+
let deref = match right_val {
95+
ToAscii(_, ty)ifneeds_ref_to_cmp(cx, ty) => "&",
96+
ToAscii(..) => "",
97+
Literal(ty) => {
98+
ifletLitKind::Char(_) | LitKind::Byte(_) = ty {
99+
"&"
100+
}else{
101+
""
102+
}
103+
},
104+
};
105+
let neg = if op.node == BinOpKind::Ne{"!"}else{""};
106+
span_lint_and_then(
107+
cx,
108+
MANUAL_IGNORE_CASE_CMP,
109+
expr.span,
110+
"manual case-insensitive ASCII comparison",
111+
|diag| {
112+
letmut app = Applicability::MachineApplicable;
113+
diag.span_suggestion_verbose(
114+
expr.span,
115+
"consider using `.eq_ignore_ascii_case()` instead",
116+
format!(
117+
"{neg}{}.eq_ignore_ascii_case({deref}{})",
118+
snippet_with_applicability(cx, left_span,"_",&mut app),
119+
snippet_with_applicability(cx, right_span,"_",&mut app)
120+
),
121+
app,
122+
);
123+
},
124+
);
125+
}
126+
}
127+
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
#![allow(clippy::all)]
2+
#![deny(clippy::manual_ignore_case_cmp)]
3+
4+
use std::ffi::{OsStr, OsString};
5+
6+
fn main() {}
7+
8+
fn variants(a: &str, b: &str) {
9+
if a.eq_ignore_ascii_case(b) {
10+
return;
11+
}
12+
if a.eq_ignore_ascii_case(b) {
13+
return;
14+
}
15+
let r = a.eq_ignore_ascii_case(b);
16+
let r = r || a.eq_ignore_ascii_case(b);
17+
r && a.eq_ignore_ascii_case(&b.to_uppercase());
18+
// !=
19+
if !a.eq_ignore_ascii_case(b) {
20+
return;
21+
}
22+
if !a.eq_ignore_ascii_case(b) {
23+
return;
24+
}
25+
let r = !a.eq_ignore_ascii_case(b);
26+
let r = r || !a.eq_ignore_ascii_case(b);
27+
r && !a.eq_ignore_ascii_case(&b.to_uppercase());
28+
}
29+
30+
fn unsupported(a: char, b: char) {
31+
// TODO:: these are rare, and might not be worth supporting
32+
a.to_ascii_lowercase() == char::to_ascii_lowercase(&b);
33+
char::to_ascii_lowercase(&a) == b.to_ascii_lowercase();
34+
char::to_ascii_lowercase(&a) == char::to_ascii_lowercase(&b);
35+
}
36+
37+
fn char(a: char, b: char) {
38+
a.eq_ignore_ascii_case(&b);
39+
a.to_ascii_lowercase() == *&b.to_ascii_lowercase();
40+
*&a.to_ascii_lowercase() == b.to_ascii_lowercase();
41+
a.eq_ignore_ascii_case(&'a');
42+
'a'.eq_ignore_ascii_case(&b);
43+
}
44+
fn u8(a: u8, b: u8) {
45+
a.eq_ignore_ascii_case(&b);
46+
a.eq_ignore_ascii_case(&b'a');
47+
b'a'.eq_ignore_ascii_case(&b);
48+
}
49+
fn ref_str(a: &str, b: &str) {
50+
a.eq_ignore_ascii_case(b);
51+
a.to_uppercase().eq_ignore_ascii_case(b);
52+
a.eq_ignore_ascii_case("a");
53+
"a".eq_ignore_ascii_case(b);
54+
}
55+
fn ref_ref_str(a: &&str, b: &&str) {
56+
a.eq_ignore_ascii_case(b);
57+
a.to_uppercase().eq_ignore_ascii_case(b);
58+
a.eq_ignore_ascii_case("a");
59+
"a".eq_ignore_ascii_case(b);
60+
}
61+
fn string(a: String, b: String) {
62+
a.eq_ignore_ascii_case(&b);
63+
a.eq_ignore_ascii_case("a");
64+
"a".eq_ignore_ascii_case(&b);
65+
&a.to_ascii_lowercase() == &b.to_ascii_lowercase();
66+
&&a.to_ascii_lowercase() == &&b.to_ascii_lowercase();
67+
a.eq_ignore_ascii_case("a");
68+
"a".eq_ignore_ascii_case(&b);
69+
}
70+
fn ref_string(a: String, b: &String) {
71+
a.eq_ignore_ascii_case(b);
72+
a.eq_ignore_ascii_case("a");
73+
"a".eq_ignore_ascii_case(b);
74+
75+
b.eq_ignore_ascii_case(&a);
76+
b.eq_ignore_ascii_case("a");
77+
"a".eq_ignore_ascii_case(&a);
78+
}
79+
fn string_ref_str(a: String, b: &str) {
80+
a.eq_ignore_ascii_case(b);
81+
a.eq_ignore_ascii_case("a");
82+
"a".eq_ignore_ascii_case(b);
83+
84+
b.eq_ignore_ascii_case(&a);
85+
b.eq_ignore_ascii_case("a");
86+
"a".eq_ignore_ascii_case(&a);
87+
}
88+
fn ref_u8slice(a: &[u8], b: &[u8]) {
89+
a.eq_ignore_ascii_case(b);
90+
}
91+
fn u8vec(a: Vec<u8>, b: Vec<u8>) {
92+
a.eq_ignore_ascii_case(&b);
93+
}
94+
fn ref_u8vec(a: Vec<u8>, b: &Vec<u8>) {
95+
a.eq_ignore_ascii_case(b);
96+
b.eq_ignore_ascii_case(&a);
97+
}
98+
fn ref_osstr(a: &OsStr, b: &OsStr) {
99+
a.eq_ignore_ascii_case(b);
100+
}
101+
fn osstring(a: OsString, b: OsString) {
102+
a.eq_ignore_ascii_case(b);
103+
}
104+
fn ref_osstring(a: OsString, b: &OsString) {
105+
a.eq_ignore_ascii_case(b);
106+
b.eq_ignore_ascii_case(a);
107+
}

‎tests/ui/manual_ignore_case_cmp.rs‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
#![allow(clippy::all)]
2+
#![deny(clippy::manual_ignore_case_cmp)]
3+
4+
use std::ffi::{OsStr,OsString};
5+
6+
fnmain(){}
7+
8+
fnvariants(a:&str,b:&str){
9+
if a.to_ascii_lowercase() == b.to_ascii_lowercase(){
10+
return;
11+
}
12+
if a.to_ascii_uppercase() == b.to_ascii_uppercase(){
13+
return;
14+
}
15+
let r = a.to_ascii_lowercase() == b.to_ascii_lowercase();
16+
let r = r || a.to_ascii_uppercase() == b.to_ascii_uppercase();
17+
r && a.to_ascii_lowercase() == b.to_uppercase().to_ascii_lowercase();
18+
// !=
19+
if a.to_ascii_lowercase() != b.to_ascii_lowercase(){
20+
return;
21+
}
22+
if a.to_ascii_uppercase() != b.to_ascii_uppercase(){
23+
return;
24+
}
25+
let r = a.to_ascii_lowercase() != b.to_ascii_lowercase();
26+
let r = r || a.to_ascii_uppercase() != b.to_ascii_uppercase();
27+
r && a.to_ascii_lowercase() != b.to_uppercase().to_ascii_lowercase();
28+
}
29+
30+
fnunsupported(a:char,b:char){
31+
// TODO:: these are rare, and might not be worth supporting
32+
a.to_ascii_lowercase() == char::to_ascii_lowercase(&b);
33+
char::to_ascii_lowercase(&a) == b.to_ascii_lowercase();
34+
char::to_ascii_lowercase(&a) == char::to_ascii_lowercase(&b);
35+
}
36+
37+
fnchar(a:char,b:char){
38+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
39+
a.to_ascii_lowercase() == *&b.to_ascii_lowercase();
40+
*&a.to_ascii_lowercase() == b.to_ascii_lowercase();
41+
a.to_ascii_lowercase() == 'a';
42+
'a' == b.to_ascii_lowercase();
43+
}
44+
fnu8(a:u8,b:u8){
45+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
46+
a.to_ascii_lowercase() == b'a';
47+
b'a' == b.to_ascii_lowercase();
48+
}
49+
fnref_str(a:&str,b:&str){
50+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
51+
a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase();
52+
a.to_ascii_lowercase() == "a";
53+
"a" == b.to_ascii_lowercase();
54+
}
55+
fnref_ref_str(a:&&str,b:&&str){
56+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
57+
a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase();
58+
a.to_ascii_lowercase() == "a";
59+
"a" == b.to_ascii_lowercase();
60+
}
61+
fnstring(a:String,b:String){
62+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
63+
a.to_ascii_lowercase() == "a";
64+
"a" == b.to_ascii_lowercase();
65+
&a.to_ascii_lowercase() == &b.to_ascii_lowercase();
66+
&&a.to_ascii_lowercase() == &&b.to_ascii_lowercase();
67+
a.to_ascii_lowercase() == "a";
68+
"a" == b.to_ascii_lowercase();
69+
}
70+
fnref_string(a:String,b:&String){
71+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
72+
a.to_ascii_lowercase() == "a";
73+
"a" == b.to_ascii_lowercase();
74+
75+
b.to_ascii_lowercase() == a.to_ascii_lowercase();
76+
b.to_ascii_lowercase() == "a";
77+
"a" == a.to_ascii_lowercase();
78+
}
79+
fnstring_ref_str(a:String,b:&str){
80+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
81+
a.to_ascii_lowercase() == "a";
82+
"a" == b.to_ascii_lowercase();
83+
84+
b.to_ascii_lowercase() == a.to_ascii_lowercase();
85+
b.to_ascii_lowercase() == "a";
86+
"a" == a.to_ascii_lowercase();
87+
}
88+
fnref_u8slice(a:&[u8],b:&[u8]){
89+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
90+
}
91+
fnu8vec(a:Vec<u8>,b:Vec<u8>){
92+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
93+
}
94+
fnref_u8vec(a:Vec<u8>,b:&Vec<u8>){
95+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
96+
b.to_ascii_lowercase() == a.to_ascii_lowercase();
97+
}
98+
fnref_osstr(a:&OsStr,b:&OsStr){
99+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
100+
}
101+
fnosstring(a:OsString,b:OsString){
102+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
103+
}
104+
fnref_osstring(a:OsString,b:&OsString){
105+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
106+
b.to_ascii_lowercase() == a.to_ascii_lowercase();
107+
}

0 commit comments

Comments
 (0)