add lint for recreation of an entire struct

[phi-gamma]

This lint makes Clippy warn about situations where an owned struct is
essentially recreated by moving all its fields into a new instance of
the struct. The lint is not machine-applicable because the source
struct may have been partially moved.

This lint originated in something I spotted during peer review. While
working on their branch a colleague ended up with a commit where a
function returned a struct that 1:1 replicated one of its owned inputs
from its members. Initially I suspected they hadn’t run their code
through Clippy but AFAICS there is no lint for this situation yet.

changelog: new lint: [redundant_owned_struct_recreation]

New lint checklist

• [+] Followed [lint naming conventions][lint_naming]
• [+] Added passing UI tests (including committed .stderr file)
• [+] cargo test passes locally
• [+] Executed cargo dev update_lints
• [+] Added lint documentation
• [+] Run cargo dev fmt

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @y21 (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

[phi-gamma]

Hi, is there a way to tell in a late pass whether one or more
of a struct’s members have been moved? That might enable
machine-applicability of this lint.

[y21]

This might be better as an addition to the unnecessary_struct_initialization lint. It has essentially the same idea as this one, except it only emits a warning for Foo { ..foo } (w/o any fields), so it could be improved to also emit a warning if all fields copy from the base.

In particular, this lint also suffers from the same issues that that other lint does, which is that struct reinitialization only requires the fields to be Copy, whereas this lint's suggestion forces a move of the whole struct, which, if the value is later used again doesn't work if the struct is not Copy, and is also the reason why that other lint is in nursery (#10547 is the issue).

[phi-gamma]

Thanks for the pointer, I’ll look into folding it into ``unnecessary_struct_initialization``!

In particular, this lint also suffers from the same issues that that other lint does, which is that struct reinitialization only requires the fields to be `Copy`, whereas this lint's suggestion forces a move of the whole struct, which doesn't work if the struct is not `Copy`, and is also the reason why that other lint is in `nursery` (#10547 is the issue).

Would it be sufficient to restrict the lint to ``Copy`` types?

[y21]

I thought about this for a bit and yeah, I think restricting it to only Copy types probably fixes the linked issue, but it might also regress too many "true positives" where a warning would be fine.

Maybe what we could do to avoid false positives while still warning on some useful cases would be to not emit a warning if:

• the struct itself is not Copy
• but any of its fields are

My reasoning is that if the struct is not Copy and none of its fields are, then Foo { bar: foo.bar }; also moves the entire struct effectively just like foo; does and in either case no fields are accessible afterwards, so the suggested code shouldn't result in more errors. If however there is one Copy field and one non-Copy field, then Foo { bar: foo.bar, baz: foo.baz }; would still allow using the Copy field whereas foo; again makes it inaccessible.

---

However I think for the purposes of this PR it would be easier to just not worry about the possible false positives that had already existed and only extend the unnecessary_struct_initialization lint to also catch Foo { bar: foo.bar } (in addition to what it already does).

Fixing those false positives could be done separately and the lint is in the nursery category, so that's fine.

[phi-gamma]

*However* I think for the purposes of this PR it would be easier to just not worry about the possible false positives that had already existed and only extend the `unnecessary_struct_initialization` lint to also catch `Foo { bar: foo.bar }` (in addition to what it already does).

I’ll take a stab at this.

Fixing those false positives could be done separately and the lint is in the `nursery` category, so that's fine.

Understood, I’ll save that for another PR. Thanks once more for the thorough response!