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

[needless_pass_by_ref_mut]: Do not lint if passed as a fn-like argument #11207

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

Centri3
Copy link
Member

@Centri3 Centri3 commented Jul 21, 2023

Fixes #11182 and also fixes #11199 (though this is kind of a duplicate)

There's likely a case or two I've missed, so this likely needs a bit more work but it seems to work fine with the tests I've added.

PS, the diff for the test is useless because it iterates over a hashmap before linting. Seems to work fine but we could maybe change this for consistency's sake

changelog: [needless_pass_by_ref_mut]: No longer lints if the function is passed as a fn-like argument

@rustbot
Copy link
Collaborator

rustbot commented Jul 21, 2023

r? @giraffate

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 21, 2023
@Centri3 Centri3 changed the title Do not lint if used as &mut closure [needless_pass_by_ref_mut]: Do not lint if passed as a fn-like argument Jul 21, 2023
@Centri3 Centri3 force-pushed the #11182 branch 2 times, most recently from 929e266 to 3a3961a Compare July 22, 2023 11:19
@y21
Copy link
Member

y21 commented Jul 22, 2023

I think this fix would still lint incorrectly on my provided example in one of the linked issues: #11199 (comment)
Probably makes sense to have that as a test as well
Edit: Oh that's even a test, I'm blind. 😅

@Centri3
Copy link
Member Author

Centri3 commented Jul 22, 2023

Yeah and I believe it'll still lint stuff like

fn then(a: &mut u32) {}

fn els(a: &mut u32) {
    *a = 2;
}

if true {
    then
} else {
    els
}

Which isn't good

@bors
Copy link
Collaborator

bors commented Jul 23, 2023

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

@GuillaumeGomez
Copy link
Member

Looks good to me, great work!

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Jul 25, 2023

📌 Commit 7b5ea7a has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 25, 2023

⌛ Testing commit 7b5ea7a with merge 59a8c53...

bors added a commit that referenced this pull request Jul 25, 2023
[`needless_pass_by_ref_mut`]: Do not lint if passed as a fn-like argument

Fixes #11182 and also fixes #11199 (though this is kind of a duplicate)

There's likely a case or two I've missed, so this likely needs a bit more work but it seems to work fine with the tests I've added.

PS, the diff for the test is useless because it iterates over a hashmap before linting. Seems to work fine but we could maybe change this for consistency's sake

changelog: [`needless_pass_by_ref_mut`]: No longer lints if the function is passed as a fn-like argument
@bors
Copy link
Collaborator

bors commented Jul 25, 2023

💔 Test failed - checks-action_test

@Centri3
Copy link
Member Author

Centri3 commented Jul 25, 2023

Looks like dogfood failed because of #10120.
@bors r=giraffate

@bors
Copy link
Collaborator

bors commented Jul 25, 2023

📌 Commit ef482d1 has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 25, 2023

⌛ Testing commit ef482d1 with merge 43b0e11...

@bors
Copy link
Collaborator

bors commented Jul 25, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 43b0e11 to master...

@bors bors merged commit 43b0e11 into rust-lang:master Jul 25, 2023
4 checks passed
@Centri3 Centri3 deleted the #11182 branch July 25, 2023 00:44
@bors bors mentioned this pull request Jul 25, 2023
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
6 participants