about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAleksei Latyshev <alex_700_95@mail.ru>2020-04-25 23:33:11 +0300
committerAleksei Latyshev <alex_700_95@mail.ru>2020-05-17 12:17:03 +0300
commit07f1edf2d43efa0ef53e5b6c56c895bc9738ab94 (patch)
tree5aca01abf7bd0e347521acb71630d65ee65ca09e
parentcb7f9679a63075b3fce2fdc69f7b02fe0f482464 (diff)
downloadrust-07f1edf2d43efa0ef53e5b6c56c895bc9738ab94.tar.gz
rust-07f1edf2d43efa0ef53e5b6c56c895bc9738ab94.zip
improve and generalize `option_and_then_some` lint
- rename it to bind_instead_of_map
-rw-r--r--CHANGELOG.md4
-rw-r--r--clippy_lints/src/lib.rs7
-rw-r--r--clippy_lints/src/loops.rs2
-rw-r--r--clippy_lints/src/methods/bind_instead_of_map.rs309
-rw-r--r--clippy_lints/src/methods/mod.rs100
-rw-r--r--clippy_lints/src/types.rs2
-rw-r--r--src/lintlist/mod.rs14
-rw-r--r--tests/ui/bind_instead_of_map.fixed (renamed from tests/ui/option_and_then_some.fixed)4
-rw-r--r--tests/ui/bind_instead_of_map.rs (renamed from tests/ui/option_and_then_some.rs)2
-rw-r--r--tests/ui/bind_instead_of_map.stderr (renamed from tests/ui/option_and_then_some.stderr)18
-rw-r--r--tests/ui/bind_instead_of_map_multipart.rs61
-rw-r--r--tests/ui/bind_instead_of_map_multipart.stderr73
-rw-r--r--tests/ui/blocks_in_if_conditions.fixed4
-rw-r--r--tests/ui/blocks_in_if_conditions.rs4
-rw-r--r--tests/ui/option_map_or_none.fixed2
-rw-r--r--tests/ui/option_map_or_none.rs2
16 files changed, 503 insertions, 105 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d05819a973a..7abefe65424 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -315,7 +315,7 @@ Released 2019-11-07
   * [`missing_safety_doc`] [#4535](https://github.com/rust-lang/rust-clippy/pull/4535)
   * [`mem_replace_with_uninit`] [#4511](https://github.com/rust-lang/rust-clippy/pull/4511)
   * [`suspicious_map`] [#4394](https://github.com/rust-lang/rust-clippy/pull/4394)
-  * [`option_and_then_some`] [#4386](https://github.com/rust-lang/rust-clippy/pull/4386)
+  * `option_and_then_some` [#4386](https://github.com/rust-lang/rust-clippy/pull/4386)
   * [`manual_saturating_arithmetic`] [#4498](https://github.com/rust-lang/rust-clippy/pull/4498)
 * Deprecate `unused_collect` lint. This is fully covered by rustc's `#[must_use]` on `collect` [#4348](https://github.com/rust-lang/rust-clippy/pull/4348)
 * Move `type_repetition_in_bounds` to pedantic group [#4403](https://github.com/rust-lang/rust-clippy/pull/4403)
@@ -1273,6 +1273,7 @@ Released 2018-09-13
 [`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops
 [`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
 [`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
+[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
 [`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
 [`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
 [`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
@@ -1494,7 +1495,6 @@ 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_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
 [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap
 [`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index bda0d5c0458..ec198b684b6 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -650,6 +650,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &mem_replace::MEM_REPLACE_OPTION_WITH_NONE,
         &mem_replace::MEM_REPLACE_WITH_DEFAULT,
         &mem_replace::MEM_REPLACE_WITH_UNINIT,
+        &methods::BIND_INSTEAD_OF_MAP,
         &methods::CHARS_LAST_CMP,
         &methods::CHARS_NEXT_CMP,
         &methods::CLONE_DOUBLE_REF,
@@ -676,7 +677,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::MAP_UNWRAP_OR,
         &methods::NEW_RET_NO_SELF,
         &methods::OK_EXPECT,
-        &methods::OPTION_AND_THEN_SOME,
         &methods::OPTION_AS_REF_DEREF,
         &methods::OPTION_MAP_OR_NONE,
         &methods::OR_FUN_CALL,
@@ -1291,6 +1291,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&mem_replace::MEM_REPLACE_OPTION_WITH_NONE),
         LintId::of(&mem_replace::MEM_REPLACE_WITH_DEFAULT),
         LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT),
+        LintId::of(&methods::BIND_INSTEAD_OF_MAP),
         LintId::of(&methods::CHARS_LAST_CMP),
         LintId::of(&methods::CHARS_NEXT_CMP),
         LintId::of(&methods::CLONE_DOUBLE_REF),
@@ -1307,7 +1308,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
         LintId::of(&methods::NEW_RET_NO_SELF),
         LintId::of(&methods::OK_EXPECT),
-        LintId::of(&methods::OPTION_AND_THEN_SOME),
         LintId::of(&methods::OPTION_AS_REF_DEREF),
         LintId::of(&methods::OPTION_MAP_OR_NONE),
         LintId::of(&methods::OR_FUN_CALL),
@@ -1559,10 +1559,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&matches::MATCH_AS_REF),
         LintId::of(&matches::MATCH_SINGLE_BINDING),
         LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
+        LintId::of(&methods::BIND_INSTEAD_OF_MAP),
         LintId::of(&methods::CLONE_ON_COPY),
         LintId::of(&methods::FILTER_NEXT),
         LintId::of(&methods::FLAT_MAP_IDENTITY),
-        LintId::of(&methods::OPTION_AND_THEN_SOME),
         LintId::of(&methods::OPTION_AS_REF_DEREF),
         LintId::of(&methods::SEARCH_IS_SOME),
         LintId::of(&methods::SKIP_WHILE_NEXT),
@@ -1784,6 +1784,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) {
     ls.register_renamed("clippy::new_without_default_derive", "clippy::new_without_default");
     ls.register_renamed("clippy::cyclomatic_complexity", "clippy::cognitive_complexity");
     ls.register_renamed("clippy::const_static_lifetime", "clippy::redundant_static_lifetimes");
+    ls.register_renamed("clippy::option_and_then_some", "clippy::bind_instead_of_map");
     ls.register_renamed("clippy::block_in_if_condition_expr", "clippy::blocks_in_if_conditions");
     ls.register_renamed("clippy::block_in_if_condition_stmt", "clippy::blocks_in_if_conditions");
     ls.register_renamed("clippy::option_map_unwrap_or", "clippy::map_unwrap_or");
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 4a9c411d7c8..84e8a010738 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1462,7 +1462,7 @@ fn check_for_loop_over_map_kv<'a, 'tcx>(
                         let map = sugg::Sugg::hir(cx, arg, "map");
                         multispan_sugg(
                             diag,
-                            "use the corresponding method".into(),
+                            "use the corresponding method",
                             vec![
                                 (pat_span, snippet(cx, new_pat_span, kind).into_owned()),
                                 (arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl)),
diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs
new file mode 100644
index 00000000000..32e86637569
--- /dev/null
+++ b/clippy_lints/src/methods/bind_instead_of_map.rs
@@ -0,0 +1,309 @@
+use super::{contains_return, BIND_INSTEAD_OF_MAP};
+use crate::utils::{
+    in_macro, match_qpath, match_type, method_calls, multispan_sugg_with_applicability, paths, remove_blocks, snippet,
+    snippet_with_macro_callsite, span_lint_and_sugg, span_lint_and_then,
+};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_hir::intravisit::{self, Visitor};
+use rustc_lint::LateContext;
+use rustc_middle::hir::map::Map;
+use rustc_span::Span;
+
+pub(crate) struct OptionAndThenSome;
+impl BindInsteadOfMap for OptionAndThenSome {
+    const TYPE_NAME: &'static str = "Option";
+    const TYPE_QPATH: &'static [&'static str] = &paths::OPTION;
+
+    const BAD_METHOD_NAME: &'static str = "and_then";
+    const BAD_VARIANT_NAME: &'static str = "Some";
+    const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::OPTION_SOME;
+
+    const GOOD_METHOD_NAME: &'static str = "map";
+}
+
+pub(crate) struct ResultAndThenOk;
+impl BindInsteadOfMap for ResultAndThenOk {
+    const TYPE_NAME: &'static str = "Result";
+    const TYPE_QPATH: &'static [&'static str] = &paths::RESULT;
+
+    const BAD_METHOD_NAME: &'static str = "and_then";
+    const BAD_VARIANT_NAME: &'static str = "Ok";
+    const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::RESULT_OK;
+
+    const GOOD_METHOD_NAME: &'static str = "map";
+}
+
+pub(crate) struct ResultOrElseErrInfo;
+impl BindInsteadOfMap for ResultOrElseErrInfo {
+    const TYPE_NAME: &'static str = "Result";
+    const TYPE_QPATH: &'static [&'static str] = &paths::RESULT;
+
+    const BAD_METHOD_NAME: &'static str = "or_else";
+    const BAD_VARIANT_NAME: &'static str = "Err";
+    const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::RESULT_ERR;
+
+    const GOOD_METHOD_NAME: &'static str = "map_err";
+}
+
+pub(crate) trait BindInsteadOfMap {
+    const TYPE_NAME: &'static str;
+    const TYPE_QPATH: &'static [&'static str];
+
+    const BAD_METHOD_NAME: &'static str;
+    const BAD_VARIANT_NAME: &'static str;
+    const BAD_VARIANT_QPATH: &'static [&'static str];
+
+    const GOOD_METHOD_NAME: &'static str;
+
+    fn no_op_msg() -> String {
+        format!(
+            "using `{}.{}({})`, which is a no-op",
+            Self::TYPE_NAME,
+            Self::BAD_METHOD_NAME,
+            Self::BAD_VARIANT_NAME
+        )
+    }
+
+    fn lint_msg() -> String {
+        format!(
+            "using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`",
+            Self::TYPE_NAME,
+            Self::BAD_METHOD_NAME,
+            Self::BAD_VARIANT_NAME,
+            Self::GOOD_METHOD_NAME
+        )
+    }
+
+    fn lint_closure_autofixable(
+        cx: &LateContext<'_, '_>,
+        expr: &hir::Expr<'_>,
+        args: &[hir::Expr<'_>],
+        closure_expr: &hir::Expr<'_>,
+        closure_args_span: Span,
+    ) -> bool {
+        if_chain! {
+            if let hir::ExprKind::Call(ref some_expr, ref some_args) = closure_expr.kind;
+            if let hir::ExprKind::Path(ref qpath) = some_expr.kind;
+            if match_qpath(qpath, Self::BAD_VARIANT_QPATH);
+            if some_args.len() == 1;
+            then {
+                let inner_expr = &some_args[0];
+
+                if contains_return(inner_expr) {
+                    return false;
+                }
+
+                let some_inner_snip = if inner_expr.span.from_expansion() {
+                    snippet_with_macro_callsite(cx, inner_expr.span, "_")
+                } else {
+                    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!("{}.{}({} {})", option_snip, Self::GOOD_METHOD_NAME, closure_args_snip, some_inner_snip);
+                span_lint_and_sugg(
+                    cx,
+                    BIND_INSTEAD_OF_MAP,
+                    expr.span,
+                    Self::lint_msg().as_ref(),
+                    "try this",
+                    note,
+                    Applicability::MachineApplicable,
+                );
+                true
+            } else {
+                false
+            }
+        }
+    }
+
+    fn lint_closure(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) {
+        let mut suggs = Vec::new();
+        let can_sugg = find_all_ret_expressions(cx, closure_expr, |ret_expr| {
+            if_chain! {
+                if !in_macro(ret_expr.span);
+                if let hir::ExprKind::Call(ref func_path, ref args) = ret_expr.kind;
+                if let hir::ExprKind::Path(ref qpath) = func_path.kind;
+                if match_qpath(qpath, Self::BAD_VARIANT_QPATH);
+                if args.len() == 1;
+                if !contains_return(&args[0]);
+                then {
+                    suggs.push((ret_expr.span, args[0].span.source_callsite()));
+                    true
+                } else {
+                    false
+                }
+            }
+        });
+
+        if can_sugg {
+            span_lint_and_then(cx, BIND_INSTEAD_OF_MAP, expr.span, Self::lint_msg().as_ref(), |diag| {
+                multispan_sugg_with_applicability(
+                    diag,
+                    "try this",
+                    Applicability::MachineApplicable,
+                    std::iter::once((*method_calls(expr, 1).2.get(0).unwrap(), Self::GOOD_METHOD_NAME.into())).chain(
+                        suggs
+                            .into_iter()
+                            .map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())),
+                    ),
+                )
+            });
+        }
+    }
+
+    /// Lint use of `_.and_then(|x| Some(y))` for `Option`s
+    fn lint(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
+        if !match_type(cx, cx.tables.expr_ty(&args[0]), Self::TYPE_QPATH) {
+            return;
+        }
+
+        match args[1].kind {
+            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);
+
+                if !Self::lint_closure_autofixable(cx, expr, args, closure_expr, closure_args_span) {
+                    Self::lint_closure(cx, expr, closure_expr);
+                }
+            },
+            // `_.and_then(Some)` case, which is no-op.
+            hir::ExprKind::Path(ref qpath) if match_qpath(qpath, Self::BAD_VARIANT_QPATH) => {
+                span_lint_and_sugg(
+                    cx,
+                    BIND_INSTEAD_OF_MAP,
+                    expr.span,
+                    Self::no_op_msg().as_ref(),
+                    "use the expression directly",
+                    snippet(cx, args[0].span, "..").into(),
+                    Applicability::MachineApplicable,
+                );
+            },
+            _ => {},
+        }
+    }
+}
+
+/// returns `true` if expr contains match expr desugared from try
+fn contains_try(expr: &hir::Expr<'_>) -> bool {
+    struct TryFinder {
+        found: bool,
+    }
+
+    impl<'hir> intravisit::Visitor<'hir> for TryFinder {
+        type Map = Map<'hir>;
+
+        fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
+            intravisit::NestedVisitorMap::None
+        }
+
+        fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) {
+            if self.found {
+                return;
+            }
+            match expr.kind {
+                hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar) => self.found = true,
+                _ => intravisit::walk_expr(self, expr),
+            }
+        }
+    }
+
+    let mut visitor = TryFinder { found: false };
+    visitor.visit_expr(expr);
+    visitor.found
+}
+
+fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_, '_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool
+where
+    F: FnMut(&'hir hir::Expr<'hir>) -> bool,
+{
+    struct RetFinder<F> {
+        in_stmt: bool,
+        failed: bool,
+        cb: F,
+    }
+
+    struct WithStmtGuarg<'a, F> {
+        val: &'a mut RetFinder<F>,
+        prev_in_stmt: bool,
+    }
+
+    impl<F> RetFinder<F> {
+        fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> {
+            let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt);
+            WithStmtGuarg {
+                val: self,
+                prev_in_stmt,
+            }
+        }
+    }
+
+    impl<F> std::ops::Deref for WithStmtGuarg<'_, F> {
+        type Target = RetFinder<F>;
+
+        fn deref(&self) -> &Self::Target {
+            self.val
+        }
+    }
+
+    impl<F> std::ops::DerefMut for WithStmtGuarg<'_, F> {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            self.val
+        }
+    }
+
+    impl<F> Drop for WithStmtGuarg<'_, F> {
+        fn drop(&mut self) {
+            self.val.in_stmt = self.prev_in_stmt;
+        }
+    }
+
+    impl<'hir, F: FnMut(&'hir hir::Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder<F> {
+        type Map = Map<'hir>;
+
+        fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
+            intravisit::NestedVisitorMap::None
+        }
+
+        fn visit_stmt(&mut self, stmt: &'hir hir::Stmt<'_>) {
+            intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt)
+        }
+
+        fn visit_expr(&mut self, expr: &'hir hir::Expr<'_>) {
+            if self.failed {
+                return;
+            }
+            if self.in_stmt {
+                match expr.kind {
+                    hir::ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr),
+                    _ => intravisit::walk_expr(self, expr),
+                }
+            } else {
+                match expr.kind {
+                    hir::ExprKind::Match(cond, arms, _) => {
+                        self.inside_stmt(true).visit_expr(cond);
+                        for arm in arms {
+                            self.visit_expr(arm.body);
+                        }
+                    },
+                    hir::ExprKind::Block(..) => intravisit::walk_expr(self, expr),
+                    hir::ExprKind::Ret(Some(expr)) => self.visit_expr(expr),
+                    _ => self.failed |= !(self.cb)(expr),
+                }
+            }
+        }
+    }
+
+    !contains_try(expr) && {
+        let mut ret_finder = RetFinder {
+            in_stmt: false,
+            failed: false,
+            cb: callback,
+        };
+        ret_finder.visit_expr(expr);
+        !ret_finder.failed
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index e6094edc5d7..626427c15ec 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -1,3 +1,4 @@
+mod bind_instead_of_map;
 mod inefficient_to_string;
 mod manual_saturating_arithmetic;
 mod option_map_unwrap_or;
@@ -7,6 +8,7 @@ use std::borrow::Cow;
 use std::fmt;
 use std::iter;
 
+use bind_instead_of_map::BindInsteadOfMap;
 use if_chain::if_chain;
 use rustc_ast::ast;
 use rustc_errors::Applicability;
@@ -306,27 +308,34 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for usage of `_.and_then(|x| Some(y))`.
+    /// **What it does:** Checks for usage of `_.and_then(|x| Some(y))`, `_.and_then(|x| Ok(y))` or
+    /// `_.or_else(|x| Err(y))`.
     ///
     /// **Why is this bad?** Readability, this can be written more concisely as
-    /// `_.map(|x| y)`.
+    /// `_.map(|x| y)` or `_.map_err(|x| y)`.
     ///
     /// **Known problems:** None
     ///
     /// **Example:**
     ///
     /// ```rust
-    /// let x = Some("foo");
-    /// let _ = x.and_then(|s| Some(s.len()));
+    /// # fn opt() -> Option<&'static str> { Some("42") }
+    /// # fn res() -> Result<&'static str, &'static str> { Ok("42") }
+    /// let _ = opt().and_then(|s| Some(s.len()));
+    /// let _ = res().and_then(|s| if s.len() == 42 { Ok(10) } else { Ok(20) });
+    /// let _ = res().or_else(|s| if s.len() == 42 { Err(10) } else { Err(20) });
     /// ```
     ///
     /// The correct use would be:
     ///
     /// ```rust
-    /// let x = Some("foo");
-    /// let _ = x.map(|s| s.len());
+    /// # fn opt() -> Option<&'static str> { Some("42") }
+    /// # fn res() -> Result<&'static str, &'static str> { Ok("42") }
+    /// let _ = opt().map(|s| s.len());
+    /// let _ = res().map(|s| if s.len() == 42 { 10 } else { 20 });
+    /// let _ = res().map_err(|s| if s.len() == 42 { 10 } else { 20 });
     /// ```
-    pub OPTION_AND_THEN_SOME,
+    pub BIND_INSTEAD_OF_MAP,
     complexity,
     "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`"
 }
@@ -1243,7 +1252,7 @@ declare_lint_pass!(Methods => [
     MAP_UNWRAP_OR,
     RESULT_MAP_OR_INTO_OPTION,
     OPTION_MAP_OR_NONE,
-    OPTION_AND_THEN_SOME,
+    BIND_INSTEAD_OF_MAP,
     OR_FUN_CALL,
     EXPECT_FUN_CALL,
     CHARS_NEXT_CMP,
@@ -1302,7 +1311,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             ["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0], method_spans[1]),
             ["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]),
+            ["and_then", ..] => {
+                bind_instead_of_map::OptionAndThenSome::lint(cx, expr, arg_lists[0]);
+                bind_instead_of_map::ResultAndThenOk::lint(cx, expr, arg_lists[0]);
+            },
+            ["or_else", ..] => {
+                bind_instead_of_map::ResultOrElseErrInfo::lint(cx, expr, arg_lists[0]);
+            },
             ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
             ["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
             ["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
@@ -2601,73 +2616,6 @@ fn lint_map_or_none<'a, 'tcx>(
     );
 }
 
-/// 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<'_>]) {
-    const LINT_MSG: &str = "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`";
-    const NO_OP_MSG: &str = "using `Option.and_then(Some)`, which is a no-op";
-
-    let ty = cx.tables.expr_ty(&args[0]);
-    if !is_type_diagnostic_item(cx, ty, sym!(option_type)) {
-        return;
-    }
-
-    match args[1].kind {
-        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);
-            if_chain! {
-                if let hir::ExprKind::Call(ref some_expr, ref some_args) = closure_expr.kind;
-                if let hir::ExprKind::Path(ref qpath) = some_expr.kind;
-                if match_qpath(qpath, &paths::OPTION_SOME);
-                if some_args.len() == 1;
-                then {
-                    let inner_expr = &some_args[0];
-
-                    if contains_return(inner_expr) {
-                        return;
-                    }
-
-                    let some_inner_snip = if inner_expr.span.from_expansion() {
-                        snippet_with_macro_callsite(cx, inner_expr.span, "_")
-                    } else {
-                        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,
-                    );
-                }
-            }
-        },
-        // `_.and_then(Some)` case, which is no-op.
-        hir::ExprKind::Path(ref qpath) => {
-            if match_qpath(qpath, &paths::OPTION_SOME) {
-                let option_snip = snippet(cx, args[0].span, "..");
-                let note = format!("{}", option_snip);
-                span_lint_and_sugg(
-                    cx,
-                    OPTION_AND_THEN_SOME,
-                    expr.span,
-                    NO_OP_MSG,
-                    "use the expression directly",
-                    note,
-                    Applicability::MachineApplicable,
-                );
-            }
-        },
-        _ => {},
-    }
-}
-
 /// lint use of `filter().next()` for `Iterators`
 fn lint_filter_next<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index f50adbc48ab..6ed9ff22e46 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -2230,7 +2230,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher {
             );
 
             if !vis.suggestions.is_empty() {
-                multispan_sugg(diag, "...and use generic constructor".into(), vis.suggestions);
+                multispan_sugg(diag, "...and use generic constructor", vis.suggestions);
             }
         }
 
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index e5e3bf453a0..5b4e2906b5f 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -67,6 +67,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         module: "bit_mask",
     },
     Lint {
+        name: "bind_instead_of_map",
+        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: "blacklisted_name",
         group: "style",
         desc: "usage of a blacklisted/placeholder name",
@@ -1579,13 +1586,6 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         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_as_ref_deref",
         group: "complexity",
         desc: "using `as_ref().map(Deref::deref)`, which is more succinctly expressed as `as_deref()`",
diff --git a/tests/ui/option_and_then_some.fixed b/tests/ui/bind_instead_of_map.fixed
index 035bc1e5465..5815550d7a6 100644
--- a/tests/ui/option_and_then_some.fixed
+++ b/tests/ui/bind_instead_of_map.fixed
@@ -1,5 +1,5 @@
 // run-rustfix
-#![deny(clippy::option_and_then_some)]
+#![deny(clippy::bind_instead_of_map)]
 
 // need a main anyway, use it get rid of unused warnings too
 pub fn main() {
@@ -12,7 +12,7 @@ pub fn main() {
 
     // Different type
     let x: Result<u32, &str> = Ok(1);
-    let _ = x.and_then(Ok);
+    let _ = x;
 }
 
 pub fn foo() -> Option<String> {
diff --git a/tests/ui/option_and_then_some.rs b/tests/ui/bind_instead_of_map.rs
index d49da7813c6..623b100a4ce 100644
--- a/tests/ui/option_and_then_some.rs
+++ b/tests/ui/bind_instead_of_map.rs
@@ -1,5 +1,5 @@
 // run-rustfix
-#![deny(clippy::option_and_then_some)]
+#![deny(clippy::bind_instead_of_map)]
 
 // need a main anyway, use it get rid of unused warnings too
 pub fn main() {
diff --git a/tests/ui/option_and_then_some.stderr b/tests/ui/bind_instead_of_map.stderr
index 47825491765..24c6b7f9ef3 100644
--- a/tests/ui/option_and_then_some.stderr
+++ b/tests/ui/bind_instead_of_map.stderr
@@ -1,20 +1,26 @@
 error: using `Option.and_then(Some)`, which is a no-op
-  --> $DIR/option_and_then_some.rs:8:13
+  --> $DIR/bind_instead_of_map.rs:8:13
    |
 LL |     let _ = x.and_then(Some);
    |             ^^^^^^^^^^^^^^^^ help: use the expression directly: `x`
    |
 note: the lint level is defined here
-  --> $DIR/option_and_then_some.rs:2:9
+  --> $DIR/bind_instead_of_map.rs:2:9
    |
-LL | #![deny(clippy::option_and_then_some)]
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | #![deny(clippy::bind_instead_of_map)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`
-  --> $DIR/option_and_then_some.rs:9:13
+  --> $DIR/bind_instead_of_map.rs:9:13
    |
 LL |     let _ = x.and_then(|o| Some(o + 1));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.map(|o| o + 1)`
 
-error: aborting due to 2 previous errors
+error: using `Result.and_then(Ok)`, which is a no-op
+  --> $DIR/bind_instead_of_map.rs:15:13
+   |
+LL |     let _ = x.and_then(Ok);
+   |             ^^^^^^^^^^^^^^ help: use the expression directly: `x`
+
+error: aborting due to 3 previous errors
 
diff --git a/tests/ui/bind_instead_of_map_multipart.rs b/tests/ui/bind_instead_of_map_multipart.rs
new file mode 100644
index 00000000000..91d9d11e3c1
--- /dev/null
+++ b/tests/ui/bind_instead_of_map_multipart.rs
@@ -0,0 +1,61 @@
+#![deny(clippy::bind_instead_of_map)]
+#![allow(clippy::blocks_in_if_conditions)]
+
+pub fn main() {
+    let _ = Some("42").and_then(|s| if s.len() < 42 { Some(0) } else { Some(s.len()) });
+    let _ = Some("42").and_then(|s| if s.len() < 42 { None } else { Some(s.len()) });
+
+    let _ = Ok::<_, ()>("42").and_then(|s| if s.len() < 42 { Ok(0) } else { Ok(s.len()) });
+    let _ = Ok::<_, ()>("42").and_then(|s| if s.len() < 42 { Err(()) } else { Ok(s.len()) });
+
+    let _ = Err::<(), _>("42").or_else(|s| if s.len() < 42 { Err(s.len() + 20) } else { Err(s.len()) });
+    let _ = Err::<(), _>("42").or_else(|s| if s.len() < 42 { Ok(()) } else { Err(s.len()) });
+
+    hard_example();
+    macro_example();
+}
+
+fn hard_example() {
+    Some("42").and_then(|s| {
+        if {
+            if s == "43" {
+                return Some(43);
+            }
+            s == "42"
+        } {
+            return Some(45);
+        }
+        match s.len() {
+            10 => Some(2),
+            20 => {
+                if foo() {
+                    return {
+                        if foo() {
+                            return Some(20);
+                        }
+                        println!("foo");
+                        Some(3)
+                    };
+                }
+                Some(20)
+            },
+            40 => Some(30),
+            _ => Some(1),
+        }
+    });
+}
+
+fn foo() -> bool {
+    true
+}
+
+macro_rules! m {
+    () => {
+        Some(10)
+    };
+}
+
+fn macro_example() {
+    let _ = Some("").and_then(|s| if s.len() == 20 { m!() } else { Some(20) });
+    let _ = Some("").and_then(|s| if s.len() == 20 { Some(m!()) } else { Some(Some(20)) });
+}
diff --git a/tests/ui/bind_instead_of_map_multipart.stderr b/tests/ui/bind_instead_of_map_multipart.stderr
new file mode 100644
index 00000000000..50ce2f4051e
--- /dev/null
+++ b/tests/ui/bind_instead_of_map_multipart.stderr
@@ -0,0 +1,73 @@
+error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`
+  --> $DIR/bind_instead_of_map_multipart.rs:5:13
+   |
+LL |     let _ = Some("42").and_then(|s| if s.len() < 42 { Some(0) } else { Some(s.len()) });
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/bind_instead_of_map_multipart.rs:1:9
+   |
+LL | #![deny(clippy::bind_instead_of_map)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: try this
+   |
+LL |     let _ = Some("42").map(|s| if s.len() < 42 { 0 } else { s.len() });
+   |                        ^^^                       ^          ^^^^^^^
+
+error: using `Result.and_then(|x| Ok(y))`, which is more succinctly expressed as `map(|x| y)`
+  --> $DIR/bind_instead_of_map_multipart.rs:8:13
+   |
+LL |     let _ = Ok::<_, ()>("42").and_then(|s| if s.len() < 42 { Ok(0) } else { Ok(s.len()) });
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try this
+   |
+LL |     let _ = Ok::<_, ()>("42").map(|s| if s.len() < 42 { 0 } else { s.len() });
+   |                               ^^^                       ^          ^^^^^^^
+
+error: using `Result.or_else(|x| Err(y))`, which is more succinctly expressed as `map_err(|x| y)`
+  --> $DIR/bind_instead_of_map_multipart.rs:11:13
+   |
+LL |     let _ = Err::<(), _>("42").or_else(|s| if s.len() < 42 { Err(s.len() + 20) } else { Err(s.len()) });
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try this
+   |
+LL |     let _ = Err::<(), _>("42").map_err(|s| if s.len() < 42 { s.len() + 20 } else { s.len() });
+   |                                ^^^^^^^                       ^^^^^^^^^^^^          ^^^^^^^
+
+error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`
+  --> $DIR/bind_instead_of_map_multipart.rs:19:5
+   |
+LL | /     Some("42").and_then(|s| {
+LL | |         if {
+LL | |             if s == "43" {
+LL | |                 return Some(43);
+...  |
+LL | |         }
+LL | |     });
+   | |______^
+   |
+help: try this
+   |
+LL |     Some("42").map(|s| {
+LL |         if {
+LL |             if s == "43" {
+LL |                 return 43;
+LL |             }
+LL |             s == "42"
+ ...
+
+error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`
+  --> $DIR/bind_instead_of_map_multipart.rs:60:13
+   |
+LL |     let _ = Some("").and_then(|s| if s.len() == 20 { Some(m!()) } else { Some(Some(20)) });
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try this
+   |
+LL |     let _ = Some("").map(|s| if s.len() == 20 { m!() } else { Some(20) });
+   |                      ^^^                        ^^^^          ^^^^^^^^
+
+error: aborting due to 5 previous errors
+
diff --git a/tests/ui/blocks_in_if_conditions.fixed b/tests/ui/blocks_in_if_conditions.fixed
index 9040552cefc..14562c4d32c 100644
--- a/tests/ui/blocks_in_if_conditions.fixed
+++ b/tests/ui/blocks_in_if_conditions.fixed
@@ -63,10 +63,10 @@ fn block_in_assert() {
     let opt = Some(42);
     assert!(opt
         .as_ref()
-        .and_then(|val| {
+        .map(|val| {
             let mut v = val * 2;
             v -= 1;
-            Some(v * 3)
+            v * 3
         })
         .is_some());
 }
diff --git a/tests/ui/blocks_in_if_conditions.rs b/tests/ui/blocks_in_if_conditions.rs
index 2fe409b22d3..bda87650f6d 100644
--- a/tests/ui/blocks_in_if_conditions.rs
+++ b/tests/ui/blocks_in_if_conditions.rs
@@ -63,10 +63,10 @@ fn block_in_assert() {
     let opt = Some(42);
     assert!(opt
         .as_ref()
-        .and_then(|val| {
+        .map(|val| {
             let mut v = val * 2;
             v -= 1;
-            Some(v * 3)
+            v * 3
         })
         .is_some());
 }
diff --git a/tests/ui/option_map_or_none.fixed b/tests/ui/option_map_or_none.fixed
index decbae4e5af..d80c3c7c1b7 100644
--- a/tests/ui/option_map_or_none.fixed
+++ b/tests/ui/option_map_or_none.fixed
@@ -1,6 +1,6 @@
 // run-rustfix
 
-#![allow(clippy::option_and_then_some)]
+#![allow(clippy::bind_instead_of_map)]
 
 fn main() {
     let opt = Some(1);
diff --git a/tests/ui/option_map_or_none.rs b/tests/ui/option_map_or_none.rs
index 0f1d2218d5d..629842419e5 100644
--- a/tests/ui/option_map_or_none.rs
+++ b/tests/ui/option_map_or_none.rs
@@ -1,6 +1,6 @@
 // run-rustfix
 
-#![allow(clippy::option_and_then_some)]
+#![allow(clippy::bind_instead_of_map)]
 
 fn main() {
     let opt = Some(1);