Clippy suggestion on `unnecessary_unwrap` not applicable in function taking `&mut` struct and returning reference to contained element

[danielrainer]

Summary

The following code triggers the unnecessary_unwrap lint, which is understandable, since there is in fact a is_some check followed by an unwrap. However, rewriting the code as suggested by results in the function not compiling (see the commented-out version in the repro):

error[E0502]: cannot borrow `self.opt` as mutable because it is also borrowed as immutable
  --> src/main.rs:19:9
   |
15 |     fn repro(&mut self) -> &String {
   |              - let's call the lifetime of this reference `'1`
16 |         if let Some(val) = &self.opt {
   |                            --------- immutable borrow occurs here
17 |             return val;
   |                    --- returning this value requires that `self.opt` is borrowed for `'1`
18 |         }
19 |         self.opt.insert(String::new())
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here

For more information about this error, try `rustc --explain E0502`.

If there is a more idiomatic way of writing this, it would be nice to get a corresponding suggestion. Otherwise, it might be better to disable the lint for such cases. If it is not feasible to correctly identify cases where the current suggestion does not work, it might be acceptable to keep the current behavior and require manual deactivation of the lint for the affected code via annotations.

Reproducer

Code:

struct Wrapper {
    opt: Option<String>,
}

impl Wrapper {
    fn repro(&mut self) -> &String {
        if self.opt.is_some() {
            return self.opt.as_ref().unwrap();
        }
        self.opt.insert(String::new())
    }

    /*
     * fn repro(&mut self) -> &String {
     *     if let Some(val) = &self.opt {
     *         return val;
     *     }
     *     self.opt.insert(String::new())
     * }
     */
}

fn main() {
    let mut wrapper = Wrapper { opt: None };
    let _ = wrapper.repro();
}

Current output:

warning: called `unwrap` on `self.opt` after checking its variant with `is_some`
 --> src/main.rs:8:20
  |
7 |         if self.opt.is_some() {
  |         --------------------- help: try: `if let Some(<item>) = &self.opt`
8 |             return self.opt.as_ref().unwrap();
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
  = note: `#[warn(clippy::unnecessary_unwrap)]` on by default

Desired output:

a suggestion for more idiomatic code which compiles, or, if that's not possible, no lint for this code

Version

rustc 1.93.0-nightly (1d60f9e07 2025-12-01)
binary: rustc
commit-hash: 1d60f9e070c1039b263e0f035c0f03dfcc610d0f
commit-date: 2025-12-01
host: x86_64-unknown-linux-gnu
release: 1.93.0-nightly
LLVM version: 21.1.5

Additional Labels

@rustbot label +I-suggestion-causes-error

[danielrainer]

I realized that the suggested <item> can be ref val, which makes the suggestion work. It would still be nice to get a fitting suggestion for these cases, but this might be hard to implement.

If <item> is interpreted to possibly mean more than just an identifier, the suggestion is actually correct and this is not a bug. Therefore, I'll close this issue. Maybe it could be re-categorized as a suggestion for improving the lint.

[ada4a]

Hm, I'm afraid this is a bug in the newly added logic for field access linting (#15949), because in the field-less case:

fn repro_fieldless(opt: &mut Option<String>) -> &String {
    if opt.is_some() {
        return opt.as_ref().unwrap();
    }
    opt.insert(String::new())
}

the lint doesn't even warn -- though it technically could, suggesting the following:

fn repro_changed(opt: &mut Option<String>) -> &String {
    if let Some(s) = opt {
        return s;
    }
    opt.insert(String::new())
}

(notice also the lack of & before opt)

But that wouldn't work in the field-access case anyway: writing just self.opt will try to move the value, which we aren't allowed to, and &self.opt would create a new reference with a new lifetime, which ultimately upsets the borrow checker... It looks like ref is indeed the only possible way to fix this, so we should try to suggest that when in the field-access case -- or just not lint, as is done in the field-less case.

I do think that this is an important bug to fix, so I'd reopen this issue if that's okay.

[danielrainer]

I do think that this is an important bug to fix, so I'd reopen this issue if that's okay.

Sure, I don't mind having this issue open.