Add manual_ignore_cast_cmp lint

rust-lang · Sep 22, 2024 · b86410e · b86410e
1 parent 43e3384
commit b86410e
Show file tree

Hide file tree

Showing 7 changed files with 892 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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;
@@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static 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(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
+    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
@@ -0,0 +1,128 @@
+use crate::manual_ignore_case_cmp::MatchType::{Literal, ToAscii};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet;
+use clippy_utils::ty::{get_type_diagnostic_name, is_type_diagnostic_item, is_type_lang_item};
+use rustc_ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::ExprKind::{Binary, Lit, 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::{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() || a.to_ascii_lowercase() == "abc"
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn compare(a: &str, b: &str) -> bool {
+    ///    a.eq_ignore_ascii_case(b) || a.eq_ignore_ascii_case("abc")
+    /// }
+    /// ```
+    #[clippy::version = "1.82.0"]
+    pub MANUAL_IGNORE_CASE_CMP,
+    perf,
+    "manual case-insensitive ASCII comparison"
+}
+
+declare_lint_pass!(ManualIgnoreCaseCmp => [MANUAL_IGNORE_CASE_CMP]);
+
+enum MatchType<'a, 'b> {
+    ToAscii(bool, Ty<'a>),
+    Literal(&'b LitKind),
+}
+
+// && let MethodCall(right_path, right_val, _, _) = right.kind
+// && left_path.ident == right_path.ident
+fn get_ascii_type<'a, 'b>(cx: &LateContext<'a>, kind: rustc_hir::ExprKind<'b>) -> Option<(Span, MatchType<'a, 'b>)> {
+    if let MethodCall(path, expr, _, _) = kind {
+        let is_lower = match path.ident.name.as_str() {
+            "to_ascii_lowercase" => true,
+            "to_ascii_uppercase" => false,
+            _ => return None,
+        };
+        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))
+        {
+            return Some((expr.span, ToAscii(is_lower, ty_raw)));
+        }
+    } else if let Lit(expr) = kind {
+        return Some((expr.span, Literal(&expr.node)));
+    }
+    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,
+        // or one of the sides is a literal
+        // 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 Some((left_span, left_val)) = get_ascii_type(cx, left.kind)
+            && let Some((right_span, right_val)) = get_ascii_type(cx, right.kind)
+            && match (&left_val, &right_val) {
+                (ToAscii(l_lower, ..), ToAscii(r_lower, ..)) if l_lower == r_lower => true,
+                (ToAscii(..), Literal(..)) | (Literal(..), ToAscii(..)) => true,
+                _ => false,
+            }
+        {
+            let deref = match right_val {
+                ToAscii(_, ty) if needs_ref_to_cmp(cx, ty) => "&",
+                ToAscii(..) => "",
+                Literal(ty) => {
+                    if let LitKind::Char(_) | LitKind::Byte(_) = ty {
+                        "&"
+                    } else {
+                        ""
+                    }
+                },
+            };
+            let neg = if op.node == BinOpKind::Ne { "!" } else { "" };
+            span_lint_and_then(
+                cx,
+                MANUAL_IGNORE_CASE_CMP,
+                expr.span,
+                "manually doing case-insensitive ASCII comparison",
+                |diag| {
+                    diag.span_suggestion_verbose(
+                        expr.span,
+                        "consider using `.eq_ignore_ascii_case()` instead",
+                        format!(
+                            "{neg}{}.eq_ignore_ascii_case({deref}{})",
+                            snippet(cx, left_span, "_"),
+                            snippet(cx, right_span, "_")
+                        ),
+                        Applicability::MachineApplicable,
+                    );
+                },
+            );
+        }
+    }
+}
diff --git a/tests/ui/manual_ignore_case_cmp.fixed b/tests/ui/manual_ignore_case_cmp.fixed
@@ -0,0 +1,107 @@
+#![allow(clippy::all)]
+#![deny(clippy::manual_ignore_case_cmp)]
+
+use std::ffi::{OsStr, OsString};
+
+fn main() {}
+
+fn variants(a: &str, b: &str) {
+    if a.eq_ignore_ascii_case(b) {
+        return;
+    }
+    if a.eq_ignore_ascii_case(b) {
+        return;
+    }
+    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());
+    // !=
+    if !a.eq_ignore_ascii_case(b) {
+        return;
+    }
+    if !a.eq_ignore_ascii_case(b) {
+        return;
+    }
+    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 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.eq_ignore_ascii_case(&'a');
+    'a'.eq_ignore_ascii_case(&b);
+}
+fn u8(a: u8, b: u8) {
+    a.eq_ignore_ascii_case(&b);
+    a.eq_ignore_ascii_case(&b'a');
+    b'a'.eq_ignore_ascii_case(&b);
+}
+fn ref_str(a: &str, b: &str) {
+    a.eq_ignore_ascii_case(b);
+    a.to_uppercase().eq_ignore_ascii_case(b);
+    a.eq_ignore_ascii_case("a");
+    "a".eq_ignore_ascii_case(b);
+}
+fn ref_ref_str(a: &&str, b: &&str) {
+    a.eq_ignore_ascii_case(b);
+    a.to_uppercase().eq_ignore_ascii_case(b);
+    a.eq_ignore_ascii_case("a");
+    "a".eq_ignore_ascii_case(b);
+}
+fn string(a: String, b: String) {
+    a.eq_ignore_ascii_case(&b);
+    a.eq_ignore_ascii_case("a");
+    "a".eq_ignore_ascii_case(&b);
+    &a.to_ascii_lowercase() == &b.to_ascii_lowercase();
+    &&a.to_ascii_lowercase() == &&b.to_ascii_lowercase();
+    a.eq_ignore_ascii_case("a");
+    "a".eq_ignore_ascii_case(&b);
+}
+fn ref_string(a: String, b: &String) {
+    a.eq_ignore_ascii_case(b);
+    a.eq_ignore_ascii_case("a");
+    "a".eq_ignore_ascii_case(b);
+
+    b.eq_ignore_ascii_case(&a);
+    b.eq_ignore_ascii_case("a");
+    "a".eq_ignore_ascii_case(&a);
+}
+fn string_ref_str(a: String, b: &str) {
+    a.eq_ignore_ascii_case(b);
+    a.eq_ignore_ascii_case("a");
+    "a".eq_ignore_ascii_case(b);
+
+    b.eq_ignore_ascii_case(&a);
+    b.eq_ignore_ascii_case("a");
+    "a".eq_ignore_ascii_case(&a);
+}
+fn ref_u8slice(a: &[u8], b: &[u8]) {
+    a.eq_ignore_ascii_case(b);
+}
+fn u8vec(a: Vec<u8>, b: Vec<u8>) {
+    a.eq_ignore_ascii_case(&b);
+}
+fn ref_u8vec(a: Vec<u8>, b: &Vec<u8>) {
+    a.eq_ignore_ascii_case(b);
+    b.eq_ignore_ascii_case(&a);
+}
+fn ref_osstr(a: &OsStr, b: &OsStr) {
+    a.eq_ignore_ascii_case(b);
+}
+fn osstring(a: OsString, b: OsString) {
+    a.eq_ignore_ascii_case(b);
+}
+fn ref_osstring(a: OsString, b: &OsString) {
+    a.eq_ignore_ascii_case(b);
+    b.eq_ignore_ascii_case(a);
+}
diff --git a/tests/ui/manual_ignore_case_cmp.rs b/tests/ui/manual_ignore_case_cmp.rs
@@ -0,0 +1,107 @@
+#![allow(clippy::all)]
+#![deny(clippy::manual_ignore_case_cmp)]
+
+use std::ffi::{OsStr, OsString};
+
+fn main() {}
+
+fn variants(a: &str, b: &str) {
+    if a.to_ascii_lowercase() == b.to_ascii_lowercase() {
+        return;
+    }
+    if a.to_ascii_uppercase() == b.to_ascii_uppercase() {
+        return;
+    }
+    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();
+    // !=
+    if a.to_ascii_lowercase() != b.to_ascii_lowercase() {
+        return;
+    }
+    if a.to_ascii_uppercase() != b.to_ascii_uppercase() {
+        return;
+    }
+    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 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 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 ref_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 ref_ref_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 string(a: String, b: String) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+    a.to_ascii_lowercase() == "a";
+    "a" == 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 ref_string(a: String, b: &String) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+    a.to_ascii_lowercase() == "a";
+    "a" == b.to_ascii_lowercase();
+
+    b.to_ascii_lowercase() == a.to_ascii_lowercase();
+    b.to_ascii_lowercase() == "a";
+    "a" == a.to_ascii_lowercase();
+}
+fn string_ref_str(a: String, b: &str) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+    a.to_ascii_lowercase() == "a";
+    "a" == b.to_ascii_lowercase();
+
+    b.to_ascii_lowercase() == a.to_ascii_lowercase();
+    b.to_ascii_lowercase() == "a";
+    "a" == a.to_ascii_lowercase();
+}
+fn ref_u8slice(a: &[u8], b: &[u8]) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+}
+fn u8vec(a: Vec<u8>, b: Vec<u8>) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+}
+fn ref_u8vec(a: Vec<u8>, b: &Vec<u8>) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+    b.to_ascii_lowercase() == a.to_ascii_lowercase();
+}
+fn ref_osstr(a: &OsStr, b: &OsStr) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+}
+fn osstring(a: OsString, b: OsString) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+}
+fn ref_osstring(a: OsString, b: &OsString) {
+    a.to_ascii_lowercase() == b.to_ascii_lowercase();
+    b.to_ascii_lowercase() == a.to_ascii_lowercase();
+}