Add option_and_then_some lint

Author: bors

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

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 310 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 311 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/and_then_some.rs

@@ -0,0 +1,95 @@
+use crate::utils::paths;
+use crate::utils::{in_macro_or_desugar, match_qpath, match_type, remove_blocks, snippet, span_lint_and_sugg};
+use rustc::hir;
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::{declare_lint_pass, declare_tool_lint};
+use rustc_errors::Applicability;
+
+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");
+    /// x.and_then(|s| Some(s.len()));
+    /// ```
+    ///
+    /// The correct use would be:
+    ///
+    /// ```rust
+    /// let x = Some("foo");
+    /// 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_lint_pass!(AndThenSomeLint => [OPTION_AND_THEN_SOME]);
+
+const LINT_MSG: &str = "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`";
+
+impl LateLintPass<'_, '_> for AndThenSomeLint {
+    /// Lint use of `_.and_then(|x| Some(y))` for `Option`s
+    fn check_expr(&mut self, cx: &LateContext<'_, '_>, e: &hir::Expr) {
+        if in_macro_or_desugar(e.span) {
+            return;
+        }
+
+        if let hir::ExprKind::MethodCall(ref method, _, ref args) = e.node {
+            if args.len() == 2 && method.ident.as_str() == "and_then" {
+                let ty = cx.tables.expr_ty(&args[0]);
+                if !match_type(cx, ty, &paths::OPTION) {
+                    return;
+                }
+                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,
+                                            e.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);     }
+                    // },
+                    _ => {},
+                }
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -145,6 +145,7 @@ mod consts;
 mod utils;
 
 // begin lints modules, do not remove this comment, it’s used in `update_lints`
+pub mod and_then_some;
 pub mod approx_const;
 pub mod arithmetic;
 pub mod assertions_on_constants;
@@ -596,6 +597,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box integer_division::IntegerDivision);
     reg.register_late_lint_pass(box inherent_to_string::InherentToString);
     reg.register_late_lint_pass(box trait_bounds::TraitBounds);
+    reg.register_late_lint_pass(box and_then_some::AndThenSomeLint);
 
     reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -681,6 +683,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     ]);
 
     reg.register_lint_group("clippy::all", Some("clippy"), vec![
+        and_then_some::OPTION_AND_THEN_SOME,
         approx_const::APPROX_CONSTANT,
         assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
         assign_ops::ASSIGN_OP_PATTERN,
@@ -1001,6 +1004,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     ]);
 
     reg.register_lint_group("clippy::complexity", Some("clippy_complexity"), vec![
+        and_then_some::OPTION_AND_THEN_SOME,
         assign_ops::MISREFACTORED_ASSIGN_OP,
         attrs::DEPRECATED_CFG_ATTR,
         booleans::NONMINIMAL_BOOL,

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; 310] = [
+pub const ALL_LINTS: [Lint; 311] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -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: "and_then_some",
+    },
     Lint {
         name: "option_map_or_none",
         group: "style",

tests/ui/option_and_then_some.fixed

@@ -0,0 +1,16 @@
+// run-rustfix
+#![deny(clippy::option_and_then_some)]
+
+// need a main anyway, use it get rid of unused warnings too
+pub fn main() {
+    let x = Some(5);
+    // the easiest cases
+    x.and_then(Some);
+    x.map(|o| o + 1);
+    // and an easy counter-example
+    x.and_then(|o| if o < 32 { Some(o) } else { None });
+
+    // Different type
+    let x: Result<u32, &str> = Ok(1);
+    x.and_then(Ok);
+}

tests/ui/option_and_then_some.rs

@@ -0,0 +1,16 @@
+// run-rustfix
+#![deny(clippy::option_and_then_some)]
+
+// need a main anyway, use it get rid of unused warnings too
+pub fn main() {
+    let x = Some(5);
+    // the easiest cases
+    x.and_then(Some);
+    x.and_then(|o| Some(o + 1));
+    // and an easy counter-example
+    x.and_then(|o| if o < 32 { Some(o) } else { None });
+
+    // Different type
+    let x: Result<u32, &str> = Ok(1);
+    x.and_then(Ok);
+}

tests/ui/option_and_then_some.stderr

@@ -0,0 +1,14 @@
+error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`
+  --> $DIR/option_and_then_some.rs:9:5
+   |
+LL |     x.and_then(|o| Some(o + 1));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.map(|o| o + 1)`
+   |
+note: lint level defined here
+  --> $DIR/option_and_then_some.rs:2:9
+   |
+LL | #![deny(clippy::option_and_then_some)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+