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

assigning_clones should respect MSRV #12511

Merged
merged 3 commits into from
Mar 19, 2024

Conversation

humannum14916
Copy link
Contributor

Fixes: #12502

This PR fixes the assigning_clones lint suggesting to use clone_from or clone_into on incompatible MSRVs.

assigning_clones will suggest using either clone_from or clone_into, both of which were stabilized in 1.63. If the current MSRV is below 1.63, the lint should not trigger.

changelog: [assigning_clones]: don't lint when the MSRV is below 1.63.

@rustbot
Copy link
Collaborator

rustbot commented Mar 19, 2024

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 19, 2024
@Alexendoo
Copy link
Member

Great, thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 19, 2024

📌 Commit 7c0b2dd has been approved by Alexendoo

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 19, 2024

⌛ Testing commit 7c0b2dd with merge 89aba8d...

@bors
Copy link
Contributor

bors commented Mar 19, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing 89aba8d to master...

@bors bors merged commit 89aba8d into rust-lang:master Mar 19, 2024
8 checks passed
@Kobzol
Copy link
Contributor

Kobzol commented Mar 20, 2024

Hi, thanks for the PR, but I'm pretty sure that clone_from was not stabilized in 1.63 (https://doc.rust-lang.org/src/core/clone.rs.html#168). This should only limit clone_into.

@humannum14916
Copy link
Contributor Author

Whoops! Sorry about that, I really should have double-checked exactly where to apply the MSRV check. I'll start working on a new PR to fix that, it looks like the proper location for the check would be in is_ok_to_suggest on a call.target of TargetTrait::ToOwned.

bors added a commit that referenced this pull request Mar 20, 2024
Make `assigning_clones` MSRV check more precise

Continuation of #12511

`clone_into` is the only suggestion subject to the 1.63 MSRV requirement, and the lint should still emit other suggestions regardless of the MSRV.

changelog: [assigning_clones]: only apply MSRV check to `clone_into` suggestions.
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.

assigning_clones should respect MSRV
5 participants