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.

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

Add lint for 'field_reassign_with_default` #568 #4761

Closed
wants to merge 8 commits into from

Conversation

hegza
Copy link

@hegza hegza commented Nov 1, 2019

changelog: Add lint for field_reassign_with_default that checks if mutable object + field modification is used to edit a binding initialized with Default::default() instead of struct constructor.

Fixes #568

Notes:

  • Checks for immediate reassignment of fields of a binding initialized with Default::default().
  • Implemented using EarlyLintPass, might be future proofed better with LateLintPass.
  • Does not trigger if Default::default() is used via another type implementing Default.

@hegza hegza force-pushed the issue-568 branch 2 times, most recently from 7cf3338 to 94ce83e Compare November 1, 2019 23:05
@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 3, 2019
@bors
Copy link
Contributor

bors commented Nov 7, 2019

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

@flip1995
Copy link
Member

Thanks for implementing this and sorry for not replying. This went under my radar.

Do you think, you could enhance this lint, to also catch more than one assignment. So that

let a = Default::default();
a.i = 42;
a.j = 43;

suggests

let a = A { i: 42, j: 43, ..Default::default() };

@hegza
Copy link
Author

hegza commented Feb 16, 2020

Ah, yes, sure, doesn't look too difficult. I'm sure I intended it that way but forgot about it somehow.

@bors
Copy link
Contributor

bors commented Feb 21, 2020

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

@flip1995 flip1995 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 Feb 21, 2020
@phansch
Copy link
Member

phansch commented Apr 15, 2020

Hi @hegza, are you still interested in continuing the PR? As far as I can tell the last suggestion isn't implemented yet; or there's no test case for it at least.

Let us know if you need any help =)

@hegza
Copy link
Author

hegza commented Apr 18, 2020

Yeah, sorry, trying to get papers out at work and really having to push it down on the priority list. I'm interested, just busy.

@flip1995
Copy link
Member

Thanks for contributing to Clippy! Sadly this PR was not updated in quite some time. If you waited on input from a reviewer, we're sorry that this fell under the radar. If you want to continue to work on this, just reopen the PR and/or ping a reviewer.

@flip1995 flip1995 closed this May 25, 2020
@flip1995 flip1995 added S-inactive-closed Status: Closed due to inactivity 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
bors added a commit that referenced this pull request Nov 4, 2020
Add lint for 'field_reassign_with_default` #568

changelog: Add lint for field_reassign_with_default that checks if mutable object + field modification is used to edit a binding initialized with Default::default() instead of struct constructor.

Fixes #568

Notes:
- Checks for reassignment of one or more fields of a binding initialized with Default::default().
- Implemented using EarlyLintPass, might be future proofed better with LateLintPass.
- Does not trigger if Default::default() is used via another type implementing Default.
- This is a re-open of [PR#4761](#4761), but I couldn't figure out how to re-open that one so here's a new one with the requested changes :S
bors added a commit that referenced this pull request Nov 4, 2020
Add lint for 'field_reassign_with_default` #568

changelog: Add lint for field_reassign_with_default that checks if mutable object + field modification is used to edit a binding initialized with Default::default() instead of struct constructor.

Fixes #568

Notes:
- Checks for reassignment of one or more fields of a binding initialized with Default::default().
- Implemented using EarlyLintPass, might be future proofed better with LateLintPass.
- Does not trigger if Default::default() is used via another type implementing Default.
- This is a re-open of [PR#4761](#4761), but I couldn't figure out how to re-open that one so here's a new one with the requested changes :S
@HKalbasi
Copy link
Member

finished in #5911
@rustbot label -S-inactive-closed

@rustbot rustbot removed the S-inactive-closed Status: Closed due to inactivity label Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging this pull request may close these issues.

suggest struct constructor instead of mutable object + field mod
6 participants