Skip to content

Commit c71f0be

Browse files
borsbors
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](#13334) Closes #13204
2 parents 55f6029 + d1dec19 commit c71f0be

7 files changed

+891
-0
lines changed

‎CHANGELOG.md

+1
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

+1
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

+2
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
}

‎clippy_lints/src/manual_ignore_case_cmp.rs

+127
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
use crate::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+
pub MANUAL_IGNORE_CASE_CMP,
37+
perf,
38+
"manual case-insensitive ASCII comparison"
39+
}
40+
41+
declare_lint_pass!(ManualIgnoreCaseCmp => [MANUAL_IGNORE_CASE_CMP]);
42+
43+
enum MatchType<'a, 'b> {
44+
ToAscii(bool, Ty<'a>),
45+
Literal(&'b LitKind),
46+
}
47+
48+
fn get_ascii_type<'a, 'b>(cx: &LateContext<'a>, kind: rustc_hir::ExprKind<'b>) -> Option<(Span, MatchType<'a, 'b>)> {
49+
if let MethodCall(path, expr, _, _) = kind {
50+
let is_lower = match path.ident.name.as_str() {
51+
"to_ascii_lowercase" => true,
52+
"to_ascii_uppercase" => false,
53+
_ => return None,
54+
};
55+
let ty_raw = cx.typeck_results().expr_ty(expr);
56+
let ty = ty_raw.peel_refs();
57+
if needs_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+
return Some((expr.span, ToAscii(is_lower, ty_raw)));
63+
}
64+
} else if let Lit(expr) = kind {
65+
return Some((expr.span, Literal(&expr.node)));
66+
}
67+
None
68+
}
69+
70+
/// Returns true if the type needs to be dereferenced to be compared
71+
fn needs_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+
impl LateLintPass<'_> for ManualIgnoreCaseCmp {
79+
fn check_expr(&mut self, 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+
if let Binary(op, left, right) = &expr.kind
85+
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
86+
&& let Some((left_span, left_val)) = get_ascii_type(cx, left.kind)
87+
&& let Some((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) if needs_ref_to_cmp(cx, ty) => "&",
96+
ToAscii(..) => "",
97+
Literal(ty) => {
98+
if let LitKind::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+
let mut 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+
}

‎tests/ui/manual_ignore_case_cmp.fixed

+107
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

+107
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.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+
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.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+
fn u8(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+
fn ref_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+
fn ref_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+
fn string(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+
fn ref_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+
fn string_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+
fn ref_u8slice(a: &[u8], b: &[u8]) {
89+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
90+
}
91+
fn u8vec(a: Vec<u8>, b: Vec<u8>) {
92+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
93+
}
94+
fn ref_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+
fn ref_osstr(a: &OsStr, b: &OsStr) {
99+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
100+
}
101+
fn osstring(a: OsString, b: OsString) {
102+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
103+
}
104+
fn ref_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)