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

[get_unwrap]: include a borrow in the suggestion if argument is not an integer literal #10979

Merged
merged 3 commits into from
Jun 22, 2023

Conversation

y21
Copy link
Member

@y21 y21 commented Jun 17, 2023

Fixes #9909

I have to say, I don't really understand what the previous logic was trying to do, but this fixes the linked bug.
It was checking if the argument passed to .get() can be parsed as a usize (i.e. if it's an integer literal, probably?), and if not, it wouldn't include a borrow? I don't know how we came to that conclusion, but that logic doesn't work:

let slice = &[1, 2];
let _r: &i32 = slice.get({ 1 }).unwrap();
// previous suggestion: slice[{ 1 }]
// the suggestion should be: &slice[{ 1 }]

Here the argument passed to it isn't an integer literal, but it should still include a borrow, because it would otherwise change the type from &i32 to i32.

The exception is that if the parent of the get().unwrap() expr is a dereference or a method call or the like, we don't need an explicit borrow because it's automatically inserted by the compiler

changelog: [get_unwrap]: include a borrow in the suggestion if argument is not an integer literal

@rustbot
Copy link
Collaborator

rustbot commented Jun 17, 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 Jun 17, 2023
clippy_lints/src/methods/get_unwrap.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/get_unwrap.rs Outdated Show resolved Hide resolved
tests/ui/get_unwrap.fixed Outdated Show resolved Hide resolved
@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Jun 21, 2023

📌 Commit bdb2a17 has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 21, 2023

⌛ Testing commit bdb2a17 with merge 6ec2388...

@bors
Copy link
Contributor

bors commented Jun 22, 2023

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

@bors bors merged commit 6ec2388 into rust-lang:master Jun 22, 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
Development

Successfully merging this pull request may close these issues.

get_unwrap changes mutability
4 participants