`unwrap_or_default` suggests replacing `T::new`

[Kinrany]

Summary

unwrap_or_default suggests replacing unwrap_or_else(T::new) with unwrap_or_default() when Default is implemented via new.

Lint Name

unwrap_or_default

Reproducer

I tried this code (playground):

pub struct Foo(u32);

impl Foo {
    fn new() -> Self {
        Self(0)
    }
}

impl Default for Foo {
    fn default() -> Self {
        Self::new()
    }
}

pub fn foo(x: Option<Foo>) -> Foo {
    x.unwrap_or_else(Foo::new)
}

I saw this happen:

warning: use of `unwrap_or_else` to construct default value
  --> src/lib.rs:16:7
   |
16 |     x.unwrap_or_else(Foo::new)
   |       ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_or_default
   = note: `#[warn(clippy::unwrap_or_default)]` on by default

I expected to see no warning. The implementation of Default could be changed to something other than new, potentially making all code that was changed to unwrap_or_default invalid.

Version

rustc 1.73.0 (cc66ad468 2023-10-03)
binary: rustc
commit-hash: cc66ad468955717ab92600c770da8c1601a4ff33
commit-date: 2023-10-03
host: aarch64-apple-darwin
release: 1.73.0
LLVM version: 17.0.2

Additional Labels

No response

[saecki]

This would be even clearer if the new constructor returned Self(1) or any other value than the default constructor. Like this

[bjorn-ove]

I just had this issue with the ulid crate. I used map.get("some-key").unwrap_or_else(Ulid::new), but was told unwrap_or_default() was the better option. This is obviously wrong given that Ulid::new() returns a random value, while Ulid::default() returns 0.

[Kinrany]

Yeah, turns out, for the very common case of randomly generated IDs the behavior is flat out wrong.

I feel like it needs to be moved back to nursery until we figure out what the desired behavior is.