lint: add map(f).unwrap_or(a) for Result

Author: Guanqun Lu

clippy_lints/src/lib.rs

@@ -611,6 +611,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,
@@ -1014,6 +1015,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 renamed to 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,27 @@ 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 = if unwrap_snippet == "None" { "None" } else { "a" };
+            let suggest = if unwrap_snippet == "None" {
+                "and_then(f)"
+            } else {
+                "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)"
+            "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
-        );
+
         // 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 +80,29 @@ 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,
+                if is_option {
+                    OPTION_MAP_UNWRAP_OR
+                } else {
+                    RESULT_MAP_UNWRAP_OR
+                },
+                expr.span,
+                &msg,
+                expr.span,
+                &note,
+            );
         } else if same_span && multiline {
-            span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg);
+            span_lint(
+                cx,
+                if is_option {
+                    OPTION_MAP_UNWRAP_OR
+                } else {
+                    RESULT_MAP_UNWRAP_OR
+                },
+                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;
@@ -251,7 +251,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(_, _)`.
@@ -268,6 +268,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(_)`.
     ///
@@ -1075,6 +1093,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,
@@ -1128,7 +1147,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; 333] = [
+pub const ALL_LINTS: [Lint; 334] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1687,6 +1687,13 @@ pub const ALL_LINTS: [Lint; 333] = [
         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",