Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

single_char_insert_str: lint using insert_str() on single-char literals and suggest insert() #6037

Merged
merged 5 commits into from
Nov 3, 2020
Merged

single_char_insert_str: lint using insert_str() on single-char literals and suggest insert() #6037

merged 5 commits into from
Nov 3, 2020

Conversation

matthiaskrgr
Copy link
Member

Fixes #6026

changelog: add single_char_insert_str lint which lints using string.insert_str() with single char literals and suggests string.insert() with a char

@rust-highfive
Copy link

r? @ebroto

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 13, 2020
ebroto
ebroto suggested changes Sep 20, 2020
View reviewed changes
Copy link
Member

@ebroto ebroto left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in reviewing, I did not have access to my computer during this week.

In the related issue you were thinking about extending single_char_push_str, but ended up adding a new lint. Out of curiosity, what made you change your mind? (I'm fine with both options).

clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved
tests/ui/single_char_push_str.fixed Outdated Show resolved Hide resolved
@ebroto ebroto 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 Sep 21, 2020
@matthiaskrgr
Copy link
Member Author

I made it a separate lint because this way users could disable push_str and insert_str independently but it's probably better to just make it one lint since the patterns are so similar.

@matthiaskrgr matthiaskrgr force-pushed the single_char_insert branch 2 times, most recently from 9f5d8bb to 956a847 Compare September 24, 2020 14:40
@matthiaskrgr matthiaskrgr 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 Sep 24, 2020
ebroto
ebroto suggested changes Sep 24, 2020
View reviewed changes
Copy link
Member

@ebroto ebroto left a comment

Choose a reason for hiding this comment

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

I've added some remarks after the unification of the lints

tests/ui/single_char_pattern.fixed Outdated Show resolved Hide resolved
clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved
@ebroto ebroto 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 Sep 24, 2020
@ebroto
Copy link
Member

ebroto commented Oct 7, 2020

I've addressed the remaining remarks in the PR.

r? @flip1995 for my changes

@rust-highfive rust-highfive assigned flip1995 and unassigned ebroto Oct 7, 2020
flip1995
flip1995 reviewed Oct 10, 2020
View reviewed changes
clippy_lints/src/lib.rs Show resolved Hide resolved
clippy_lints/src/methods/mod.rs Outdated Show resolved Hide resolved
tests/ui/single_char_add_str.rs Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Oct 30, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

matthiaskrgr and others added 4 commits October 30, 2020 23:28
@matthiaskrgr @ebroto
single_char_push_str lint using insert_str() on single-char literals …
2350ee7 
…and suggest insert()

changelog: single_char_push_str: lint using string.insert_str() with single char literals and suggests string.insert() with a char

Fixes rust-lang#6026
@matthiaskrgr @ebroto
handle macros returning Strings in single_char_push_str and single_ch…
c6412ae 
…ar_insert_str
@matthiaskrgr @ebroto
get_hint_if_single_char_arg: fix bug where multi-char letters are not…
c1eb8ce 
… detected properly
@ebroto
Rename single_char_push_str to single_char_add_str
d958269
@flip1995
Copy link
Member

flip1995 commented Nov 3, 2020

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Nov 3, 2020

📌 Commit f8ac1f9 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Nov 3, 2020

⌛ Testing commit f8ac1f9 with merge 3ee9c2e...

@bors
Copy link
Contributor

bors commented Nov 3, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 3ee9c2e to master...

@bors bors merged commit 3ee9c2e into rust-lang:master Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 left review comments

@alex-700 alex-700 alex-700 left review comments

@ebroto ebroto ebroto requested changes

Assignees

@flip1995 flip1995

Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

single-char-push-str: expand to catch insert_str(n, "x") and suggest insert()
6 participants
@matthiaskrgr @rust-highfive @ebroto @bors @flip1995 @alex-700