diff --git a/clippy_lints/src/manual_ignore_case_cmp.rs b/clippy_lints/src/manual_ignore_case_cmp.rs index 2173617bd5aa..34062b60da53 100644 --- a/clippy_lints/src/manual_ignore_case_cmp.rs +++ b/clippy_lints/src/manual_ignore_case_cmp.rs @@ -1,13 +1,13 @@ use crate::manual_ignore_case_cmp::MatchType::{Literal, ToAscii}; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::res::MaybeDef as _; +use clippy_utils::res::{MaybeDef as _, MaybeResPath as _}; use clippy_utils::source::snippet_with_context; -use clippy_utils::sym; +use clippy_utils::{method_chain_args, sym}; use rustc_ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::ExprKind::{Binary, Lit, MethodCall}; +use rustc_hir::ExprKind::{Binary, Closure, Lit, MethodCall}; use rustc_hir::attrs::lang_items::LangItem; -use rustc_hir::{BinOpKind, Expr}; +use rustc_hir::{BinOpKind, Expr, PatKind}; use rustc_lint::{LateContext, LateLintPass, declare_lint_pass}; use rustc_middle::ty; use rustc_middle::ty::{Ty, UintTy}; @@ -68,6 +68,115 @@ fn get_ascii_type<'a>(cx: &LateContext<'a>, kind: rustc_hir::ExprKind<'_>) -> Op None } +struct CharsMap<'tcx> { + expr: &'tcx Expr<'tcx>, + is_lower: bool, +} + +fn ascii_case_map_closure(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { + if let Closure(closure) = expr.kind + && let body = cx.tcx.hir_body(closure.body) + && body.params.len() == 1 + && let PatKind::Binding(_, binding, ..) = body.params[0].pat.kind + && let MethodCall(path, receiver, [], _) = body.value.kind + && receiver.res_local_id() == Some(binding) + { + match path.ident.name { + sym::to_ascii_lowercase => Some(true), + sym::to_ascii_uppercase => Some(false), + _ => None, + } + } else { + None + } +} + +fn is_str_like_chars_receiver(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let ty = cx.typeck_results().expr_ty(expr).peel_refs(); + ty.is_str() || ty.is_lang_item(cx, LangItem::String) +} + +fn ascii_case_mapped_chars<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option> { + if let Some(args) = method_chain_args(expr, &[sym::chars, sym::map]) + && args[0].1.is_empty() + && let [map_arg] = args[1].1 + && let Some(is_lower) = ascii_case_map_closure(cx, map_arg) + && is_str_like_chars_receiver(cx, args[0].0) + { + Some(CharsMap { + expr: args[0].0, + is_lower, + }) + } else { + None + } +} + +fn get_chars_cmp<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, +) -> Option<(bool, &'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> { + if let MethodCall(path, cmp_expr, [], _) = expr.kind { + let is_eq = if path.ident.name == sym::is_eq { + true + } else if path.ident.name == sym::is_ne { + false + } else { + return None; + }; + + if let MethodCall(path, left_iter, [right_iter], _) = cmp_expr.kind + && path.ident.name == sym::cmp + && let Some(left) = ascii_case_mapped_chars(cx, left_iter) + && let Some(right) = ascii_case_mapped_chars(cx, right_iter) + && left.is_lower == right.is_lower + { + return Some((is_eq, left.expr, right.expr)); + } + } + + None +} + +fn emit_lint( + cx: &LateContext<'_>, + expr: &Expr<'_>, + left_span: Span, + right_span: Span, + right_val: &MatchType<'_>, + neg: &str, +) { + let deref = match right_val { + ToAscii(_, ty) if needs_ref_to_cmp(cx, *ty) => "&", + Literal(LitKind::Char(_) | LitKind::Byte(_)) => "&", + ToAscii(..) | Literal(_) => "", + }; + span_lint_and_then( + cx, + MANUAL_IGNORE_CASE_CMP, + expr.span, + "manual case-insensitive ASCII comparison", + |diag| { + let mut app = Applicability::MachineApplicable; + let (left_snip, _) = snippet_with_context(cx, left_span, expr.span.ctxt(), "..", &mut app); + let (right_snip, _) = snippet_with_context(cx, right_span, expr.span.ctxt(), "..", &mut app); + diag.span_suggestion_verbose( + expr.span, + "consider using `.eq_ignore_ascii_case()` instead", + format!("{neg}{left_snip}.eq_ignore_ascii_case({deref}{right_snip})"), + app, + ); + }, + ); +} + +fn emit_chars_cmp_lint(cx: &LateContext<'_>, expr: &Expr<'_>, is_eq: bool, left: &Expr<'_>, right: &Expr<'_>) { + let ty = cx.typeck_results().expr_ty(right); + let right_val = ToAscii(true, ty); + let neg = if is_eq { "" } else { "!" }; + emit_lint(cx, expr, left.span, right.span, &right_val, neg); +} + /// 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() @@ -76,8 +185,8 @@ fn needs_ref_to_cmp(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { || ty.is_lang_item(cx, LangItem::String) } -impl LateLintPass<'_> for ManualIgnoreCaseCmp { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) { +impl<'tcx> LateLintPass<'tcx> for ManualIgnoreCaseCmp { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { // 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 @@ -92,35 +201,10 @@ impl LateLintPass<'_> for ManualIgnoreCaseCmp { _ => 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, - "manual case-insensitive ASCII comparison", - |diag| { - let mut app = Applicability::MachineApplicable; - let (left_snip, _) = snippet_with_context(cx, left_span, expr.span.ctxt(), "..", &mut app); - let (right_snip, _) = snippet_with_context(cx, right_span, expr.span.ctxt(), "..", &mut app); - diag.span_suggestion_verbose( - expr.span, - "consider using `.eq_ignore_ascii_case()` instead", - format!("{neg}{left_snip}.eq_ignore_ascii_case({deref}{right_snip})"), - app, - ); - }, - ); + emit_lint(cx, expr, left_span, right_span, &right_val, neg); + } else if let Some((is_eq, left, right)) = get_chars_cmp(cx, expr) { + emit_chars_cmp_lint(cx, expr, is_eq, left, right); } } } diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index eff08caa695e..6051ced465d4 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -378,8 +378,10 @@ generate! { is_diagnostic_item, is_digit, is_empty, + is_eq, is_err, is_file, + is_ne, is_none, is_none_or, is_ok, diff --git a/tests/ui/manual_ignore_case_cmp.fixed b/tests/ui/manual_ignore_case_cmp.fixed index 50d1011a5b2e..e1afc4510ef6 100644 --- a/tests/ui/manual_ignore_case_cmp.fixed +++ b/tests/ui/manual_ignore_case_cmp.fixed @@ -161,6 +161,50 @@ fn ref_osstring(a: OsString, b: &OsString) { //~^ manual_ignore_case_cmp } +fn chars_cmp(a: &str, b: &str, s: String) { + a.eq_ignore_ascii_case(b); + //~^^^^ manual_ignore_case_cmp + + !a.eq_ignore_ascii_case(b); + //~^^^^ manual_ignore_case_cmp + + s.eq_ignore_ascii_case(".xyz"); + //~^^^^ manual_ignore_case_cmp + + a.chars() + .map(|c| c.to_ascii_lowercase()) + .cmp(b.chars().map(|c| c.to_ascii_uppercase())) + .is_eq(); +} + +fn chars_cmp_in_closure(file_name: OsString) -> bool { + file_name.to_str().is_some_and(|dir| { + dir.eq_ignore_ascii_case(".xyz") + //~^^^^ manual_ignore_case_cmp + }) +} + +fn chars_cmp_deref_to_str(s: S) -> bool { + s.chars() + .map(|c| c.to_ascii_lowercase()) + .cmp(s.chars().map(|c| c.to_ascii_lowercase())) + .is_eq() +} + +struct S; + +impl S { + fn eq_ignore_ascii_case(self) {} +} + +impl std::ops::Deref for S { + type Target = str; + + fn deref(&self) -> &Self::Target { + "" + } +} + fn wrongly_unmangled_macros(a: &str, b: &str) -> bool { struct S<'a> { inner: &'a str, diff --git a/tests/ui/manual_ignore_case_cmp.rs b/tests/ui/manual_ignore_case_cmp.rs index 9e42c7ee941b..7223e2412e54 100644 --- a/tests/ui/manual_ignore_case_cmp.rs +++ b/tests/ui/manual_ignore_case_cmp.rs @@ -161,6 +161,62 @@ fn ref_osstring(a: OsString, b: &OsString) { //~^ manual_ignore_case_cmp } +fn chars_cmp(a: &str, b: &str, s: String) { + a.chars() + .map(|c| c.to_ascii_lowercase()) + .cmp(b.chars().map(|c| c.to_ascii_lowercase())) + .is_eq(); + //~^^^^ manual_ignore_case_cmp + + a.chars() + .map(|c| c.to_ascii_uppercase()) + .cmp(b.chars().map(|c| c.to_ascii_uppercase())) + .is_ne(); + //~^^^^ manual_ignore_case_cmp + + s.chars() + .map(|c| c.to_ascii_lowercase()) + .cmp(".xyz".chars().map(|c| c.to_ascii_lowercase())) + .is_eq(); + //~^^^^ manual_ignore_case_cmp + + a.chars() + .map(|c| c.to_ascii_lowercase()) + .cmp(b.chars().map(|c| c.to_ascii_uppercase())) + .is_eq(); +} + +fn chars_cmp_in_closure(file_name: OsString) -> bool { + file_name.to_str().is_some_and(|dir| { + dir.chars() + .map(|c| c.to_ascii_lowercase()) + .cmp(".xyz".chars().map(|c| c.to_ascii_lowercase())) + .is_eq() + //~^^^^ manual_ignore_case_cmp + }) +} + +fn chars_cmp_deref_to_str(s: S) -> bool { + s.chars() + .map(|c| c.to_ascii_lowercase()) + .cmp(s.chars().map(|c| c.to_ascii_lowercase())) + .is_eq() +} + +struct S; + +impl S { + fn eq_ignore_ascii_case(self) {} +} + +impl std::ops::Deref for S { + type Target = str; + + fn deref(&self) -> &Self::Target { + "" + } +} + fn wrongly_unmangled_macros(a: &str, b: &str) -> bool { struct S<'a> { inner: &'a str, diff --git a/tests/ui/manual_ignore_case_cmp.stderr b/tests/ui/manual_ignore_case_cmp.stderr index 2f698e076ed3..f9e99787f23e 100644 --- a/tests/ui/manual_ignore_case_cmp.stderr +++ b/tests/ui/manual_ignore_case_cmp.stderr @@ -589,7 +589,79 @@ LL + b.eq_ignore_ascii_case(a); | error: manual case-insensitive ASCII comparison - --> tests/ui/manual_ignore_case_cmp.rs:178:5 + --> tests/ui/manual_ignore_case_cmp.rs:165:5 + | +LL | / a.chars() +LL | | .map(|c| c.to_ascii_lowercase()) +LL | | .cmp(b.chars().map(|c| c.to_ascii_lowercase())) +LL | | .is_eq(); + | |________________^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL - a.chars() +LL - .map(|c| c.to_ascii_lowercase()) +LL - .cmp(b.chars().map(|c| c.to_ascii_lowercase())) +LL - .is_eq(); +LL + a.eq_ignore_ascii_case(b); + | + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:171:5 + | +LL | / a.chars() +LL | | .map(|c| c.to_ascii_uppercase()) +LL | | .cmp(b.chars().map(|c| c.to_ascii_uppercase())) +LL | | .is_ne(); + | |________________^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL - a.chars() +LL - .map(|c| c.to_ascii_uppercase()) +LL - .cmp(b.chars().map(|c| c.to_ascii_uppercase())) +LL - .is_ne(); +LL + !a.eq_ignore_ascii_case(b); + | + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:177:5 + | +LL | / s.chars() +LL | | .map(|c| c.to_ascii_lowercase()) +LL | | .cmp(".xyz".chars().map(|c| c.to_ascii_lowercase())) +LL | | .is_eq(); + | |________________^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL - s.chars() +LL - .map(|c| c.to_ascii_lowercase()) +LL - .cmp(".xyz".chars().map(|c| c.to_ascii_lowercase())) +LL - .is_eq(); +LL + s.eq_ignore_ascii_case(".xyz"); + | + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:191:9 + | +LL | / dir.chars() +LL | | .map(|c| c.to_ascii_lowercase()) +LL | | .cmp(".xyz".chars().map(|c| c.to_ascii_lowercase())) +LL | | .is_eq() + | |____________________^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL - dir.chars() +LL - .map(|c| c.to_ascii_lowercase()) +LL - .cmp(".xyz".chars().map(|c| c.to_ascii_lowercase())) +LL - .is_eq() +LL + dir.eq_ignore_ascii_case(".xyz") + | + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:234:5 | LL | dot_inner!(a).to_ascii_lowercase() == dot_inner!(b).to_ascii_lowercase() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -601,7 +673,7 @@ LL + dot_inner!(a).eq_ignore_ascii_case(dot_inner!(b)) | error: manual case-insensitive ASCII comparison - --> tests/ui/manual_ignore_case_cmp.rs:180:12 + --> tests/ui/manual_ignore_case_cmp.rs:236:12 | LL | || dot_inner!(a).to_ascii_lowercase() == "abc" | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -612,5 +684,5 @@ LL - || dot_inner!(a).to_ascii_lowercase() == "abc" LL + || dot_inner!(a).eq_ignore_ascii_case("abc") | -error: aborting due to 51 previous errors +error: aborting due to 55 previous errors