diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e5d1688e4a7..9206b50c8b7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5621,6 +5621,7 @@ Released 2018-09-13 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten [`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one +[`manual_ignore_case_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ignore_case_cmp [`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 16c64830e70d..f2f543fa9a57 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -305,6 +305,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::manual_float_methods::MANUAL_IS_FINITE_INFO, crate::manual_float_methods::MANUAL_IS_INFINITE_INFO, crate::manual_hash_one::MANUAL_HASH_ONE_INFO, + crate::manual_ignore_case_cmp::MANUAL_IGNORE_CASE_CMP_INFO, crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO, crate::manual_is_power_of_two::MANUAL_IS_POWER_OF_TWO_INFO, crate::manual_let_else::MANUAL_LET_ELSE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3604090b68cc..dc768c607c90 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -206,6 +206,7 @@ mod manual_clamp; mod manual_div_ceil; mod manual_float_methods; mod manual_hash_one; +mod manual_ignore_case_cmp; mod manual_is_ascii_check; mod manual_is_power_of_two; mod manual_let_else; @@ -942,5 +943,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf))); store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo)); store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions)); + store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_ignore_case_cmp.rs b/clippy_lints/src/manual_ignore_case_cmp.rs new file mode 100644 index 000000000000..761176befecb --- /dev/null +++ b/clippy_lints/src/manual_ignore_case_cmp.rs @@ -0,0 +1,97 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; +use clippy_utils::ty::{get_type_diagnostic_name, is_type_diagnostic_item, is_type_lang_item}; +use rustc_errors::Applicability; +use rustc_hir::ExprKind::{Binary, MethodCall}; +use rustc_hir::{BinOpKind, Expr, LangItem}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_middle::ty::{Ty, UintTy}; +use rustc_session::declare_lint_pass; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for manual case-insensitive ASCII comparison. + /// + /// ### Why is this bad? + /// The `eq_ignore_ascii_case` method is faster because it does not allocate + /// memory for the new strings, and it is more readable. + /// + /// ### Example + /// ```no_run + /// fn compare(a: &str, b: &str) -> bool { + /// a.to_ascii_lowercase() == b.to_ascii_lowercase() + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn compare(a: &str, b: &str) -> bool { + /// a.eq_ignore_ascii_case(b) + /// } + /// ``` + #[clippy::version = "1.82.0"] + pub MANUAL_IGNORE_CASE_CMP, + perf, + "manual case-insensitive ASCII comparison" +} + +declare_lint_pass!(ManualIgnoreCaseCmp => [MANUAL_IGNORE_CASE_CMP]); + +fn get_ascii_type<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option> { + let ty_raw = cx.typeck_results().expr_ty(expr); + let ty = ty_raw.peel_refs(); + if needs_ref_to_cmp(cx, ty) + || ty.is_str() + || ty.is_slice() + || matches!(get_type_diagnostic_name(cx, ty), Some(sym::OsStr | sym::OsString)) + { + Some(ty_raw) + } else { + None + } +} + +/// Returns true if the type needs to be dereferenced to be compared +fn needs_ref_to_cmp(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + ty.is_char() + || *ty.kind() == ty::Uint(UintTy::U8) + || is_type_diagnostic_item(cx, ty, sym::Vec) + || is_type_lang_item(cx, ty, LangItem::String) +} + +impl LateLintPass<'_> for ManualIgnoreCaseCmp { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) { + // check if expression represents a comparison of two strings + // using .to_ascii_lowercase() or .to_ascii_uppercase() methods + // Offer to replace it with .eq_ignore_ascii_case() method + if let Binary(op, left, right) = &expr.kind + && (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne) + && let MethodCall(left_path, left_val, _, _) = left.kind + && let MethodCall(right_path, right_val, _, _) = right.kind + && left_path.ident == right_path.ident + && matches!( + left_path.ident.name.as_str(), + "to_ascii_lowercase" | "to_ascii_uppercase" + ) + && get_ascii_type(cx, left_val).is_some() + && let Some(rtype) = get_ascii_type(cx, right_val) + { + // FIXME: there must be a better way to add dereference operator + let deref = if needs_ref_to_cmp(cx, rtype) { "&" } else { "" }; + span_lint_and_sugg( + cx, + MANUAL_IGNORE_CASE_CMP, + expr.span, + "manual case-insensitive ASCII comparison", + "consider using `.eq_ignore_ascii_case()` instead", + format!( + "{}.eq_ignore_ascii_case({deref}{})", + snippet(cx, left_val.span, "_"), + snippet(cx, right_val.span, "_") + ), + Applicability::MachineApplicable, + ); + } + } +} diff --git a/tests/ui/manual_ignore_case_cmp.fixed b/tests/ui/manual_ignore_case_cmp.fixed new file mode 100644 index 000000000000..9a492193dc8a --- /dev/null +++ b/tests/ui/manual_ignore_case_cmp.fixed @@ -0,0 +1,83 @@ +#![allow(clippy::all)] +#![deny(clippy::manual_ignore_case_cmp)] + +use std::ffi::{OsStr, OsString}; + +fn main() {} + +fn variants(a: &str, b: &str) -> bool { + if a.eq_ignore_ascii_case(b) { + return true; + } + if a.eq_ignore_ascii_case(b) { + return true; + } + let r = a.eq_ignore_ascii_case(b); + let r = r || a.eq_ignore_ascii_case(b); + r && a.eq_ignore_ascii_case(&b.to_uppercase()) +} + +fn unsupported(a: char, b: char) { + // TODO:: these are rare, and might not be worth supporting + a.to_ascii_lowercase() == char::to_ascii_lowercase(&b); + char::to_ascii_lowercase(&a) == b.to_ascii_lowercase(); + char::to_ascii_lowercase(&a) == char::to_ascii_lowercase(&b); +} + +fn simple_char(a: char, b: char) { + a.eq_ignore_ascii_case(&b); + a.to_ascii_lowercase() == *&b.to_ascii_lowercase(); + *&a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == 'a'; + 'a' == b.to_ascii_lowercase(); +} +fn simple_u8(a: u8, b: u8) { + a.eq_ignore_ascii_case(&b); + a.to_ascii_lowercase() == b'a'; + b'a' == b.to_ascii_lowercase(); +} +fn simple_str(a: &str, b: &str) { + a.eq_ignore_ascii_case(b); + a.to_uppercase().eq_ignore_ascii_case(b); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn simple_string(a: String, b: String) { + a.eq_ignore_ascii_case(&b); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn simple_string2(a: String, b: &String) { + a.eq_ignore_ascii_case(b); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn simple_string3(a: &String, b: String) { + a.eq_ignore_ascii_case(&b); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn simple_u8slice(a: &[u8], b: &[u8]) { + a.eq_ignore_ascii_case(b); +} +fn simple_u8vec(a: Vec, b: Vec) { + a.eq_ignore_ascii_case(&b); +} +fn simple_u8vec2(a: Vec, b: &Vec) { + a.eq_ignore_ascii_case(b); +} +fn simple_u8vec3(a: &Vec, b: Vec) { + a.eq_ignore_ascii_case(&b); +} +fn simple_osstr(a: &OsStr, b: &OsStr) { + a.eq_ignore_ascii_case(b); +} +fn simple_osstring(a: OsString, b: OsString) { + a.eq_ignore_ascii_case(b); +} +fn simple_osstring2(a: OsString, b: &OsString) { + a.eq_ignore_ascii_case(b); +} +fn simple_osstring3(a: &OsString, b: OsString) { + a.eq_ignore_ascii_case(b); +} diff --git a/tests/ui/manual_ignore_case_cmp.rs b/tests/ui/manual_ignore_case_cmp.rs new file mode 100644 index 000000000000..bd09343c54ff --- /dev/null +++ b/tests/ui/manual_ignore_case_cmp.rs @@ -0,0 +1,83 @@ +#![allow(clippy::all)] +#![deny(clippy::manual_ignore_case_cmp)] + +use std::ffi::{OsStr, OsString}; + +fn main() {} + +fn variants(a: &str, b: &str) -> bool { + if a.to_ascii_lowercase() == b.to_ascii_lowercase() { + return true; + } + if a.to_ascii_uppercase() == b.to_ascii_uppercase() { + return true; + } + let r = a.to_ascii_lowercase() == b.to_ascii_lowercase(); + let r = r || a.to_ascii_uppercase() == b.to_ascii_uppercase(); + r && a.to_ascii_lowercase() == b.to_uppercase().to_ascii_lowercase() +} + +fn unsupported(a: char, b: char) { + // TODO:: these are rare, and might not be worth supporting + a.to_ascii_lowercase() == char::to_ascii_lowercase(&b); + char::to_ascii_lowercase(&a) == b.to_ascii_lowercase(); + char::to_ascii_lowercase(&a) == char::to_ascii_lowercase(&b); +} + +fn simple_char(a: char, b: char) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == *&b.to_ascii_lowercase(); + *&a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == 'a'; + 'a' == b.to_ascii_lowercase(); +} +fn simple_u8(a: u8, b: u8) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == b'a'; + b'a' == b.to_ascii_lowercase(); +} +fn simple_str(a: &str, b: &str) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn simple_string(a: String, b: String) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn simple_string2(a: String, b: &String) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn simple_string3(a: &String, b: String) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn simple_u8slice(a: &[u8], b: &[u8]) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn simple_u8vec(a: Vec, b: Vec) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn simple_u8vec2(a: Vec, b: &Vec) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn simple_u8vec3(a: &Vec, b: Vec) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn simple_osstr(a: &OsStr, b: &OsStr) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn simple_osstring(a: OsString, b: OsString) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn simple_osstring2(a: OsString, b: &OsString) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn simple_osstring3(a: &OsString, b: OsString) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} diff --git a/tests/ui/manual_ignore_case_cmp.stderr b/tests/ui/manual_ignore_case_cmp.stderr new file mode 100644 index 000000000000..53c56e44411d --- /dev/null +++ b/tests/ui/manual_ignore_case_cmp.stderr @@ -0,0 +1,128 @@ +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:9:8 + | +LL | if a.to_ascii_lowercase() == b.to_ascii_lowercase() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + | +note: the lint level is defined here + --> tests/ui/manual_ignore_case_cmp.rs:2:9 + | +LL | #![deny(clippy::manual_ignore_case_cmp)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:12:8 + | +LL | if a.to_ascii_uppercase() == b.to_ascii_uppercase() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:15:13 + | +LL | let r = a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:16:18 + | +LL | let r = r || a.to_ascii_uppercase() == b.to_ascii_uppercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:17:10 + | +LL | r && a.to_ascii_lowercase() == b.to_uppercase().to_ascii_lowercase() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(&b.to_uppercase())` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:28:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(&b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:35:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(&b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:40:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:41:5 + | +LL | a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.to_uppercase().eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:46:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(&b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:51:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:56:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(&b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:61:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:64:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(&b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:67:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:70:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(&b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:73:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:76:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:79:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:82:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.eq_ignore_ascii_case()` instead: `a.eq_ignore_ascii_case(b)` + +error: aborting due to 20 previous errors +