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

Address unnecessary_to_owned false positive #8794

Merged
merged 1 commit into from
May 7, 2022

Conversation

smoelius
Copy link
Contributor

@smoelius smoelius commented May 6, 2022

My proposed fix for #8759 is to revise the conditions that delineate redundant_clone and unnecessary_to_owned:

        // Only flag cases satisfying at least one of the following three conditions:
        // * the referent and receiver types are distinct
        // * the referent/receiver type is a copyable array
        // * the method is `Cow::into_owned`
        // This restriction is to ensure there is no overlap between `redundant_clone` and this
        // lint. It also avoids the following false positive:
        //  https://github.com/rust-lang/rust-clippy/issues/8759
        //   Arrays are a bit of a corner case. Non-copyable arrays are handled by
        // `redundant_clone`, but copyable arrays are not.

This change causes a few cases that were previously flagged by unnecessary_to_owned to no longer be flagged. But one could argue those cases would be better handled by redundant_clone.

Closes #8759

changelog: none

@rust-highfive
Copy link

r? @llogiq

(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 May 6, 2022
@llogiq
Copy link
Contributor

llogiq commented May 7, 2022

That sounds sensible. Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented May 7, 2022

📌 Commit 91a822c has been approved by llogiq

@bors
Copy link
Contributor

bors commented May 7, 2022

⌛ Testing commit 91a822c with merge 9c78883...

@bors
Copy link
Contributor

bors commented May 7, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 9c78883 to master...

@bors bors merged commit 9c78883 into rust-lang:master May 7, 2022
@smoelius smoelius deleted the fix-8759 branch May 7, 2022 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unnecessary_to_owned false positive
4 participants