Add lint to check manual pattern char comparison #12849

AurelienFT · 2024-05-25T12:37:32Z

This PR adds a new lint asked in #12490

This lint catches manual char comparison in pattern of string functions and propose to use char or array of char instead.

As it's my first contribution i'm not feeling very safe about not matching too much or missing some cases.

Thanks in advance for taking time to review and propose feedback !

changelog: new lint [manual_pattern_char_comparison]

rustbot · 2024-05-25T12:37:36Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Alexendoo (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

tests/ui/search_is_some.rs

clippy_lints/src/methods/manual_pattern_char_comparison.rs

clippy_lints/src/string_patterns.rs

clippy_utils/src/str_utils.rs

AurelienFT · 2024-05-28T09:34:54Z

I have applied all the suggestions. Thank you for the very deep explanations as a first contributor it's very appreciable. I learnt a lot of things in Clippy and it motivate me to continue to contribute !

AurelienFT · 2024-05-29T13:22:27Z

@Alexendoo thank you for the heart, that's kind ! If there anything I need to do for this PR or I need to wait ? I don't want to be rude, it's because I'm not very familiar with the process so I prefer to ask

Alexendoo · 2024-05-30T14:06:46Z

I plan to do another read through of it soon, then after that I'll create a final comment period thread on Zulip for others to weigh in

clippy_lints/src/string_patterns.rs

Alexendoo · 2024-06-02T19:55:33Z

Opened the FCP here also: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/FCP.20.60manual_pattern_char_comparison.60

bors · 2024-06-03T09:18:11Z

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

clippy_lints/src/string_patterns.rs

AurelienFT · 2024-06-07T19:43:48Z

I addressed all the comments.

Furthermore I saw that last time I forget to push modification on function get_char_span that reduce the code to less than 10 lines thanks to your ideas @Alexendoo.

Everything should be fine now

bors · 2024-06-11T11:24:33Z

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

Alexendoo · 2024-06-11T18:38:27Z

If you could squash the commits I think this is ready to merge, the FCP has been running long enough without issue

rustbot · 2024-06-11T19:54:50Z

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git pull --rebase https://github.com/rust-lang/rust-clippy.git master
$ git push --force-with-lease

The following commits are merge commits:

c658805

…ith single_char_pattern lint

AurelienFT · 2024-06-11T19:57:56Z

@Alexendoo Seems ok to me. (I'm not very confident with my git skills 😆 )

Alexendoo · 2024-06-11T21:58:08Z

Yep, looks great thanks!

@bors r+

bors · 2024-06-11T21:58:11Z

📌 Commit c86b19f has been approved by Alexendoo

It is now in the queue for this repository.

bors · 2024-06-11T21:59:18Z

⌛ Testing commit c86b19f with merge 6f1b05b...

…lexendoo Add lint to check manual pattern char comparison This PR adds a new lint asked in #12490 This lint catches manual char comparison in pattern of string functions and propose to use `char` or array of `char` instead. As it's my first contribution i'm not feeling very safe about not matching too much or missing some cases. Thanks in advance for taking time to review and propose feedback !

bors · 2024-06-11T21:59:46Z

💔 Test failed - checks-action_test

Alexendoo · 2024-06-11T22:14:37Z

@bors retry

bors · 2024-06-11T22:15:45Z

⌛ Testing commit c86b19f with merge 740b72e...

bors · 2024-06-11T22:25:21Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing 740b72e to master...

AurelienFT · 2024-06-11T23:30:59Z

@Alexendoo I want to make you a special thanks for being so nice an pedagogy for this PR, loved it. You are really kind !

Alexendoo · 2024-06-12T22:37:43Z

Thank you! Congrats on the first contribution

Summary: Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`: * Feature stabilizations: * `io_slice_advance` ([#62726](rust-lang/rust#62726)) * `panic_info_message` ([#66745](rust-lang/rust#66745)) * `fs_try_exists` ([#83186](rust-lang/rust#83186)) * `lint_reasons` ([#120924](rust-lang/rust#120924)) * `duration_abs_diff` ([#117618](rust-lang/rust#117618)) * `effects` ([#102090](rust-lang/rust#102090)) * New `clippy` lints: * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287)) * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849)) * Other: * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748)) * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174)) * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355)) ignore-conflict-markers Reviewed By: zertosh, dtolnay Differential Revision: D63864870 fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262

rustbot assigned Alexendoo May 25, 2024

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

Alexendoo reviewed May 25, 2024

View reviewed changes

tests/ui/search_is_some.rs Outdated Show resolved Hide resolved

clippy_lints/src/methods/manual_pattern_char_comparison.rs Outdated Show resolved Hide resolved

clippy_lints/src/methods/manual_pattern_char_comparison.rs Outdated Show resolved Hide resolved

AurelienFT requested a review from Alexendoo May 26, 2024 18:54

Alexendoo reviewed May 27, 2024

View reviewed changes

AurelienFT requested a review from Alexendoo May 28, 2024 09:34

Alexendoo reviewed Jun 2, 2024

View reviewed changes

Alexendoo added S-final-comment-period Status: final comment period it will be merged unless new objections are raised (~1 week) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jun 2, 2024

AurelienFT force-pushed the manual-char-comparison-pattern branch from 06aae81 to 9939549 Compare June 5, 2024 09:56

Alexendoo reviewed Jun 6, 2024

View reviewed changes

clippy_lints/src/string_patterns.rs Outdated Show resolved Hide resolved

clippy_lints/src/string_patterns.rs Outdated Show resolved Hide resolved

clippy_lints/src/string_patterns.rs Outdated Show resolved Hide resolved

clippy_lints/src/string_patterns.rs Outdated Show resolved Hide resolved

AurelienFT force-pushed the manual-char-comparison-pattern branch from 48f9915 to 6dbcf14 Compare June 7, 2024 19:50

AurelienFT force-pushed the manual-char-comparison-pattern branch from 6dbcf14 to 73adcce Compare June 11, 2024 13:29

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jun 11, 2024

Add lint to check manual pattern char comparison and merge its code w…

c86b19f

…ith single_char_pattern lint

AurelienFT force-pushed the manual-char-comparison-pattern branch from c658805 to c86b19f Compare June 11, 2024 19:56

rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jun 11, 2024

Alexendoo approved these changes Jun 11, 2024

View reviewed changes

bors merged commit 740b72e into rust-lang:master Jun 11, 2024
8 checks passed

taiki-e mentioned this pull request Jun 15, 2024

manual_pattern_char_comparison should respect MSRV #12936

Closed

y21 mentioned this pull request Nov 16, 2024

Manual char string search pattern #12490

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add lint to check manual pattern char comparison #12849

Add lint to check manual pattern char comparison #12849

AurelienFT commented May 25, 2024 •

edited by Alexendoo

Loading

rustbot commented May 25, 2024

AurelienFT commented May 28, 2024 •

edited

Loading

AurelienFT commented May 29, 2024

Alexendoo commented May 30, 2024

Alexendoo commented Jun 2, 2024

bors commented Jun 3, 2024

AurelienFT commented Jun 7, 2024

bors commented Jun 11, 2024

Alexendoo commented Jun 11, 2024

rustbot commented Jun 11, 2024

AurelienFT commented Jun 11, 2024

Alexendoo commented Jun 11, 2024

bors commented Jun 11, 2024

bors commented Jun 11, 2024

bors commented Jun 11, 2024

Alexendoo commented Jun 11, 2024

bors commented Jun 11, 2024

bors commented Jun 11, 2024

AurelienFT commented Jun 11, 2024

Alexendoo commented Jun 12, 2024

Add lint to check manual pattern char comparison #12849

Add lint to check manual pattern char comparison #12849

Conversation

AurelienFT commented May 25, 2024 • edited by Alexendoo Loading

rustbot commented May 25, 2024

AurelienFT commented May 28, 2024 • edited Loading

AurelienFT commented May 29, 2024

Alexendoo commented May 30, 2024

Alexendoo commented Jun 2, 2024

bors commented Jun 3, 2024

AurelienFT commented Jun 7, 2024

bors commented Jun 11, 2024

Alexendoo commented Jun 11, 2024

rustbot commented Jun 11, 2024

AurelienFT commented Jun 11, 2024

Alexendoo commented Jun 11, 2024

bors commented Jun 11, 2024

bors commented Jun 11, 2024

bors commented Jun 11, 2024

Alexendoo commented Jun 11, 2024

bors commented Jun 11, 2024

bors commented Jun 11, 2024

AurelienFT commented Jun 11, 2024

Alexendoo commented Jun 12, 2024

AurelienFT commented May 25, 2024 •

edited by Alexendoo

Loading

AurelienFT commented May 28, 2024 •

edited

Loading