Skip to content

Commit b86410e

Browse files
nyuriknyurik
committed
Add manual_ignore_cast_cmp lint
1 parent 43e3384 commit b86410e

7 files changed

+892
-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
@@ -305,6 +305,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
305305
crate::manual_float_methods::MANUAL_IS_FINITE_INFO,
306306
crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
307307
crate::manual_hash_one::MANUAL_HASH_ONE_INFO,
308+
crate::manual_ignore_case_cmp::MANUAL_IGNORE_CASE_CMP_INFO,
308309
crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
309310
crate::manual_is_power_of_two::MANUAL_IS_POWER_OF_TWO_INFO,
310311
crate::manual_let_else::MANUAL_LET_ELSE_INFO,

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ mod manual_clamp;
206206
mod manual_div_ceil;
207207
mod manual_float_methods;
208208
mod manual_hash_one;
209+
mod manual_ignore_case_cmp;
209210
mod manual_is_ascii_check;
210211
mod manual_is_power_of_two;
211212
mod manual_let_else;
@@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
944945
store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
945946
store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
946947
store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
948+
store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
947949
// add lints here, do not remove this comment, it's used in `new_lint`
948950
}

‎clippy_lints/src/manual_ignore_case_cmp.rs

+128
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
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;
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+
// && let MethodCall(right_path, right_val, _, _) = right.kind
49+
// && left_path.ident == right_path.ident
50+
fn get_ascii_type<'a, 'b>(cx: &LateContext<'a>, kind: rustc_hir::ExprKind<'b>) -> Option<(Span, MatchType<'a, 'b>)> {
51+
if let MethodCall(path, expr, _, _) = kind {
52+
let is_lower = match path.ident.name.as_str() {
53+
"to_ascii_lowercase" => true,
54+
"to_ascii_uppercase" => false,
55+
_ => return None,
56+
};
57+
let ty_raw = cx.typeck_results().expr_ty(expr);
58+
let ty = ty_raw.peel_refs();
59+
if needs_ref_to_cmp(cx, ty)
60+
|| ty.is_str()
61+
|| ty.is_slice()
62+
|| matches!(get_type_diagnostic_name(cx, ty), Some(sym::OsStr | sym::OsString))
63+
{
64+
return Some((expr.span, ToAscii(is_lower, ty_raw)));
65+
}
66+
} else if let Lit(expr) = kind {
67+
return Some((expr.span, Literal(&expr.node)));
68+
}
69+
None
70+
}
71+
72+
/// Returns true if the type needs to be dereferenced to be compared
73+
fn needs_ref_to_cmp(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
74+
ty.is_char()
75+
|| *ty.kind() == ty::Uint(UintTy::U8)
76+
|| is_type_diagnostic_item(cx, ty, sym::Vec)
77+
|| is_type_lang_item(cx, ty, LangItem::String)
78+
}
79+
80+
impl LateLintPass<'_> for ManualIgnoreCaseCmp {
81+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
82+
// check if expression represents a comparison of two strings
83+
// using .to_ascii_lowercase() or .to_ascii_uppercase() methods,
84+
// or one of the sides is a literal
85+
// Offer to replace it with .eq_ignore_ascii_case() method
86+
if let Binary(op, left, right) = &expr.kind
87+
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
88+
&& let Some((left_span, left_val)) = get_ascii_type(cx, left.kind)
89+
&& let Some((right_span, right_val)) = get_ascii_type(cx, right.kind)
90+
&& match (&left_val, &right_val) {
91+
(ToAscii(l_lower, ..), ToAscii(r_lower, ..)) if l_lower == r_lower => true,
92+
(ToAscii(..), Literal(..)) | (Literal(..), ToAscii(..)) => true,
93+
_ => false,
94+
}
95+
{
96+
let deref = match right_val {
97+
ToAscii(_, ty) if needs_ref_to_cmp(cx, ty) => "&",
98+
ToAscii(..) => "",
99+
Literal(ty) => {
100+
if let LitKind::Char(_) | LitKind::Byte(_) = ty {
101+
"&"
102+
} else {
103+
""
104+
}
105+
},
106+
};
107+
let neg = if op.node == BinOpKind::Ne { "!" } else { "" };
108+
span_lint_and_then(
109+
cx,
110+
MANUAL_IGNORE_CASE_CMP,
111+
expr.span,
112+
"manually doing case-insensitive ASCII comparison",
113+
|diag| {
114+
diag.span_suggestion_verbose(
115+
expr.span,
116+
"consider using `.eq_ignore_ascii_case()` instead",
117+
format!(
118+
"{neg}{}.eq_ignore_ascii_case({deref}{})",
119+
snippet(cx, left_span, "_"),
120+
snippet(cx, right_span, "_")
121+
),
122+
Applicability::MachineApplicable,
123+
);
124+
},
125+
);
126+
}
127+
}
128+
}

‎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)