Skip to content

fix manual_ignore_case_cmp chars iterator cmp - #16962

Open
cyphercodes wants to merge 2 commits into
rust-lang:masterfrom
cyphercodes:fix/manual-ignore-case-cmp-chars
Open

fix manual_ignore_case_cmp chars iterator cmp#16962
cyphercodes wants to merge 2 commits into
rust-lang:masterfrom
cyphercodes:fix/manual-ignore-case-cmp-chars

Conversation

@cyphercodes

@cyphercodes cyphercodes commented May 4, 2026

Copy link
Copy Markdown
Contributor

changelog: [manual_ignore_case_cmp]: detect case-folded .chars() iterators compared with Iterator::cmp

Fixes #16961

Summary

  • Detect .chars().map(|c| c.to_ascii_lowercase()/to_ascii_uppercase()).cmp(...).is_eq() forms.
  • Also handle the corresponding .is_ne() form by suggesting !eq_ignore_ascii_case.
  • Add UI coverage for borrowed strings, owned strings, and the to_str().is_some_and(...) closure reproducer.

Tests

  • cargo fmt --check -- clippy_lints/src/manual_ignore_case_cmp.rs tests/ui/manual_ignore_case_cmp.rs clippy_utils/src/sym.rs
  • TESTNAME=manual_ignore_case_cmp cargo uibless
  • TESTNAME=manual_ignore_case_cmp cargo uitest
  • cargo test --test dogfood
  • git diff --check

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 4, 2026
@rustbot

rustbot commented May 4, 2026

Copy link
Copy Markdown
Collaborator

r? @samueltardieu

rustbot has assigned @samueltardieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@cyphercodes
cyphercodes force-pushed the fix/manual-ignore-case-cmp-chars branch from 6155d22 to f58a16a Compare May 4, 2026 04:29
&& args[0].1.is_empty()
&& let [map_arg] = args[1].1
&& let Some(is_lower) = ascii_case_map_closure(cx, map_arg)
&& cx.typeck_results().expr_ty_adjusted(args[0].0).peel_refs().is_str()

@samueltardieu samueltardieu Jun 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably be stricter than that, and check if this is a (possibly multi-level) reference to str. Otherwise, you may end up generating fixes that are incorrect, such as in this (convoluted) example:

struct S;

impl S {
    fn eq_ignore_case_cmp(self) {}
}

impl std::ops::Deref for S {
    type Target = str;
    fn deref(&self) -> &Self::Target {
        ""
    }
}

fn bogus(s: S) -> bool {
    s.chars()
        .map(|c| c.to_ascii_lowercase())
        .cmp(s.chars().map(|c| c.to_ascii_lowercase()))
        .is_eq()
}

This will suggest s.eq_ignore_ascii_case(s) which won't compile.

View changes since the review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jun 17, 2026
@rustbot

rustbot commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@cyphercodes

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback by making the .chars().map(...).cmp(...) matcher stricter: it now only lints receivers whose unadjusted type is str/references to str or String, avoiding arbitrary Deref<Target = str> types that could produce invalid suggestions.

Added a regression case for a custom Deref<Target = str> type.

Verification:

  • cargo fmt -- clippy_lints/src/manual_ignore_case_cmp.rs tests/ui/manual_ignore_case_cmp.rs
  • TESTNAME=manual_ignore_case_cmp cargo uibless
  • TESTNAME=manual_ignore_case_cmp cargo uitest
  • cargo check -p clippy_lints
  • git diff --check

@samueltardieu

Copy link
Copy Markdown
Member

Verification:

* `cargo fmt -- clippy_lints/src/manual_ignore_case_cmp.rs tests/ui/manual_ignore_case_cmp.rs`

* `TESTNAME=manual_ignore_case_cmp cargo uibless`

* `TESTNAME=manual_ignore_case_cmp cargo uitest`

* `cargo check -p clippy_lints`

* `git diff --check`

Please avoid this LLM generated noise, especially when answering review comments.

@rustbot

This comment has been minimized.

@cyphercodes
cyphercodes force-pushed the fix/manual-ignore-case-cmp-chars branch from 31df384 to 3a6e1a8 Compare July 17, 2026 18:55
@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@cyphercodes
cyphercodes force-pushed the fix/manual-ignore-case-cmp-chars branch from 3a6e1a8 to 11e6336 Compare August 21, 2026 09:09
@rustbot

This comment has been minimized.

@cyphercodes

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Aug 21, 2026
@rustbot

This comment has been minimized.

@cyphercodes
cyphercodes force-pushed the fix/manual-ignore-case-cmp-chars branch from 11e6336 to 16e17f2 Compare August 24, 2026 09:16
@rustbot

rustbot commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

manual_ignore_case_cmp: Please detect forms using .chars() and Iterator::cmp as well

3 participants