Add lints suggesting map_or() and map_or_else()

In accordance with the latter lint, replace map().unwrap_or_else() in
src/mut_mut.rs with map_or_else()

README.md

@@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
 [Jump to usage instructions](#usage)
 
 ##Lints
-There are 77 lints included in this crate:
+There are 79 lints included in this crate:
 
 name                                                                                                     | default | meaning
 ---------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -53,6 +53,8 @@ name
 [non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal)                   | allow   | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead
 [nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options)     | warn    | nonsensical combination of options for opening a file
 [ok_expect](https://github.com/Manishearth/rust-clippy/wiki#ok_expect)                                   | warn    | using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result
+[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or)             | warn    | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`)
+[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else)   | warn    | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)`)
 [option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used)                 | allow   | using `Option.unwrap()`, which should at least get a better message using `expect()`
 [precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence)                                 | warn    | catches operations where precedence may be unclear. See the wiki for a list of cases caught
 [ptr_arg](https://github.com/Manishearth/rust-clippy/wiki#ptr_arg)                                       | warn    | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively

src/lib.rs

@@ -156,6 +156,8 @@ pub fn plugin_registrar(reg: &mut Registry) {
         matches::MATCH_REF_PATS,
         matches::SINGLE_MATCH,
         methods::OK_EXPECT,
+        methods::OPTION_MAP_UNWRAP_OR,
+        methods::OPTION_MAP_UNWRAP_OR_ELSE,
         methods::SHOULD_IMPLEMENT_TRAIT,
         methods::STR_TO_STRING,
         methods::STRING_TO_STRING,

src/methods.rs

@@ -5,7 +5,7 @@ use rustc::middle::subst::{Subst, TypeSpace};
 use std::iter;
 use std::borrow::Cow;
 
-use utils::{snippet, span_lint, match_path, match_type, walk_ptrs_ty_depth,
+use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, walk_ptrs_ty_depth,
     walk_ptrs_ty};
 use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH};
 
@@ -34,12 +34,18 @@ declare_lint!(pub WRONG_PUB_SELF_CONVENTION, Allow,
 declare_lint!(pub OK_EXPECT, Warn,
               "using `ok().expect()`, which gives worse error messages than \
                calling `expect` directly on the Result");
-
+declare_lint!(pub OPTION_MAP_UNWRAP_OR, Warn,
+              "using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as \
+               `map_or(a, f)`)");
+declare_lint!(pub OPTION_MAP_UNWRAP_OR_ELSE, Warn,
+              "using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as \
+               `map_or_else(g, f)`)");
 
 impl LintPass for MethodsPass {
     fn get_lints(&self) -> LintArray {
         lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING,
-                    SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, OK_EXPECT)
+                    SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, OK_EXPECT, OPTION_MAP_UNWRAP_OR,
+                    OPTION_MAP_UNWRAP_OR_ELSE)
     }
 }
 
@@ -92,6 +98,66 @@ impl LateLintPass for MethodsPass {
                     }
                 }
             }
+            // check Option.map(_).unwrap_or(_)
+            else if name.node.as_str() == "unwrap_or" {
+                if let ExprMethodCall(ref inner_name, _, ref inner_args) = args[0].node {
+                    if inner_name.node.as_str() == "map"
+                            && match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &OPTION_PATH) {
+                        // lint message
+                        let msg =
+                            "called `map(f).unwrap_or(a)` on an Option value. This can be done \
+                             more directly by calling `map_or(a, f)` instead";
+                        // get args to map() and unwrap_or()
+                        let map_arg = snippet(cx, inner_args[1].span, "..");
+                        let unwrap_arg = snippet(cx, args[1].span, "..");
+                        // lint, with note if neither arg is > 1 line and both map() and
+                        // unwrap_or() have the same span
+                        let multiline = map_arg.lines().count() > 1
+                                        || unwrap_arg.lines().count() > 1;
+                        let same_span = inner_args[1].span.expn_id == args[1].span.expn_id;
+                        if same_span && !multiline {
+                            span_note_and_lint(
+                                cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span,
+                                &format!("replace this with map_or({1}, {0})",
+                                         map_arg, unwrap_arg)
+                            );
+                        }
+                        else if same_span && multiline {
+                            span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg);
+                        };
+                    }
+                }
+            }
+            // check Option.map(_).unwrap_or_else(_)
+            else if name.node.as_str() == "unwrap_or_else" {
+                if let ExprMethodCall(ref inner_name, _, ref inner_args) = args[0].node {
+                    if inner_name.node.as_str() == "map"
+                            && match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &OPTION_PATH) {
+                        // lint message
+                        let msg =
+                            "called `map(f).unwrap_or_else(g)` on an Option value. This can be \
+                             done more directly by calling `map_or_else(g, f)` instead";
+                        // get args to map() and unwrap_or_else()
+                        let map_arg = snippet(cx, inner_args[1].span, "..");
+                        let unwrap_arg = snippet(cx, args[1].span, "..");
+                        // lint, with note if neither arg is > 1 line and both map() and
+                        // unwrap_or_else() have the same span
+                        let multiline = map_arg.lines().count() > 1
+                                        || unwrap_arg.lines().count() > 1;
+                        let same_span = inner_args[1].span.expn_id == args[1].span.expn_id;
+                        if same_span && !multiline {
+                            span_note_and_lint(
+                                cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg, expr.span,
+                                &format!("replace this with map_or_else({1}, {0})",
+                                         map_arg, unwrap_arg)
+                            );
+                        }
+                        else if same_span && multiline {
+                            span_lint(cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg);
+                        };
+                    }
+                }
+            }
         }
     }
 

src/mut_mut.rs

@@ -39,17 +39,20 @@ fn check_expr_mut(cx: &LateContext, expr: &Expr) {
     }
 
     unwrap_addr(expr).map_or((), |e| {
-        unwrap_addr(e).map(|_| {
+        unwrap_addr(e).map_or_else(
-            span_lint(cx, MUT_MUT, expr.span,
+            || {
-                      "generally you want to avoid `&mut &mut _` if possible")
+                if let TyRef(_, TypeAndMut{ty: _, mutbl: MutMutable}) =
-        }).unwrap_or_else(|| {
+                    cx.tcx.expr_ty(e).sty {
-            if let TyRef(_, TypeAndMut{ty: _, mutbl: MutMutable}) =
+                        span_lint(cx, MUT_MUT, expr.span,
-                cx.tcx.expr_ty(e).sty {
+                                  "this expression mutably borrows a mutable reference. \
-                    span_lint(cx, MUT_MUT, expr.span,
+                                   Consider reborrowing")
-                              "this expression mutably borrows a mutable reference. \
-                               Consider reborrowing")
                 }
-        })
+            },
+            |_| {
+                span_lint(cx, MUT_MUT, expr.span,
+                          "generally you want to avoid `&mut &mut _` if possible")
+            }
+        )
     })
 }
 

tests/compile-fail/methods.rs

@@ -34,6 +34,55 @@ impl Mul<T> for T {
     fn mul(self, other: T) -> T { self } // no error, obviously
 }
 
+/// Utility macro to test linting behavior in `option_methods()`
+/// The lints included in `option_methods()` should not lint if the call to map is partially
+/// within a macro
+macro_rules! opt_map {
+    ($opt:expr, $map:expr) => {($opt).map($map)};
+}
+
+/// Checks implementation of the following lints:
+/// OPTION_MAP_UNWRAP_OR
+/// OPTION_MAP_UNWRAP_OR_ELSE
+fn option_methods() {
+    let opt = Some(1);
+
+    // Check OPTION_MAP_UNWRAP_OR
+    // single line case
+    let _ = opt.map(|x| x + 1) //~  ERROR called `map(f).unwrap_or(a)`
+                               //~| NOTE replace this
+               .unwrap_or(0); // should lint even though this call is on a separate line
+    // multi line cases
+    let _ = opt.map(|x| { //~ ERROR called `map(f).unwrap_or(a)`
+                        x + 1
+                    }
+              ).unwrap_or(0);
+    let _ = opt.map(|x| x + 1) //~ ERROR called `map(f).unwrap_or(a)`
+               .unwrap_or({
+                    0
+                });
+    // macro case
+    let _ = opt_map!(opt, |x| x + 1).unwrap_or(0); // should not lint
+
+    // Check OPTION_MAP_UNWRAP_OR_ELSE
+    // single line case
+    let _ = opt.map(|x| x + 1) //~  ERROR called `map(f).unwrap_or_else(g)`
+                               //~| NOTE replace this
+               .unwrap_or_else(|| 0); // should lint even though this call is on a separate line
+    // multi line cases
+    let _ = opt.map(|x| { //~ ERROR called `map(f).unwrap_or_else(g)`
+                        x + 1
+                    }
+              ).unwrap_or_else(|| 0);
+    let _ = opt.map(|x| x + 1) //~ ERROR called `map(f).unwrap_or_else(g)`
+               .unwrap_or_else(||
+                    0
+                );
+    // macro case
+    let _ = opt_map!(opt, |x| x + 1).unwrap_or_else(|| 0); // should not lint
+
+}
+
 fn main() {
     use std::io;