Add map unwrap or lint for Result

CHANGELOG.md

@@ -1176,6 +1176,7 @@ Released 2018-09-13
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
 [`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
 [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
+[`result_map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or
 [`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
 [`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
 [`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 340 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 341 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/lib.rs

@@ -621,6 +621,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         &methods::OPTION_UNWRAP_USED,
         &methods::OR_FUN_CALL,
         &methods::RESULT_EXPECT_USED,
+        &methods::RESULT_MAP_UNWRAP_OR,
         &methods::RESULT_MAP_UNWRAP_OR_ELSE,
         &methods::RESULT_UNWRAP_USED,
         &methods::SEARCH_IS_SOME,
@@ -1036,6 +1037,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&methods::MAP_FLATTEN),
         LintId::of(&methods::OPTION_MAP_UNWRAP_OR),
         LintId::of(&methods::OPTION_MAP_UNWRAP_OR_ELSE),
+        LintId::of(&methods::RESULT_MAP_UNWRAP_OR),
         LintId::of(&methods::RESULT_MAP_UNWRAP_OR_ELSE),
         LintId::of(&misc::USED_UNDERSCORE_BINDING),
         LintId::of(&misc_early::UNSEPARATED_LITERAL_SUFFIX),

clippy_lints/src/methods/option_map_unwrap_or.rs → clippy_lints/src/methods/map_unwrap_or.rs

@@ -6,17 +6,19 @@ use rustc::lint::LateContext;
 use rustc_data_structures::fx::FxHashSet;
 use syntax_pos::symbol::Symbol;
 
-use super::OPTION_MAP_UNWRAP_OR;
+use super::{OPTION_MAP_UNWRAP_OR, RESULT_MAP_UNWRAP_OR};
 
-/// lint use of `map().unwrap_or()` for `Option`s
+/// lint use of `map().unwrap_or()` for `Option`s and `Result`s
 pub(super) fn lint<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
     expr: &hir::Expr,
     map_args: &'tcx [hir::Expr],
     unwrap_args: &'tcx [hir::Expr],
 ) {
-    // lint if the caller of `map()` is an `Option`
-    if match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::OPTION) {
+    let is_option = match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::OPTION);
+    let is_result = match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::RESULT);
+
+    if is_option || is_result {
         if !is_copy(cx, cx.tables.expr_ty(&unwrap_args[1])) {
             // Do not lint if the `map` argument uses identifiers in the `map`
             // argument that are also used in the `unwrap_or` argument
@@ -43,19 +45,33 @@ pub(super) fn lint<'a, 'tcx>(
         let map_snippet = snippet(cx, map_args[1].span, "..");
         let unwrap_snippet = snippet(cx, unwrap_args[1].span, "..");
         // lint message
-        // comparing the snippet from source to raw text ("None") below is safe
-        // because we already have checked the type.
-        let arg = if unwrap_snippet == "None" { "None" } else { "a" };
-        let suggest = if unwrap_snippet == "None" {
-            "and_then(f)"
+        let msg = if is_option {
+            // comparing the snippet from source to raw text ("None") below is safe
+            // because we already have checked the type.
+            let (arg, suggest) = if unwrap_snippet == "None" {
+                ("None", "and_then(f)")
+            } else {
+                ("a", "map_or(a, f)")
+            };
+
+            format!(
+                "called `map(f).unwrap_or({})` on an Option value. \
+                 This can be done more directly by calling `{}` instead",
+                arg, suggest
+            )
         } else {
-            "map_or(a, f)"
+            debug_assert!(is_result);
+            "called `map(f).unwrap_or(a)` on a Result value. \
+             This can be done more directly by calling `map_or(a, f)` instead"
+                .to_string()
         };
-        let msg = &format!(
-            "called `map(f).unwrap_or({})` on an Option value. \
-             This can be done more directly by calling `{}` instead",
-            arg, suggest
-        );
+
+        let lint_type = if is_option {
+            OPTION_MAP_UNWRAP_OR
+        } else {
+            RESULT_MAP_UNWRAP_OR
+        };
+
         // lint, with note if neither arg is > 1 line and both map() and
         // unwrap_or() have the same span
         let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1;
@@ -70,9 +86,9 @@ pub(super) fn lint<'a, 'tcx>(
                 "replace `map({}).unwrap_or({})` with `{}`",
                 map_snippet, unwrap_snippet, suggest
             );
-            span_note_and_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span, &note);
+            span_note_and_lint(cx, lint_type, expr.span, &msg, expr.span, &note);
         } else if same_span && multiline {
-            span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg);
+            span_lint(cx, lint_type, expr.span, &msg);
         };
     }
 }

clippy_lints/src/methods/mod.rs

@@ -1,6 +1,6 @@
 mod inefficient_to_string;
 mod manual_saturating_arithmetic;
-mod option_map_unwrap_or;
+mod map_unwrap_or;
 mod unnecessary_filter_map;
 
 use std::borrow::Cow;
@@ -252,7 +252,7 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for usage of `_.map(_).unwrap_or(_)`.
+    /// **What it does:** Checks for usage of `_.map(_).unwrap_or(_)` for `Option`.
     ///
     /// **Why is this bad?** Readability, this can be written more concisely as
     /// `_.map_or(_, _)`.
@@ -269,6 +269,24 @@ declare_clippy_lint! {
     "using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `_.map(_).unwrap_or(_)` for `Result`.
+    ///
+    /// **Why is this bad?** Readability, this can be written more concisely as
+    /// `_.map_or(_, _)`.
+    ///
+    /// **Known problems:** The order of the arguments is not in execution order
+    ///
+    /// **Example:**
+    /// ```rust
+    /// # let x: Result<usize, ()> = Ok(1);
+    /// x.map(|a| a + 1).unwrap_or(0);
+    /// ```
+    pub RESULT_MAP_UNWRAP_OR,
+    pedantic,
+    "using `Result.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for usage of `_.map(_).unwrap_or_else(_)`.
     ///
@@ -1093,6 +1111,7 @@ declare_lint_pass!(Methods => [
     WRONG_PUB_SELF_CONVENTION,
     OK_EXPECT,
     OPTION_MAP_UNWRAP_OR,
+    RESULT_MAP_UNWRAP_OR,
     OPTION_MAP_UNWRAP_OR_ELSE,
     RESULT_MAP_UNWRAP_OR_ELSE,
     OPTION_MAP_OR_NONE,
@@ -1147,7 +1166,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             ["unwrap", ..] => lint_unwrap(cx, expr, arg_lists[0]),
             ["expect", "ok"] => lint_ok_expect(cx, expr, arg_lists[1]),
             ["expect", ..] => lint_expect(cx, expr, arg_lists[0]),
-            ["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0]),
+            ["unwrap_or", "map"] => map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0]),
             ["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]),
             ["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
             ["and_then", ..] => lint_option_and_then_some(cx, expr, arg_lists[0]),

src/lintlist/mod.rs

@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 340] = [
+pub const ALL_LINTS: [Lint; 341] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1708,6 +1708,13 @@ pub const ALL_LINTS: [Lint; 340] = [
         deprecation: None,
         module: "map_unit_fn",
     },
+    Lint {
+        name: "result_map_unwrap_or",
+        group: "pedantic",
+        desc: "using `Result.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`",
+        deprecation: None,
+        module: "methods",
+    },
     Lint {
         name: "result_map_unwrap_or_else",
         group: "pedantic",

tests/ui/result_map_unwrap_or.rs

@@ -0,0 +1,14 @@
+#![warn(clippy::result_map_unwrap_or)]
+
+fn main() {
+    let res: Result<usize, ()> = Ok(1);
+
+    // Check `RESULT_MAP_OR_NONE`.
+    // Single line case.
+    let _ = res.map(|x| x + 1).unwrap_or(0);
+    // Multi-line case.
+    #[rustfmt::skip]
+    let _ = res.map(|x| {
+                        x + 1
+                       }).unwrap_or(0);
+}

tests/ui/result_map_unwrap_or.stderr

@@ -0,0 +1,20 @@
+error: called `map(f).unwrap_or(a)` on a Result value. This can be done more directly by calling `map_or(a, f)` instead
+  --> $DIR/result_map_unwrap_or.rs:8:13
+   |
+LL |     let _ = res.map(|x| x + 1).unwrap_or(0);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::result-map-unwrap-or` implied by `-D warnings`
+   = note: replace `map(|x| x + 1).unwrap_or(0)` with `map_or(0, |x| x + 1)`
+
+error: called `map(f).unwrap_or(a)` on a Result value. This can be done more directly by calling `map_or(a, f)` instead
+  --> $DIR/result_map_unwrap_or.rs:11:13
+   |
+LL |       let _ = res.map(|x| {
+   |  _____________^
+LL | |                         x + 1
+LL | |                        }).unwrap_or(0);
+   | |______________________________________^
+
+error: aborting due to 2 previous errors
+