Move and_then_some to clippy_lints/src/methods/mod.rs

Author: tesuji

CHANGELOG.md

@@ -1088,6 +1088,7 @@ Released 2018-09-13
 [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
 [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
 [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
+[`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some
 [`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
 [`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
 [`option_map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or

clippy_lints/src/lib.rs

@@ -794,6 +794,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         methods::ITER_SKIP_NEXT,
         methods::NEW_RET_NO_SELF,
         methods::OK_EXPECT,
+        methods::OPTION_AND_THEN_SOME,
         methods::OPTION_MAP_OR_NONE,
         methods::OR_FUN_CALL,
         methods::SEARCH_IS_SOME,
@@ -1032,6 +1033,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         methods::CLONE_ON_COPY,
         methods::FILTER_NEXT,
         methods::FLAT_MAP_IDENTITY,
+        methods::OPTION_AND_THEN_SOME,
         methods::SEARCH_IS_SOME,
         methods::UNNECESSARY_FILTER_MAP,
         methods::USELESS_ASREF,

clippy_lints/src/methods/mod.rs

@@ -264,6 +264,32 @@ declare_clippy_lint! {
     "using `Option.map_or(None, f)`, which is more succinctly expressed as `and_then(f)`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `_.and_then(|x| Some(y))`.
+    ///
+    /// **Why is this bad?** Readability, this can be written more concisely as
+    /// `_.map(|x| y)`.
+    ///
+    /// **Known problems:** None
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let x = Some("foo");
+    /// let _ = x.and_then(|s| Some(s.len()));
+    /// ```
+    ///
+    /// The correct use would be:
+    ///
+    /// ```rust
+    /// let x = Some("foo");
+    /// let _ = x.map(|s| s.len());
+    /// ```
+    pub OPTION_AND_THEN_SOME,
+    complexity,
+    "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for usage of `_.filter(_).next()`.
     ///
@@ -900,6 +926,7 @@ declare_lint_pass!(Methods => [
     OPTION_MAP_UNWRAP_OR_ELSE,
     RESULT_MAP_UNWRAP_OR_ELSE,
     OPTION_MAP_OR_NONE,
+    OPTION_AND_THEN_SOME,
     OR_FUN_CALL,
     EXPECT_FUN_CALL,
     CHARS_NEXT_CMP,
@@ -948,6 +975,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             ["unwrap_or", "map"] => option_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]),
             ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
             ["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
             ["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
@@ -2052,6 +2080,57 @@ fn lint_map_or_none<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr,
     }
 }
 
+/// Lint use of `_.and_then(|x| Some(y))` for `Option`s
+fn lint_option_and_then_some(cx: &LateContext<'_, '_>, expr: &hir::Expr, args: &[hir::Expr]) {
+    let ty = cx.tables.expr_ty(&args[0]);
+    if !match_type(cx, ty, &paths::OPTION) {
+        return;
+    }
+
+    const LINT_MSG: &str = "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`";
+
+    match args[1].node {
+        hir::ExprKind::Closure(_, _, body_id, closure_args_span, _) => {
+            let closure_body = cx.tcx.hir().body(body_id);
+            let closure_expr = remove_blocks(&closure_body.value);
+
+            // let note = format!("{:?}", args[1]);
+            // span_note_and_lint(cx, OPTION_AND_THEN_SOME, closure_args_span, "Outer debugging
+            // information", closure_args_span, &note);
+
+            if let hir::ExprKind::Call(ref some_expr, ref some_args) = closure_expr.node {
+                if let hir::ExprKind::Path(ref qpath) = some_expr.node {
+                    if match_qpath(qpath, &paths::OPTION_SOME) {
+                        if some_args.len() == 1 {
+                            let inner_expr = &some_args[0];
+                            let some_inner_snip = snippet(cx, inner_expr.span, "..");
+                            let closure_args_snip = snippet(cx, closure_args_span, "..");
+                            let option_snip = snippet(cx, args[0].span, "..");
+                            let note = format!("{}.map({} {})", option_snip, closure_args_snip, some_inner_snip);
+                            span_lint_and_sugg(
+                                cx,
+                                OPTION_AND_THEN_SOME,
+                                expr.span,
+                                LINT_MSG,
+                                "try this",
+                                note,
+                                Applicability::MachineApplicable,
+                            );
+                        }
+                    }
+                }
+            }
+        },
+        // hir::ExprKind::Path(ref qpath) => {
+        //     if match_qpath(qpath, &paths::OPTION_SOME) {
+        //         let note = format!("{:?}", args[1]);
+        //         span_note_and_lint(cx, OPTION_AND_THEN_SOME, args[1].span, "Some debugging information",
+        // args[1].span, &note);     }
+        // },
+        _ => {},
+    }
+}
+
 /// lint use of `filter().next()` for `Iterators`
 fn lint_filter_next<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, filter_args: &'tcx [hir::Expr]) {
     // lint if caller of `.filter().next()` is an Iterator

src/lintlist/mod.rs

@@ -1316,6 +1316,13 @@ pub const ALL_LINTS: [Lint; 310] = [
         deprecation: None,
         module: "eq_op",
     },
+    Lint {
+        name: "option_and_then_some",
+        group: "complexity",
+        desc: "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`",
+        deprecation: None,
+        module: "methods",
+    },
     Lint {
         name: "option_map_or_none",
         group: "style",

tests/ui/option_and_then_some.fixed

@@ -12,5 +12,5 @@ pub fn main() {
 
     // Different type
     let x: Result<u32, &str> = Ok(1);
-    x.and_then(Ok);
+    let _ = x.and_then(Ok);
 }

tests/ui/option_and_then_some.rs

@@ -12,5 +12,5 @@ pub fn main() {
 
     // Different type
     let x: Result<u32, &str> = Ok(1);
-    x.and_then(Ok);
+    let _ = x.and_then(Ok);
 }