manual_ok_err prevents type ascription

[tamird]

Summary

There are some cases where a match expression provides more type information than a Result::ok call, such as when the Err arm ascribes the type. This gives the reader more information about the error being discarded. manual_ok_err should not trigger in this case.

@samueltardieu @blyxyas

Lint Name

manual_ok_err

Reproducer

I tried this code:

                let addr = match u64::from_str_radix(addr, 16) {
                    Ok(addr) => Some(addr),
                    Err(ParseIntError { .. }) => None,
                }?;

I saw this happen:

error: manual implementation of `ok`
   --> aya/src/util.rs:261:28
    |
261 |                   let addr = match u64::from_str_radix(addr, 16) {
    |  ____________________________^
262 | |                     Ok(addr) => Some(addr),
263 | |                     Err(ParseIntError { .. }) => None,
264 | |                 }?;
    | |_________________^ help: replace with: `u64::from_str_radix(addr, 16).ok()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_err

I expected to see this happen:
Nothing.

Version

rustc 1.84.0 (9fc6b4312 2025-01-07)
binary: rustc
commit-hash: 9fc6b43126469e3858e2fe86cafb4f0fd5068869
commit-date: 2025-01-07
host: aarch64-apple-darwin
release: 1.84.0
LLVM version: 19.1.5

Additional Labels

No response

[tamird]

The same thing is true in the reverse case:

            match result {
                Ok(()) => None,
                Err(err) => Some(err),
            }

should also be considered a false positive - I want the reader to know (and the compiler to enforce) the type being discarded.

[samueltardieu]

Couldn't the same be said for a lot of lints which rewrite code? Note that the lint can be turned off in your Clippy configuration, either locally if you want to keep the type ascription, or globally at the project level.

[tamird]

Maybe. Do you have some specific examples?

Thanks, I'm aware of how to work around this. I filed this issue because I don't think the current behavior is correct.

[samueltardieu]

It is not incorrect IMO, given that the suggestion works and will give the very same result. What you are complaining about, and I understand what you are saying, is that you lose extra information that you had placed there for a reason (documentation, or ensuring that the signature of the function your scrutinee came from doesn't change).

My opinion on this issue, which might not be shared by others, is that this kind of usage is rare compared to the number of patterns that can be rightfully bettered by the lint, and that if you use this kind of pattern and you want to use Clippy, then it is easy to disable this particular lint either locally or globally if your code style mandates it.

As for other lints which act similarly regarding type ascription, I could come up rapidly with those two examples by looking at the manual_* lints. In those two examples, removing the type might not even be equivalent, contrary to your example, because those types may determine the type of the method receiver:

let _ = Some(127).map_or(Err("error"), |v: u64| Ok(v));

and

let _ = (0..10).filter(|n: &i32| n.checked_add(1).is_some()).map(|n: i32| n.checked_add(1).unwrap());

[tamird]

I'm not saying this lint is categorically wrong, only that it should recognize and allow this pattern.