Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

new_without_default: do not suggest deriving #5616

Merged
merged 1 commit into from
May 25, 2020
Merged

new_without_default: do not suggest deriving #5616

merged 1 commit into from
May 25, 2020

Conversation

euclio
Copy link
Contributor

@euclio euclio commented May 18, 2020

changelog: do not suggest deriving Default in new_without_default

This commit changes the behavior of the new_without_default lint to not suggest deriving Default. This suggestion is misleading if the new implementation does something different than what a derived Default implementation would do, because then the two methods would not be equivalent.

Instead, the can_derive_default check is removed, and we always suggest implementing Default in terms of new().

@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 19, 2020
@phansch phansch self-requested a review May 22, 2020 07:31
phansch
phansch suggested changes May 22, 2020
View reviewed changes
Copy link
Member

@phansch phansch left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think the lint description also needs to be updated:

https://github.com/euclio/rust-clippy/blob/no-derive-suggestion/clippy_lints/src/new_without_default.rs#L20

LGTM otherwise, thanks!

@phansch phansch 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 May 22, 2020
@euclio euclio force-pushed the no-derive-suggestion branch from 92c564a to a578bed Compare May 22, 2020 15:50
@flip1995 flip1995 added S-waiting-on-bors Status: The marked PR was approved and is only waiting bors and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 25, 2020
@flip1995
Copy link
Member

@bors r=phansch,flip1995

@bors
Copy link
Contributor

bors commented May 25, 2020

📌 Commit a578bed has been approved by phansch,flip1995

@bors
Copy link
Contributor

bors commented May 25, 2020

⌛ Testing commit a578bed with merge c41916d...

@bors
Copy link
Contributor

bors commented May 25, 2020

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

@bors bors merged commit c41916d into rust-lang:master May 25, 2020
@euclio euclio deleted the no-derive-suggestion branch May 25, 2020 18:42
This was referenced Dec 10, 2020
Open
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@phansch phansch phansch requested changes

Assignees
No one assigned
Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@euclio @flip1995 @bors @phansch