Add manual_ignore_cast_cmp lint

[nyurik]

// bad
fn compare(a: &str, b: &str) -> bool {
    a.to_ascii_lowercase() == b.to_ascii_lowercase()
    || a.to_ascii_lowercase() == "abc"
}

// good
fn compare(a: &str, b: &str) -> bool {
   a.eq_ignore_ascii_case(b)
   || a.eq_ignore_ascii_case("abc")
}

• Followed [lint naming conventions][lint_naming]
• Added passing UI tests (including committed .stderr file)
• cargo test passes locally
• Executed cargo dev update_lints
• Added lint documentation
• Run cargo dev fmt

changelog: New lint: [manual_ignore_case_cmp] perf
#13334

Closes #13204

[rustbot]

r? @xFrednet

rustbot has assigned @xFrednet.
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

[bors]

☔ The latest upstream changes (presumably #13247) made this pull request unmergeable. Please resolve the merge conflicts.

[xFrednet]

Solid start, let me know if anything is unclear :D

[xFrednet]

Can you explain these lines?

They feel a bit weird. The peel_refs removes all potential references from the type, which means that some ascii types will fail the needs_ref_to_cmp function even if ty_raw would pass it.

[nyurik]

@xFrednet what's the better way to do this comparison?

[nyurik]

i was thinking some more on this, and tried to come up with a unit test that would fail, but was not able to do that... are you certain this is an issue?

[xFrednet]

I think the naming tripped me up. When using this function you should make sure that the raw type is still a reference. (ty_raw.is_ref() && needs_ref_to_cmp(cx, ty)) should do the trick

[nyurik]

@xFrednet I tried your suggestion, but it starts to miss half of the test cases like these:

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() == "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";

    b.to_ascii_lowercase() == a.to_ascii_lowercase();
    "a" == a.to_ascii_lowercase();
}

[bors]

☔ The latest upstream changes (presumably #12987) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #13322) made this pull request unmergeable. Please resolve the merge conflicts.

[xFrednet]

Three small nits, but the rest looks good to me :D

[nyurik]

thx, updated

[nyurik]

CI is also having some download issues, seems unrelated

[nyurik]

@xFrednet hi, anything else left on this one? thx!

[xFrednet]

This version looks good to me! I've also checked and we don't need an MSRV check, since eq_ignore_ascii_case and to_ascii_lowercase have the same MSRV

I created an FCP: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/FCP.20.60manual_ignore_cast_cmp.60/near/474019623

[xFrednet]

It looks like the FCP passed without comments :D

Thank you for the implementation:

---

Roses are red,
Violets are blue,
Final comments are done,
there were none

[bors]

📌 Commit d1dec19 has been approved by xFrednet

It is now in the queue for this repository.

[bors]

⌛ Testing commit d1dec19 with merge c71f0be...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing c71f0be to master...