new lint: [or_else_then_unwrap]

CHANGELOG.md

@@ -6643,6 +6643,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

@@ -442,6 +442,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

@@ -89,6 +89,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;
@@ -918,6 +919,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(..))`,
@@ -4706,6 +4743,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,
@@ -5500,6 +5538,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,66 @@
+use crate::clippy_utils::res::{MaybeDef, MaybeQPath};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_errors::Applicability;
+use rustc_hir::lang_items::LangItem;
+use rustc_hir::{Body, Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty::AdtDef;
+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, or_else_arg_content) = match ty
+        .ty_adt_def()
+        .map(AdtDef::did)
+        .and_then(|did| cx.tcx.get_diagnostic_name(did))
+    {
+        Some(sym::Option)
+            if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::OptionSome) =>
+        {
+            ("found `.or_else(|| Some(…)).unwrap()`", content)
+        },
+        Some(sym::Result)
+            if let Some(content) = get_content_if_ctor_matches_in_closure(cx, or_else_arg, LangItem::ResultOk) =>
+        {
+            ("found `.or_else(|| Ok(…)).unwrap()`", content)
+        },
+        // 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)
+    );
+
+    let span = unwrap_expr.span.with_lo(or_span.lo());
+    span_lint_and_then(cx, OR_ELSE_THEN_UNWRAP, span, title, |diag| {
+        diag.span_suggestion_verbose(span, "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
+        && let Body {
+            params: [],
+            value: body,
+        } = cx.tcx.hir_body(closure.body)
+        && let ExprKind::Call(some_or_ok, [arg]) = body.kind
+        && some_or_ok.res(cx).ctor_parent(cx).is_lang_item(cx, item)
+    {
+        Some(arg.span.source_callsite())
+    } else {
+        None
+    }
+}

tests/ui/or_else_then_unwrap.fixed

@@ -0,0 +1,74 @@
+//@aux-build:proc_macros.rs
+
+#![warn(clippy::or_then_unwrap)]
+#![allow(clippy::map_identity, clippy::let_unit_value, clippy::unnecessary_literal_unwrap)]
+
+extern crate proc_macros;
+
+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")); //~ 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_else` 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
+
+    // We don't lint external macros
+    proc_macros::external! {
+       let option: Option<Wrapper> = None;
+       let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap();
+    };
+}

tests/ui/or_else_then_unwrap.rs

@@ -0,0 +1,77 @@
+//@aux-build:proc_macros.rs
+
+#![warn(clippy::or_then_unwrap)]
+#![allow(clippy::map_identity, clippy::let_unit_value, clippy::unnecessary_literal_unwrap)]
+
+extern crate proc_macros;
+
+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(); //~ 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_else` 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
+
+    // We don't lint external macros
+    proc_macros::external! {
+       let option: Option<Wrapper> = None;
+       let _ = option.or_else(|| Some(Wrapper::new("fallback"))).unwrap();
+    };
+}

tests/ui/or_else_then_unwrap.stderr

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