feat: add use_unwrap_or

8 files changed, 157 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2bc393d6042..9475c674983 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3538,6 +3538,7 @@ Released 2018-09-13
 [`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
 [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug
 [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self
+[`use_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_unwrap_or
 [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding
 [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref
 [`useless_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_attribute
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index 653d4daa6b6..a42f4cb6d70 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -310,6 +310,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(unwrap::PANICKING_UNWRAP),
     LintId::of(unwrap::UNNECESSARY_UNWRAP),
     LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS),
+    LintId::of(use_unwrap_or::USE_UNWRAP_OR),
     LintId::of(useless_conversion::USELESS_CONVERSION),
     LintId::of(vec::USELESS_VEC),
     LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH),
diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs
index 68d6c6ce5f7..94ff53c2a60 100644
--- a/clippy_lints/src/lib.register_complexity.rs
+++ b/clippy_lints/src/lib.register_complexity.rs
@@ -94,6 +94,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
     LintId::of(unit_types::UNIT_ARG),
     LintId::of(unnecessary_sort_by::UNNECESSARY_SORT_BY),
     LintId::of(unwrap::UNNECESSARY_UNWRAP),
+    LintId::of(use_unwrap_or::USE_UNWRAP_OR),
     LintId::of(useless_conversion::USELESS_CONVERSION),
     LintId::of(zero_div_zero::ZERO_DIVIDED_BY_ZERO),
 ])
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index 1a45763a869..d1e13647e7e 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -528,6 +528,7 @@ store.register_lints(&[
     unwrap_in_result::UNWRAP_IN_RESULT,
     upper_case_acronyms::UPPER_CASE_ACRONYMS,
     use_self::USE_SELF,
+    use_unwrap_or::USE_UNWRAP_OR,
     useless_conversion::USELESS_CONVERSION,
     vec::USELESS_VEC,
     vec_init_then_push::VEC_INIT_THEN_PUSH,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 504235d0d1e..9c9e9643bc9 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -394,6 +394,7 @@ mod unwrap;
 mod unwrap_in_result;
 mod upper_case_acronyms;
 mod use_self;
+mod use_unwrap_or;
 mod useless_conversion;
 mod vec;
 mod vec_init_then_push;
@@ -866,6 +867,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
             ignore_publish: cargo_ignore_publish,
         })
     });
+    store.register_late_pass(|| Box::new(use_unwrap_or::UseUnwrapOr));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/clippy_lints/src/use_unwrap_or.rs b/clippy_lints/src/use_unwrap_or.rs
new file mode 100644
index 00000000000..f42e28246d4
--- /dev/null
+++ b/clippy_lints/src/use_unwrap_or.rs
@@ -0,0 +1,105 @@
+use if_chain::if_chain;
+use rustc_hir::*;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{Span, sym};
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::ty::is_type_diagnostic_item;
+
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `.or(…).unwrap()` calls to Options and Results.
+    ///
+    /// ### Why is this bad?
+    /// You should use `.unwrap_or(…)` instead for clarity.
+    ///
+    /// ### Example
+    /// ```rust
+    /// // Result
+    /// let port = result.or::<Error>(Ok(fallback)).unwrap();
+    ///
+    /// // Option
+    /// let value = option.or(Some(fallback)).unwrap();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// // Result
+    /// let port = result.unwrap_or(fallback);
+    ///
+    /// // Option
+    /// let value = option.unwrap_or(fallback);
+    /// ```
+    #[clippy::version = "1.61.0"]
+    pub USE_UNWRAP_OR,
+    complexity,
+    "checks for `.or(…).unwrap()` calls to Options and Results."
+}
+declare_lint_pass!(UseUnwrapOr => [USE_UNWRAP_OR]);
+
+impl<'tcx> LateLintPass<'tcx> for UseUnwrapOr {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+
+        // look for x.or().unwrap()
+        if_chain! {
+            if let ExprKind::MethodCall(path, args, unwrap_span) = expr.kind;
+            if path.ident.name.as_str() == "unwrap";
+            if let Some(caller) = args.first();
+            if let ExprKind::MethodCall(caller_path, caller_args, or_span) = caller.kind;
+            if caller_path.ident.name.as_str() == "or";
+            then {
+                let ty = cx.typeck_results().expr_ty(&caller_args[0]); // get type of x (we later check if it's Option or Result)
+                let title;
+                let arg = &caller_args[1]; // the argument or(xyz) is called with
+
+                if is_type_diagnostic_item(&cx, ty, sym::Option) {
+                    title = ".or(Some(…)).unwrap() found";
+                    if !is(arg, "Some") {
+                        return;
+                    }
+
+                } else if is_type_diagnostic_item(&cx, ty, sym::Result) {
+                    title = ".or(Ok(…)).unwrap() found";
+                    if !is(arg, "Ok") {
+                        return;
+                    }
+                } else {
+                    // Someone has implemented a struct with .or(...).unwrap() chaining,
+                    // but it's not an Option or a Result, so bail
+                    return;
+                }
+
+                // span = or_span + unwrap_span
+                let span = Span::new(or_span.lo(), unwrap_span.hi(), or_span.ctxt(), or_span.parent());
+
+                span_lint_and_help(
+                    cx,
+                    USE_UNWRAP_OR,
+                    span,
+                    title,
+                    None,
+                    "use `unwrap_or()` instead"
+                );
+            }
+        }
+    }
+}
+
+/// is expr a Call to name?
+/// name might be "Some", "Ok", "Err", etc.
+fn is<'a>(expr: &Expr<'a>, name: &str) -> bool {
+    if_chain! {
+        if let ExprKind::Call(some_expr, _some_args) = expr.kind;
+        if let ExprKind::Path(path) = &some_expr.kind;
+        if let QPath::Resolved(_, path) = path;
+        if let Some(path_segment) = path.segments.first();
+        if path_segment.ident.name.as_str() == name;
+        then {
+            return true;
+        }
+        else {
+            return false;
+        }
+    }
+}
+
diff --git a/tests/ui/use_unwrap_or.rs b/tests/ui/use_unwrap_or.rs
new file mode 100644
index 00000000000..0bfabaae885
--- /dev/null
+++ b/tests/ui/use_unwrap_or.rs
@@ -0,0 +1,27 @@
+#![warn(clippy::use_unwrap_or)]
+
+struct SomeStruct {}
+impl SomeStruct {
+    fn or(self, _: Option<Self>) -> Self { self }
+    fn unwrap(&self){}
+}
+
+fn main() {
+    let option: Option<&str> = None;
+    let _ = option.or(Some("fallback")).unwrap(); // should trigger lint
+
+    let result: Result<&str, &str> = Err("Error");
+    let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint
+
+    // Not Option/Result
+    let instance = SomeStruct {};
+    let _ = instance.or(Some(SomeStruct {})).unwrap(); // should not trigger lint
+
+    // None in or
+    let option: Option<&str> = None;
+    let _ = option.or(None).unwrap(); // should not trigger lint
+
+    // Not Err in or
+    let result: Result<&str, &str> = Err("Error");
+    let _ = result.or::<&str>(Err("Other Error")).unwrap(); // should not trigger lint
+}
diff --git a/tests/ui/use_unwrap_or.stderr b/tests/ui/use_unwrap_or.stderr
new file mode 100644
index 00000000000..5bbba41f7ac
--- /dev/null
+++ b/tests/ui/use_unwrap_or.stderr
@@ -0,0 +1,19 @@
+error: .or(Some(…)).unwrap() found
+  --> $DIR/use_unwrap_or.rs:11:20
+   |
+LL |     let _ = option.or(Some("fallback")).unwrap(); // should trigger lint
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::use-unwrap-or` implied by `-D warnings`
+   = help: use `unwrap_or()` instead
+
+error: .or(Ok(…)).unwrap() found
+  --> $DIR/use_unwrap_or.rs:14:20
+   |
+LL |     let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use `unwrap_or()` instead
+
+error: aborting due to 2 previous errors
+