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

Suggest to replace Option::as_ref().cloned() with Option::replace() #12009

Closed
psychon opened this issue Dec 25, 2023 · 0 comments · Fixed by #12051
Closed

Suggest to replace Option::as_ref().cloned() with Option::replace() #12009

psychon opened this issue Dec 25, 2023 · 0 comments · Fixed by #12051
Labels
A-lint Area: New lints

Comments

@psychon
Copy link

psychon commented Dec 25, 2023

What it does

Search for code doing .as_ref().cloned() on an Option<T> and suggests to just use .clone() directly.

Advantage

  • More direct / explicit code that is hopefully easier to understand

Drawbacks

🤷

Example

Self-contained example:

fn foo(bar: &Option<Vec<u8>>) -> Option<Vec<u8>> {
    bar.as_ref().cloned()
}

Could be written as:

fn foo(bar: &Option<Vec<u8>>) -> Option<Vec<u8>> {
    bar.clone()
}

Recently, I was looking at some code in winit and failed to understand this code. There is a cache using Mutex<Option<...>>. The code here tries to either copy the value contained in the cache or fill the cache.

The first part of my problem was that I somehow thought that .as_ref().cloned() does something to the inner contents and not the Option directly

@psychon psychon added the A-lint Area: New lints label Dec 25, 2023
bors added a commit that referenced this issue Jan 3, 2024
new lint: `option_as_ref_cloned`

Closes #12009

Adds a new lint that looks for `.as_ref().cloned()` on `Option`s. That's the same as just `.clone()`-ing the option directly.

changelog: new lint: [`option_as_ref_cloned`]
@bors bors closed this as completed in ee8bfb7 Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant