Skip to content

unnecessary_map_or suggests code that does not compile #14201

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

Closed
Kobzol opened this issue Feb 12, 2025 · 5 comments · Fixed by #14207
Closed

unnecessary_map_or suggests code that does not compile #14201

Kobzol opened this issue Feb 12, 2025 · 5 comments · Fixed by #14207
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@Kobzol
Copy link
Contributor

Kobzol commented Feb 12, 2025

Summary

The unnecessary_map_or lint can produce a suggestion that causes the resulting code to not compile. It suggests replacing map_or(false, |v| v == x) with == Some(x), but the problem is that is x isn't Copy, then Some(x) moves x, and it then cannot be used further.

Lint Name

unnecessary_map_or

Reproducer

I tried this code:

fn get_foo() -> Option<String> { None }

fn main() {
    let s = String::from("foo");
    let eq = get_foo().map_or(false, |v| v == s); // Flagged with clippy::unnecessary_map_or
    //let eq = get_foo() == Some(s); // Suggested replacement. Does not compile, because it moves `s`

    println!("{s}");
}

Version

rustc 1.84.1 (e71f9a9a9 2025-01-27)
binary: rustc
commit-hash: e71f9a9a98b0faf423844bf0ba7438f29dc27d58
commit-date: 2025-01-27
host: x86_64-unknown-linux-gnu
release: 1.84.1
LLVM version: 19.1.5

Additional Labels

No response

@Kobzol Kobzol added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Feb 12, 2025
@y21
Copy link
Member

y21 commented Feb 12, 2025

I guess we could still suggest get_foo().as_ref() == Some(&s) for !Copy types? (Or fall back to the more general get_foo().is_some_and(|v| v == s))

@Kobzol
Copy link
Contributor Author

Kobzol commented Feb 12, 2025

I used as_ref to solve this issue in the code where the lint was fired. Not sure if it works in all cases, but if the types match, it probably should.

@spirali
Copy link

spirali commented Feb 12, 2025

@rustbot claim

@samueltardieu
Copy link
Contributor

@spirali Oops, I didn't see that you claimed it (my browser was open on this page and didn't refresh). If you want I can close my PR and let you do yours.

@spirali
Copy link

spirali commented Feb 12, 2025

@samueltardieu No problem, do not close your PR. I can still learn on this and then compare our solutions or I will try to find another entry task.

github-merge-queue bot pushed a commit that referenced this issue Feb 15, 2025
…not implement `Copy` (#14207)

Fix #14201

changelog: [`unnecessary_map_or`]: do not consume the comparison value
if it does not implement `Copy`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
4 participants