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

Improve suggestions for several lints #6197

Merged
merged 5 commits into from
Oct 30, 2020
Merged

Improve suggestions for several lints #6197

merged 5 commits into from
Oct 30, 2020

Conversation

ThibsG
Copy link
Contributor

@ThibsG ThibsG commented Oct 19, 2020

This PR is a follow-up of this Zulip discussion.

It unifies placeholders for methods module and improves several suggestions for filter_next, filter_map_next and map_unwrap_or lints.

changelog: none

@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 Oct 19, 2020
ebroto
ebroto suggested changes Oct 21, 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.

Thanks for doing this!

Shouldn't we move the tests that now have MachineApplicable to separate files enabling run-rustfix?

clippy_lints/src/methods/mod.rs 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
clippy_lints/src/methods/mod.rs Show resolved Hide resolved
clippy_lints/src/methods/mod.rs Show resolved Hide resolved
clippy_lints/src/methods/mod.rs Show resolved Hide resolved
clippy_lints/src/methods/mod.rs Show resolved Hide resolved
tests/ui/map_unwrap_or.stderr Outdated
| |_____________________________^
|
= note: replace `map(|x| x + 1).unwrap_or_else(|| 0)` with `map_or_else(|| 0, |x| x + 1)`
| |_____________________________^ help: try this: `opt.map_or_else(|| 0, |x| x + 1)`
Copy link
Member

Choose a reason for hiding this comment

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

much nicer and auto-applicable ❤️

@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 Oct 21, 2020
@ThibsG
Copy link
Contributor Author

ThibsG commented Oct 22, 2020 •
edited
Loading

Shouldn't we move the tests that now have MachineApplicable to separate files enabling run-rustfix?

Indeed. I hesitated to do so, as it hasn't been done before and is not so much work to do, so I was thinking maybe there was previous issues on this lint for auto-applicable suggestions 😄

@ebroto
Copy link
Member

ebroto commented Oct 22, 2020

so I was thinking maybe there was previous issues on this lint for auto-applicable suggestions

I think those may predate auto-applicable suggestions, so that could be a reason it wasn't done before. I think we can give this a shot if you agree.

@ThibsG ThibsG force-pushed the ImproveFilterNext branch from 9150895 to e75534e Compare October 23, 2020 07:24
@ThibsG
Copy link
Contributor Author

ThibsG commented Oct 23, 2020 •
edited
Loading

I wonder why multi-line cases are not giving suggestions.
The span seems to be well boxed, Do multi-lines are more error-prone by definition?

@ebroto ebroto 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 Oct 25, 2020
ebroto
ebroto suggested changes Oct 25, 2020
View reviewed changes
tests/ui/map_unwrap_or_else_fixable.rs Outdated Show resolved Hide resolved
tests/ui/map_unwrap_or.rs Outdated Show resolved Hide resolved
tests/ui/map_unwrap_or_else_fixable.fixed Outdated Show resolved Hide resolved
tests/ui/map_unwrap_or_else_fixable.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 Oct 25, 2020
@ThibsG ThibsG force-pushed the ImproveFilterNext branch from 9a203c9 to c0dd1f9 Compare October 26, 2020 10:16
@ebroto
Copy link
Member

ebroto commented Oct 30, 2020

@bors r+

Thanks!!

@bors
Copy link
Contributor

bors commented Oct 30, 2020

📌 Commit c0dd1f9 has been approved by ebroto

@bors
Copy link
Contributor

bors commented Oct 30, 2020

⌛ Testing commit c0dd1f9 with merge 7387b87...

@bors
Copy link
Contributor

bors commented Oct 30, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: ebroto
Pushing 7387b87 to master...

@bors bors merged commit 7387b87 into rust-lang:master Oct 30, 2020
This was referenced Oct 30, 2020
Merged
Merged
Closed
@ThibsG ThibsG deleted the ImproveFilterNext branch October 30, 2020 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebroto ebroto ebroto approved these changes

Assignees

@ebroto ebroto

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.
4 participants
@ThibsG @rust-highfive @ebroto @bors