new lint: [or_else_then_unwrap]

CHANGELOG.md

@@ -6485,6 +6485,7 @@ Released 2018-09-13
 [`option_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or_else
 [`option_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_option
 [`option_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_unwrap_used
+[`or_else_then_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_else_then_unwrap
 [`or_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call
 [`or_then_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_then_unwrap
 [`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

clippy_lints/src/declared_lints.rs

@@ -444,6 +444,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::methods::OPTION_AS_REF_DEREF_INFO,
     crate::methods::OPTION_FILTER_MAP_INFO,
     crate::methods::OPTION_MAP_OR_NONE_INFO,
+    crate::methods::OR_ELSE_THEN_UNWRAP_INFO,
     crate::methods::OR_FUN_CALL_INFO,
     crate::methods::OR_THEN_UNWRAP_INFO,
     crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,

clippy_lints/src/methods/mod.rs

@@ -87,6 +87,7 @@ mod option_as_ref_cloned;
 mod option_as_ref_deref;
 mod option_map_or_none;
 mod option_map_unwrap_or;
+mod or_else_then_unwrap;
 mod or_fun_call;
 mod or_then_unwrap;
 mod path_buf_push_overwrite;
@@ -916,6 +917,42 @@ declare_clippy_lint! {
     "using `.chars().next()` to check if a string starts with a char"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `.or_else(…).unwrap()` calls to Options and Results.
+    ///
+    /// ### Why is this bad?
+    /// You should use `.unwrap_or_else(…)` instead for clarity.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # let fallback = "fallback";
+    /// // Result
+    /// # type Error = &'static str;
+    /// # let result: Result<Vec<&str>, Error> = Err("error");
+    /// let value = result.or_else(|_| Ok::<Vec<&str>, Error>(vec![fallback])).unwrap();
+    ///
+    /// // Option
+    /// # let option: Option<Vec<&str>> = None;
+    /// let value = option.or_else(|| Some(vec![fallback])).unwrap();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # let fallback = "fallback";
+    /// // Result
+    /// # let result: Result<Vec<&str>, &str> = Err("error");
+    /// let value = result.unwrap_or_else(|_| vec![fallback]);
+    ///
+    /// // Option
+    /// # let option: Option<Vec<&str>> = None;
+    /// let value = option.unwrap_or_else(|| vec![fallback]);
+    /// ```
+    #[clippy::version = "1.90.0"]
+    pub OR_ELSE_THEN_UNWRAP,
+    complexity,
+    "checks for `.or_else(…).unwrap()` calls to Options and Results."
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for calls to `.or(foo(..))`, `.unwrap_or(foo(..))`,
@@ -4680,6 +4717,7 @@ impl_lint_pass!(Methods => [
     RESULT_MAP_OR_INTO_OPTION,
     OPTION_MAP_OR_NONE,
     BIND_INSTEAD_OF_MAP,
+    OR_ELSE_THEN_UNWRAP,
     OR_FUN_CALL,
     OR_THEN_UNWRAP,
     EXPECT_FUN_CALL,
@@ -5534,6 +5572,9 @@ impl Methods {
                         Some((sym::or, recv, [or_arg], or_span, _)) => {
                             or_then_unwrap::check(cx, expr, recv, or_arg, or_span);
                         },
+                        Some((sym::or_else, recv, [or_arg], or_span, _)) => {
+                            or_else_then_unwrap::check(cx, expr, recv, or_arg, or_span);
+                        },
                         _ => {},
                     }
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);

clippy_lints/src/methods/or_else_then_unwrap.rs

@@ -0,0 +1,81 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::{is_res_lang_ctor, path_res};
+use rustc_errors::Applicability;
+use rustc_hir::lang_items::LangItem;
+use rustc_hir::{Body, Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::{Span, sym};
+
+use super::OR_ELSE_THEN_UNWRAP;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    unwrap_expr: &Expr<'_>,
+    recv: &'tcx Expr<'tcx>,
+    or_else_arg: &'tcx Expr<'_>,
+    or_span: Span,
+) {
+    let ty = cx.typeck_results().expr_ty(recv); // get type of x (we later check if it's Option or Result)
+    let title;
+    let or_else_arg_content: Span;
+
+    if is_type_diagnostic_item(cx, ty, sym::Option) {
+        title = "found `.or_else(|| Some(…)).unwrap()`";
+        if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::OptionSome) {
+            or_else_arg_content = content;
+        } else {
+            return;
+        }
+    } else if is_type_diagnostic_item(cx, ty, sym::Result) {
+        title = "found `.or_else(|| Ok(…)).unwrap()`";
+        if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::ResultOk) {
+            or_else_arg_content = content;
+        } else {
+            return;
+        }
+    } else {
+        // Someone has implemented a struct with .or(...).unwrap() chaining,
+        // but it's not an Option or a Result, so bail
+        return;
+    }
+
+    let mut applicability = Applicability::MachineApplicable;
+    let suggestion = format!(
+        "unwrap_or_else(|| {})",
+        snippet_with_applicability(cx, or_else_arg_content, "..", &mut applicability)
+    );
+
+    span_lint_and_sugg(
+        cx,
+        OR_ELSE_THEN_UNWRAP,
+        unwrap_expr.span.with_lo(or_span.lo()),
+        title,
+        "try",
+        suggestion,
+        applicability,
+    );
+}
+
+fn get_content_if_ctor_matches_in_closure(cx: &LateContext<'_>, expr: &Expr<'_>, item: LangItem) -> Option<Span> {
+    if let ExprKind::Closure(closure) = expr.kind {
+        if let Body {
+            params: [],
+            value: body,
+        } = cx.tcx.hir_body(closure.body)
+        {
+            if let ExprKind::Call(some_expr, [arg]) = body.kind
+                && is_res_lang_ctor(cx, path_res(cx, some_expr), item)
+            {
+                Some(arg.span.source_callsite())
+            } else {
+                None
+            }
+        } else {
+            None
+        }
+    } else {
+        None
+    }
+}

tests/ui/or_else_then_unwrap.fixed

@@ -0,0 +1,66 @@
+#![warn(clippy::or_then_unwrap)]
+#![allow(clippy::map_identity, clippy::let_unit_value, clippy::unnecessary_literal_unwrap)]
+
+struct SomeStruct;
+impl SomeStruct {
+    fn or_else<F: FnOnce() -> Option<Self>>(self, _: F) -> Self {
+        self
+    }
+    fn unwrap(&self) {}
+}
+
+struct SomeOtherStruct;
+impl SomeOtherStruct {
+    fn or_else(self) -> Self {
+        self
+    }
+    fn unwrap(&self) {}
+}
+
+struct Wrapper {
+    inner: &'static str,
+}
+impl Wrapper {
+    fn new(inner: &'static str) -> Self {
+        Self { inner }
+    }
+}
+
+fn main() {
+    let option: Option<Wrapper> = None;
+    let _ = option.unwrap_or_else(|| Wrapper::new("fallback")); // should trigger lint
+    //
+    //~^^ or_else_then_unwrap
+
+    // as part of a method chain
+    let option: Option<Wrapper> = None;
+    let _ = option
+        .map(|v| v)
+        .unwrap_or_else(|| Wrapper::new("fallback"))
+        .inner
+        .to_string()
+        .chars();
+
+    // Call with macro should preserve the macro call rather than expand it
+    let option: Option<Vec<&'static str>> = None;
+    let _ = option.unwrap_or_else(|| vec!["fallback"]); // should trigger lint
+    //
+    //~^^ or_else_then_unwrap
+
+    // Not Option/Result
+    let instance = SomeStruct {};
+    let _ = instance.or_else(|| Some(SomeStruct {})).unwrap(); // should not trigger lint
+
+    // or takes no argument
+    let instance = SomeOtherStruct {};
+    let _ = instance.or_else().unwrap(); // should not trigger lint and should not panic
+
+    // None in or
+    let option: Option<Wrapper> = None;
+    #[allow(clippy::unnecessary_lazy_evaluations)]
+    let _ = option.or_else(|| None).unwrap(); // should not trigger lint
+
+    // other function between
+    let option: Option<Wrapper> = None;
+    let _ = option.or_else(|| Some(Wrapper::new("fallback"))).map(|v| v).unwrap(); // should not trigger lint
+}

tests/ui/or_else_then_unwrap.rs

@@ -0,0 +1,69 @@
+#![warn(clippy::or_then_unwrap)]
+#![allow(clippy::map_identity, clippy::let_unit_value, clippy::unnecessary_literal_unwrap)]
+
+struct SomeStruct;
+impl SomeStruct {
+    fn or_else<F: FnOnce() -> Option<Self>>(self, _: F) -> Self {
+        self
+    }
+    fn unwrap(&self) {}
+}
+
+struct SomeOtherStruct;
+impl SomeOtherStruct {
+    fn or_else(self) -> Self {
+        self
+    }
+    fn unwrap(&self) {}
+}
+
+struct Wrapper {
+    inner: &'static str,
+}
+impl Wrapper {
+    fn new(inner: &'static str) -> Self {
+        Self { inner }
+    }
+}
+
+fn main() {
+    let option: Option<Wrapper> = None;
+    let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); // should trigger lint
+    //
+    //~^^ or_else_then_unwrap
+
+    // as part of a method chain
+    let option: Option<Wrapper> = None;
+    let _ = option
+        .map(|v| v)
+        .or_else(|| Some(Wrapper::new("fallback"))) // should trigger lint
+        //
+        //~^^ or_else_then_unwrap
+        .unwrap()
+        .inner
+        .to_string()
+        .chars();
+
+    // Call with macro should preserve the macro call rather than expand it
+    let option: Option<Vec<&'static str>> = None;
+    let _ = option.or_else(|| Some(vec!["fallback"])).unwrap(); // should trigger lint
+    //
+    //~^^ or_else_then_unwrap
+
+    // Not Option/Result
+    let instance = SomeStruct {};
+    let _ = instance.or_else(|| Some(SomeStruct {})).unwrap(); // should not trigger lint
+
+    // or takes no argument
+    let instance = SomeOtherStruct {};
+    let _ = instance.or_else().unwrap(); // should not trigger lint and should not panic
+
+    // None in or
+    let option: Option<Wrapper> = None;
+    #[allow(clippy::unnecessary_lazy_evaluations)]
+    let _ = option.or_else(|| None).unwrap(); // should not trigger lint
+
+    // other function between
+    let option: Option<Wrapper> = None;
+    let _ = option.or_else(|| Some(Wrapper::new("fallback"))).map(|v| v).unwrap(); // should not trigger lint
+}

tests/ui/or_else_then_unwrap.stderr

@@ -0,0 +1,26 @@
+error: found `.or_else(|| Some(…)).unwrap()`
+  --> tests/ui/or_else_then_unwrap.rs:31:20
+   |
+LL |     let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap(); // should trigger lint
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| Wrapper::new("fallback"))`
+   |
+   = note: `-D clippy::or-else-then-unwrap` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::or_else_then_unwrap)]`
+
+error: found `.or_else(|| Some(…)).unwrap()`
+  --> tests/ui/or_else_then_unwrap.rs:39:10
+   |
+LL |           .or_else(|| Some(Wrapper::new("fallback"))) // should trigger lint
+   |  __________^
+...  |
+LL | |         .unwrap()
+   | |_________________^ help: try: `unwrap_or_else(|| Wrapper::new("fallback"))`
+
+error: found `.or_else(|| Some(…)).unwrap()`
+  --> tests/ui/or_else_then_unwrap.rs:49:20
+   |
+LL |     let _ = option.or_else(|| Some(vec!["fallback"])).unwrap(); // should trigger lint
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| vec!["fallback"])`
+
+error: aborting due to 3 previous errors
+