option_if_let_else suggestion does not compile

[lopopolo]

I tried this code:

#![warn(clippy::all)]
#![warn(clippy::pedantic)]

use core::convert::TryFrom;

#[derive(Clone, Copy)]
struct X;

struct Foo;

impl From<X> for Option<Foo> {
    fn from(_: X) -> Self {
        Some(Foo)
    }
}

#[derive(Clone)]
struct Bar;

impl From<X> for Option<Bar> {
    fn from(_: X) -> Self {
        Some(Bar)
    }
}

pub struct NoBlockGiven(X);

impl From<X> for NoBlockGiven {
    fn from(value: X) -> Self {
        Self(value)
    }
}

impl TryFrom<X> for Bar {
    type Error = NoBlockGiven;

    fn try_from(value: X) -> Result<Self, Self::Error> {
        if let Some(block) = value.into() {
            Ok(block)
        } else {
            Err(NoBlockGiven::from(value))
        }
    }
}

Clippy made this suggestion:

$ cargo clippy
    Checking clippy-test v0.1.0 (/Users/lopopolo/Downloads/clippy-test)
warning: use Option::map_or instead of an if let/else
  --> src/lib.rs:38:9
   |
38 | /         if let Some(block) = value.into() {
39 | |             Ok(block)
40 | |         } else {
41 | |             Err(NoBlockGiven::from(value))
42 | |         }
   | |_________^ help: try: `value.into().map_or(Err(NoBlockGiven::from(value)), |block| Ok(block))`
   |
note: the lint level is defined here
  --> src/lib.rs:2:9
   |
2  | #![warn(clippy::pedantic)]
   |         ^^^^^^^^^^^^^^^^
   = note: `#[warn(clippy::option_if_let_else)]` implied by `#[warn(clippy::pedantic)]`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.08s

Applying the fix results in code that does not compile:

#![warn(clippy::all)]
#![warn(clippy::pedantic)]

use core::convert::TryFrom;

#[derive(Clone, Copy)]
struct X;

struct Foo;

impl From<X> for Option<Foo> {
    fn from(_: X) -> Self {
        Some(Foo)
    }
}

#[derive(Clone)]
struct Bar;

impl From<X> for Option<Bar> {
    fn from(_: X) -> Self {
        Some(Bar)
    }
}

pub struct NoBlockGiven(X);

impl From<X> for NoBlockGiven {
    fn from(value: X) -> Self {
        Self(value)
    }
}

impl TryFrom<X> for Bar {
    type Error = NoBlockGiven;

    fn try_from(value: X) -> Result<Self, Self::Error> {
        value
            .into()
            .map_or(Err(NoBlockGiven::from(value)), |block| Ok(block))
    }
}

$ cargo clippy
    Checking clippy-test v0.1.0 (/Users/lopopolo/Downloads/clippy-test)
error[E0282]: type annotations needed
  --> src/lib.rs:39:14
   |
38 | /         value
39 | |             .into()
   | |______________^^^^_- this method call resolves to `T`
   |                |
   |                cannot infer type for type parameter `T` declared on the trait `Into`
   |
   = note: type must be known at this point

error: aborting due to previous error

For more information about this error, try `rustc --explain E0282`.
error: could not compile `clippy-test`.

To learn more, run the command again with --verbose.

Meta

• cargo clippy -V: clippy 0.0.212 (18bf6b4f0 2020-10-07)
• rustc -Vv:

  rustc 1.47.0 (18bf6b4f0 2020-10-07)
  binary: rustc
  commit-hash: 18bf6b4f01a6feaf7259ba7cdae58031af1b7b39
  commit-date: 2020-10-07
  host: x86_64-apple-darwin
  release: 1.47.0
  LLVM version: 11.0
  

[lopopolo]

Despite this bug report, I am planning on disabling this lint since in every case clippy offered the correction, I found the explicit if let ... else more readable than the combinator soup I never remember the argument order for.

A particular suggestion I did not like was replacing an if let .. else that was part of an if else chain.

diff --git a/spinoso-env/src/env/memory.rs b/spinoso-env/src/env/memory.rs
index 00b94517f2..af1ff8cff0 100644
--- a/spinoso-env/src/env/memory.rs
+++ b/spinoso-env/src/env/memory.rs
@@ -98,10 +98,10 @@ impl Memory {
             // MRI accepts names containing '=' on get and should always return
             // `nil` since these names are invalid at the OS level.
             Ok(None)
-        } else if let Some(value) = self.store.get(name) {
-            Ok(Some(Cow::Borrowed(value)))
         } else {
-            Ok(None)
+            self.store
+                .get(name)
+                .map_or(Ok(None), |value| Ok(Some(Cow::Borrowed(value))))
         }
     }

In this case, if this method were const, the suggestion would make the resulting code not const.

[flip1995]

One thing you could do to fix the compiler warning would be to use Option::from instead of .into(), which is admittedly worse code IMO.

Despite this bug report, I am planning on disabling this lint since in every case clippy offered the correction

That's totally fine, pedantic lints are meant to be really opinionated and to be allowed more often.