Skip to content

clippy::manual_unwrap_or_default / clippy::manual_unwrap_or suppose unwrapping self, violating trait function signature #15807

@chrxn1c

Description

@chrxn1c

Summary

I ran clippy on this code:
https://gist.github.com/rust-play/eb6d1e3b4818bbeba08a752b654cf77c

Clippy supposed it was reimplementing unwrap_or_default pattern:

warning: if let can be simplified with `.unwrap_or_default()`
  --> src/main.rs:11:32
   |
11 |         let calculated_field = if let Ok(x) = self { x } else { 0 };
   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `self.unwrap_or_default()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or_default
   = note: `#[warn(clippy::manual_unwrap_or_default)]` on by default

warning: `playground` (bin "playground") generated 1 warning (run `cargo clippy --fix --bin "playground"` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.36s

however, unwrapping consumes self, which causes inability to return owned self, as declared by trait:

pub(crate) trait ObserveSth {
    fn observe_sth(self, guard: &mut ObservationGuard) -> Self;
}

by allowing ![allow(clippy::manual_unwrap_or_default)] another warning arises, clippy::manual_unwrap_or`:

warning: this pattern reimplements `Result::unwrap_or`
  --> src/main.rs:12:32
   |
12 |         let calculated_field = if let Ok(x) = self { x } else { 0 };
   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `self.unwrap_or(0)`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or
   = note: `#[warn(clippy::manual_unwrap_or)]` on by default

warning: `playground` (bin "playground") generated 1 warning (run `cargo clippy --fix --bin "playground"` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.11s

the workaround is to use AsRef:

impl ObserveSth for Result<u64, std::io::Error> {
    fn observe_sth(self, guard: &mut MyGuard) -> Self {
        let calculated_field = self.as_ref().unwrap_or(&0);
        guard.calculated_field = *calculated_field;
        self
    }
}

but imo

  1. why should i be indirecting Copy-types?
  2. clippy's message is misleading, especially for beginners: how can you unwrap self and still return owned version of Self, as trait definition suggests?
  3. AsRef is not mentioned in any way in help-message

what do you guys think?

Lint Name

clippy::manual_unwrap_or_default, clippy::manual_unwrap_or

Reproducer

I tried this code:

pub(crate) struct ObservationGuard {
    pub(crate) calculated_field: u64,
}

pub(crate) trait ObserveSth {
    fn observe_sth(self, guard: &mut ObservationGuard) -> Self;
}

impl ObserveSth for Result<u64, std::io::Error> {
    fn observe_sth(self, guard: &mut ObservationGuard) -> Self {
        let calculated_field = if let Ok(x) = self { x } else { 0 };
        guard.calculated_field = calculated_field;
        self
    }
}

fn generate_calculated_result() -> Result<u64, std::io::Error> {
    Ok(5)
}
fn main() {
    let mut guard = ObservationGuard {
        calculated_field: 0,
    };
    let _calculated_and_observed_result = generate_calculated_result().observe_sth(&mut guard);
}

I saw this happen:

warning: if let can be simplified with `.unwrap_or_default()`
  --> src/main.rs:11:32
   |
11 |         let calculated_field = if let Ok(x) = self { x } else { 0 };
   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `self.unwrap_or_default()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or_default
   = note: `#[warn(clippy::manual_unwrap_or_default)]` on by default

warning: `playground` (bin "playground") generated 1 warning (run `cargo clippy --fix --bin "playground"` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.36s

By allowing #![allow(clippy::manual_unwrap_or_default)] i saw this happen:

warning: this pattern reimplements `Result::unwrap_or`
  --> src/main.rs:12:32
   |
12 |         let calculated_field = if let Ok(x) = self { x } else { 0 };
   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `self.unwrap_or(0)`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or
   = note: `#[warn(clippy::manual_unwrap_or)]` on by default

warning: `playground` (bin "playground") generated 1 warning (run `cargo clippy --fix --bin "playground"` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.11s

I expected this to happen:
Either no warning or suggestion to use .as_ref().unwrap_or(&T);

Version

Version:
rustc 1.90.0 stable (playground)
binary: rustc

Additional Labels

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: Clippy is not doing the correct thingI-false-positiveIssue: The lint was triggered on code it shouldn't have

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions