Skip to content

Commit

Permalink
Add manual_ignore_cast_cmp lint
Browse files Browse the repository at this point in the history
  • Loading branch information
nyurik committed Sep 20, 2024
1 parent 2e5b680 commit 01bce1f
Show file tree
Hide file tree
Showing 7 changed files with 395 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}
97 changes: 97 additions & 0 deletions clippy_lints/src/manual_ignore_case_cmp.rs
Original file line number Diff line number Diff line change
@@ -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<Ty<'tcx>> {
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,
);
}
}
}
83 changes: 83 additions & 0 deletions tests/ui/manual_ignore_case_cmp.fixed
Original file line number Diff line number Diff line change
@@ -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<u8>, b: Vec<u8>) {
a.eq_ignore_ascii_case(&b);
}
fn simple_u8vec2(a: Vec<u8>, b: &Vec<u8>) {
a.eq_ignore_ascii_case(b);
}
fn simple_u8vec3(a: &Vec<u8>, b: Vec<u8>) {
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);
}
83 changes: 83 additions & 0 deletions tests/ui/manual_ignore_case_cmp.rs
Original file line number Diff line number Diff line change
@@ -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<u8>, b: Vec<u8>) {
a.to_ascii_lowercase() == b.to_ascii_lowercase();
}
fn simple_u8vec2(a: Vec<u8>, b: &Vec<u8>) {
a.to_ascii_lowercase() == b.to_ascii_lowercase();
}
fn simple_u8vec3(a: &Vec<u8>, b: Vec<u8>) {
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();
}
Loading

0 comments on commit 01bce1f

Please sign in to comment.