Skip to content

Commit c2b3c6b

Browse files
nyuriknyurik
committed
Add manual_ignore_cast_cmp lint
1 parent e8ba5d1 commit c2b3c6b

7 files changed

+372
-0
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5566,6 +5566,7 @@ Released 2018-09-13
55665566
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
55675567
[`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
55685568
[`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one
5569+
[`manual_ignore_case_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ignore_case_cmp
55695570
[`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
55705571
[`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
55715572
[`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
@@ -304,6 +304,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
304304
crate::manual_float_methods::MANUAL_IS_FINITE_INFO,
305305
crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
306306
crate::manual_hash_one::MANUAL_HASH_ONE_INFO,
307+
crate::manual_ignore_case_cmp::MANUAL_IGNORE_CASE_CMP_INFO,
307308
crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
308309
crate::manual_let_else::MANUAL_LET_ELSE_INFO,
309310
crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ mod manual_bits;
205205
mod manual_clamp;
206206
mod manual_float_methods;
207207
mod manual_hash_one;
208+
mod manual_ignore_case_cmp;
208209
mod manual_is_ascii_check;
209210
mod manual_let_else;
210211
mod manual_main_separator_str;
@@ -937,5 +938,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
937938
store.register_early_pass(|| Box::new(cfg_not_test::CfgNotTest));
938939
store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
939940
store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock));
941+
store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
940942
// add lints here, do not remove this comment, it's used in `new_lint`
941943
}

‎clippy_lints/src/manual_ignore_case_cmp.rs

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet;
3+
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
4+
use rustc_errors::Applicability;
5+
use rustc_hir::ExprKind::{Binary, MethodCall};
6+
use rustc_hir::{BinOpKind, Expr, LangItem};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_middle::ty;
9+
use rustc_middle::ty::{Ty, UintTy};
10+
use rustc_session::declare_lint_pass;
11+
use rustc_span::sym;
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Checks for manual case-insensitive ASCII comparison.
16+
///
17+
/// ### Why is this bad?
18+
/// The `eq_ignore_ascii_case` method is faster because it does not allocate
19+
/// memory for the new strings, and it is more readable.
20+
///
21+
/// ### Example
22+
/// ```no_run
23+
/// fn compare(a: &str, b: &str) -> bool {
24+
/// a.to_ascii_lowercase() == b.to_ascii_lowercase()
25+
/// }
26+
/// ```
27+
/// Use instead:
28+
/// ```no_run
29+
/// fn compare(a: &str, b: &str) -> bool {
30+
/// a.eq_ignore_ascii_case(b)
31+
/// }
32+
/// ```
33+
#[clippy::version = "1.82.0"]
34+
pub MANUAL_IGNORE_CASE_CMP,
35+
nursery,
36+
"manual case-insensitive ASCII comparison"
37+
}
38+
39+
declare_lint_pass!(ManualIgnoreCaseCmp => [MANUAL_IGNORE_CASE_CMP]);
40+
41+
fn get_ascii_type<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<Ty<'tcx>> {
42+
let ty_raw = cx.typeck_results().expr_ty(expr);
43+
let ty = ty_raw.peel_refs();
44+
if needs_ref_to_cmp(cx, ty)
45+
|| ty.is_str()
46+
|| ty.is_slice()
47+
// FIXME: Need a better way to check if ty is OsStr(ing)
48+
|| ["std::ffi::OsStr", "std::ffi::OsString"].contains(&ty.to_string().as_str())
49+
{
50+
Some(ty_raw)
51+
} else {
52+
None
53+
}
54+
}
55+
56+
/// Returns true if the type needs to be dereferenced to be compared
57+
fn needs_ref_to_cmp(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
58+
ty.is_char()
59+
|| *ty.kind() == ty::Uint(UintTy::U8)
60+
|| is_type_diagnostic_item(cx, ty, sym::Vec)
61+
|| is_type_lang_item(cx, ty, LangItem::String)
62+
}
63+
64+
impl LateLintPass<'_> for ManualIgnoreCaseCmp {
65+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
66+
// check if expression represents a comparison of two strings
67+
// using .to_ascii_lowercase() or .to_ascii_uppercase() methods
68+
// Offer to replace it with .eq_ignore_ascii_case() method
69+
if let Binary(op, left, right) = &expr.kind
70+
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
71+
&& let MethodCall(left_path, left_val, _, _) = left.kind
72+
&& let MethodCall(right_path, right_val, _, _) = right.kind
73+
&& left_path.ident == right_path.ident
74+
&& matches!(
75+
left_path.ident.name.as_str(),
76+
"to_ascii_lowercase" | "to_ascii_uppercase"
77+
)
78+
&& get_ascii_type(cx, left_val).is_some()
79+
&& let Some(rtype) = get_ascii_type(cx, right_val)
80+
{
81+
// FIXME: there must be a better way to add dereference operator
82+
let deref = if needs_ref_to_cmp(cx, rtype) { "&" } else { "" };
83+
span_lint_and_sugg(
84+
cx,
85+
MANUAL_IGNORE_CASE_CMP,
86+
expr.span,
87+
"manual case-insensitive ASCII comparison",
88+
"consider using `.eq_ignore_ascii_case()` instead",
89+
format!(
90+
"{}.eq_ignore_ascii_case({deref}{})",
91+
snippet(cx, left_val.span, "_"),
92+
snippet(cx, right_val.span, "_")
93+
),
94+
Applicability::MachineApplicable,
95+
);
96+
}
97+
}
98+
}

‎tests/ui/manual_ignore_case_cmp.fixed

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
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) -> bool {
9+
if a.eq_ignore_ascii_case(b) {
10+
return true;
11+
}
12+
if a.eq_ignore_ascii_case(b) {
13+
return true;
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+
20+
fn unsupported(a: char, b: char) {
21+
// TODO:: these are rare, and might not be worth supporting
22+
a.to_ascii_lowercase() == char::to_ascii_lowercase(&b);
23+
char::to_ascii_lowercase(&a) == b.to_ascii_lowercase();
24+
char::to_ascii_lowercase(&a) == char::to_ascii_lowercase(&b);
25+
}
26+
27+
fn simple_char(a: char, b: char) {
28+
a.eq_ignore_ascii_case(&b);
29+
a.to_ascii_lowercase() == *&b.to_ascii_lowercase();
30+
*&a.to_ascii_lowercase() == b.to_ascii_lowercase();
31+
}
32+
fn simple_u8(a: u8, b: u8) {
33+
a.eq_ignore_ascii_case(&b);
34+
}
35+
fn simple_str(a: &str, b: &str) {
36+
a.eq_ignore_ascii_case(b);
37+
a.to_uppercase().eq_ignore_ascii_case(b);
38+
}
39+
fn simple_string(a: String, b: String) {
40+
a.eq_ignore_ascii_case(&b);
41+
}
42+
fn simple_string2(a: String, b: &String) {
43+
a.eq_ignore_ascii_case(b);
44+
}
45+
fn simple_string3(a: &String, b: String) {
46+
a.eq_ignore_ascii_case(&b);
47+
}
48+
fn simple_u8slice(a: &[u8], b: &[u8]) {
49+
a.eq_ignore_ascii_case(b);
50+
}
51+
fn simple_u8vec(a: Vec<u8>, b: Vec<u8>) {
52+
a.eq_ignore_ascii_case(&b);
53+
}
54+
fn simple_u8vec2(a: Vec<u8>, b: &Vec<u8>) {
55+
a.eq_ignore_ascii_case(b);
56+
}
57+
fn simple_u8vec3(a: &Vec<u8>, b: Vec<u8>) {
58+
a.eq_ignore_ascii_case(&b);
59+
}
60+
fn simple_osstr(a: &OsStr, b: &OsStr) {
61+
a.eq_ignore_ascii_case(b);
62+
}
63+
fn simple_osstring(a: OsString, b: OsString) {
64+
a.eq_ignore_ascii_case(b);
65+
}
66+
fn simple_osstring2(a: OsString, b: &OsString) {
67+
a.eq_ignore_ascii_case(b);
68+
}
69+
fn simple_osstring3(a: &OsString, b: OsString) {
70+
a.eq_ignore_ascii_case(b);
71+
}

‎tests/ui/manual_ignore_case_cmp.rs

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
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) -> bool {
9+
if a.to_ascii_lowercase() == b.to_ascii_lowercase() {
10+
return true;
11+
}
12+
if a.to_ascii_uppercase() == b.to_ascii_uppercase() {
13+
return true;
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+
20+
fn unsupported(a: char, b: char) {
21+
// TODO:: these are rare, and might not be worth supporting
22+
a.to_ascii_lowercase() == char::to_ascii_lowercase(&b);
23+
char::to_ascii_lowercase(&a) == b.to_ascii_lowercase();
24+
char::to_ascii_lowercase(&a) == char::to_ascii_lowercase(&b);
25+
}
26+
27+
fn simple_char(a: char, b: char) {
28+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
29+
a.to_ascii_lowercase() == *&b.to_ascii_lowercase();
30+
*&a.to_ascii_lowercase() == b.to_ascii_lowercase();
31+
}
32+
fn simple_u8(a: u8, b: u8) {
33+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
34+
}
35+
fn simple_str(a: &str, b: &str) {
36+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
37+
a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase();
38+
}
39+
fn simple_string(a: String, b: String) {
40+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
41+
}
42+
fn simple_string2(a: String, b: &String) {
43+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
44+
}
45+
fn simple_string3(a: &String, b: String) {
46+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
47+
}
48+
fn simple_u8slice(a: &[u8], b: &[u8]) {
49+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
50+
}
51+
fn simple_u8vec(a: Vec<u8>, b: Vec<u8>) {
52+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
53+
}
54+
fn simple_u8vec2(a: Vec<u8>, b: &Vec<u8>) {
55+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
56+
}
57+
fn simple_u8vec3(a: &Vec<u8>, b: Vec<u8>) {
58+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
59+
}
60+
fn simple_osstr(a: &OsStr, b: &OsStr) {
61+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
62+
}
63+
fn simple_osstring(a: OsString, b: OsString) {
64+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
65+
}
66+
fn simple_osstring2(a: OsString, b: &OsString) {
67+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
68+
}
69+
fn simple_osstring3(a: &OsString, b: OsString) {
70+
a.to_ascii_lowercase() == b.to_ascii_lowercase();
71+
}

0 commit comments

Comments
 (0)