Unnecessary use of `.ok()` followed by method equivalently defined on both `Option` and `Result`

What it does

There is an existing lint, ok_expect, that warns against the following code:

res.ok().expect("msg")

saying that the .ok() can be removed and expect called directly. This can be generalized to other methods that can be called on both Option and Result.

The methods I could find that this could apply to include:

• expect
• unwrap, unwrap_or, unwrap_or_default, unwrap_unchecked
• iter, iter_mut, into_iter
• map_or

I'm sure I've missed some, please add any others that could be included!

The lazily evaluated _else methods could also be included with the minor modification of adding a ignored argument to the closure for the error, e.g. res.ok().unwrap_or_else(|| 4) becomes res.unwrap_or_else(|_| 4).

This could also be applied to calls to methods with multiple arguments if .ok() is called on both (e.g. .or(), .and(), .cmp()), though this is likely less common.

The reverse lint could also be implemented, e.g. unnecessary use of ok_or(_else) on an Option followed by a method above that will never construct the Err. The call to ok_or_else could have side effects, however, making the suggested code not equivalent.

Lint Name

unnecessary_ok

Category

complexity, perf

Advantage

• Removes unnecessary code, improving clarity
• Removes an unnecessary conversion (I don't know much about compiler internals---this may be a nonexistent performance change)

Drawbacks

Might not be a common enough pattern to be worth it. I've personally seen .ok().map_or.

The unwrap_* methods require that the Err variant implement Debug, which otherwise wouldn't be necessary.

Example

let value = <Result<_, i32>>::Ok(4);
value.ok().unwrap(); // or any of the above methods

Could be written as:

let value = <Result<_, i32>>::Ok(4);
value.unwrap();

Start by examining the existing ok_expect lint and the Option and Result methods listed in the issue. Determine which conversions can be removed while preserving behavior, including the noted closure and Debug constraints. Done means defining the scope of unnecessary_ok and implementing consistent diagnostics for the supported cases.