about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAlex Macleod <alex@macleod.io>2024-07-23 20:57:35 +0000
committerAlex Macleod <alex@macleod.io>2024-07-23 20:57:35 +0000
commit493498b65fbd1a7a5bc9c0c9cfc906083ca5439f (patch)
tree8896540c673cfa301a0e604959b6b8e7bc8d65f7
parentbd1224d8aa1cc90ff8c5a2ba3f90785ec97845f5 (diff)
downloadrust-493498b65fbd1a7a5bc9c0c9cfc906083ca5439f.tar.gz
rust-493498b65fbd1a7a5bc9c0c9cfc906083ca5439f.zip
Make `BindInsteadOfMap` a struct
Makes it codegen once instead of three times
-rw-r--r--clippy.toml1
-rw-r--r--clippy_lints/src/methods/bind_instead_of_map.rs109
-rw-r--r--clippy_lints/src/methods/mod.rs7
3 files changed, 68 insertions, 49 deletions
diff --git a/clippy.toml b/clippy.toml
index 319b72e8c5d..a7b0cc56ea1 100644
--- a/clippy.toml
+++ b/clippy.toml
@@ -8,7 +8,6 @@ reason = "this function does not add a link to our documentation, please use the
 path = "rustc_lint::context::LintContext::span_lint"
 reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint*` functions instead"
 
-
 [[disallowed-methods]]
 path = "rustc_middle::ty::context::TyCtxt::node_span_lint"
 reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint_hir*` functions instead"
diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs
index fb440ce656e..20f3722e173 100644
--- a/clippy_lints/src/methods/bind_instead_of_map.rs
+++ b/clippy_lints/src/methods/bind_instead_of_map.rs
@@ -10,59 +10,80 @@ use rustc_hir::{LangItem, QPath};
 use rustc_lint::LateContext;
 use rustc_span::Span;
 
-pub(crate) struct OptionAndThenSome;
-
-impl BindInsteadOfMap for OptionAndThenSome {
-    const VARIANT_LANG_ITEM: LangItem = LangItem::OptionSome;
-    const BAD_METHOD_NAME: &'static str = "and_then";
-    const GOOD_METHOD_NAME: &'static str = "map";
+pub(super) fn check_and_then_some(
+    cx: &LateContext<'_>,
+    expr: &hir::Expr<'_>,
+    recv: &hir::Expr<'_>,
+    arg: &hir::Expr<'_>,
+) -> bool {
+    BindInsteadOfMap {
+        variant_lang_item: LangItem::OptionSome,
+        bad_method_name: "and_then",
+        good_method_name: "map",
+    }
+    .check(cx, expr, recv, arg)
 }
 
-pub(crate) struct ResultAndThenOk;
-
-impl BindInsteadOfMap for ResultAndThenOk {
-    const VARIANT_LANG_ITEM: LangItem = LangItem::ResultOk;
-    const BAD_METHOD_NAME: &'static str = "and_then";
-    const GOOD_METHOD_NAME: &'static str = "map";
+pub(super) fn check_and_then_ok(
+    cx: &LateContext<'_>,
+    expr: &hir::Expr<'_>,
+    recv: &hir::Expr<'_>,
+    arg: &hir::Expr<'_>,
+) -> bool {
+    BindInsteadOfMap {
+        variant_lang_item: LangItem::ResultOk,
+        bad_method_name: "and_then",
+        good_method_name: "map",
+    }
+    .check(cx, expr, recv, arg)
 }
 
-pub(crate) struct ResultOrElseErrInfo;
-
-impl BindInsteadOfMap for ResultOrElseErrInfo {
-    const VARIANT_LANG_ITEM: LangItem = LangItem::ResultErr;
-    const BAD_METHOD_NAME: &'static str = "or_else";
-    const GOOD_METHOD_NAME: &'static str = "map_err";
+pub(super) fn check_or_else_err(
+    cx: &LateContext<'_>,
+    expr: &hir::Expr<'_>,
+    recv: &hir::Expr<'_>,
+    arg: &hir::Expr<'_>,
+) -> bool {
+    BindInsteadOfMap {
+        variant_lang_item: LangItem::ResultErr,
+        bad_method_name: "or_else",
+        good_method_name: "map_err",
+    }
+    .check(cx, expr, recv, arg)
 }
 
-pub(crate) trait BindInsteadOfMap {
-    const VARIANT_LANG_ITEM: LangItem;
-    const BAD_METHOD_NAME: &'static str;
-    const GOOD_METHOD_NAME: &'static str;
+struct BindInsteadOfMap {
+    variant_lang_item: LangItem,
+    bad_method_name: &'static str,
+    good_method_name: &'static str,
+}
 
-    fn no_op_msg(cx: &LateContext<'_>) -> Option<String> {
-        let variant_id = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)?;
+impl BindInsteadOfMap {
+    fn no_op_msg(&self, cx: &LateContext<'_>) -> Option<String> {
+        let variant_id = cx.tcx.lang_items().get(self.variant_lang_item)?;
         let item_id = cx.tcx.parent(variant_id);
         Some(format!(
             "using `{}.{}({})`, which is a no-op",
             cx.tcx.item_name(item_id),
-            Self::BAD_METHOD_NAME,
+            self.bad_method_name,
             cx.tcx.item_name(variant_id),
         ))
     }
 
-    fn lint_msg(cx: &LateContext<'_>) -> Option<String> {
-        let variant_id = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)?;
+    fn lint_msg(&self, cx: &LateContext<'_>) -> Option<String> {
+        let variant_id = cx.tcx.lang_items().get(self.variant_lang_item)?;
         let item_id = cx.tcx.parent(variant_id);
         Some(format!(
             "using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`",
             cx.tcx.item_name(item_id),
-            Self::BAD_METHOD_NAME,
+            self.bad_method_name,
             cx.tcx.item_name(variant_id),
-            Self::GOOD_METHOD_NAME
+            self.good_method_name,
         ))
     }
 
     fn lint_closure_autofixable(
+        &self,
         cx: &LateContext<'_>,
         expr: &hir::Expr<'_>,
         recv: &hir::Expr<'_>,
@@ -71,9 +92,9 @@ pub(crate) trait BindInsteadOfMap {
     ) -> bool {
         if let hir::ExprKind::Call(some_expr, [inner_expr]) = closure_expr.kind
             && let hir::ExprKind::Path(QPath::Resolved(_, path)) = some_expr.kind
-            && Self::is_variant(cx, path.res)
+            && self.is_variant(cx, path.res)
             && !contains_return(inner_expr)
-            && let Some(msg) = Self::lint_msg(cx)
+            && let Some(msg) = self.lint_msg(cx)
         {
             let mut app = Applicability::MachineApplicable;
             let some_inner_snip = snippet_with_context(cx, inner_expr.span, closure_expr.span.ctxt(), "_", &mut app).0;
@@ -82,7 +103,7 @@ pub(crate) trait BindInsteadOfMap {
             let option_snip = snippet(cx, recv.span, "..");
             let note = format!(
                 "{option_snip}.{}({closure_args_snip} {some_inner_snip})",
-                Self::GOOD_METHOD_NAME
+                self.good_method_name
             );
             span_lint_and_sugg(cx, BIND_INSTEAD_OF_MAP, expr.span, msg, "try", note, app);
             true
@@ -91,13 +112,13 @@ pub(crate) trait BindInsteadOfMap {
         }
     }
 
-    fn lint_closure(cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) -> bool {
+    fn lint_closure(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) -> bool {
         let mut suggs = Vec::new();
         let can_sugg: bool = find_all_ret_expressions(cx, closure_expr, |ret_expr| {
             if !ret_expr.span.from_expansion()
                 && let hir::ExprKind::Call(func_path, [arg]) = ret_expr.kind
                 && let hir::ExprKind::Path(QPath::Resolved(_, path)) = func_path.kind
-                && Self::is_variant(cx, path.res)
+                && self.is_variant(cx, path.res)
                 && !contains_return(arg)
             {
                 suggs.push((ret_expr.span, arg.span.source_callsite()));
@@ -108,7 +129,7 @@ pub(crate) trait BindInsteadOfMap {
         });
         let (span, msg) = if can_sugg
             && let hir::ExprKind::MethodCall(segment, ..) = expr.kind
-            && let Some(msg) = Self::lint_msg(cx)
+            && let Some(msg) = self.lint_msg(cx)
         {
             (segment.ident.span, msg)
         } else {
@@ -119,7 +140,7 @@ pub(crate) trait BindInsteadOfMap {
                 diag,
                 "try",
                 Applicability::MachineApplicable,
-                std::iter::once((span, Self::GOOD_METHOD_NAME.into())).chain(
+                std::iter::once((span, self.good_method_name.into())).chain(
                     suggs
                         .into_iter()
                         .map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())),
@@ -130,9 +151,9 @@ pub(crate) trait BindInsteadOfMap {
     }
 
     /// Lint use of `_.and_then(|x| Some(y))` for `Option`s
-    fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool {
+    fn check(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool {
         if let Some(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def()
-            && let Some(vid) = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)
+            && let Some(vid) = cx.tcx.lang_items().get(self.variant_lang_item)
             && adt.did() == cx.tcx.parent(vid)
         {
         } else {
@@ -144,15 +165,15 @@ pub(crate) trait BindInsteadOfMap {
                 let closure_body = cx.tcx.hir().body(body);
                 let closure_expr = peel_blocks(closure_body.value);
 
-                if Self::lint_closure_autofixable(cx, expr, recv, closure_expr, fn_decl_span) {
+                if self.lint_closure_autofixable(cx, expr, recv, closure_expr, fn_decl_span) {
                     true
                 } else {
-                    Self::lint_closure(cx, expr, closure_expr)
+                    self.lint_closure(cx, expr, closure_expr)
                 }
             },
             // `_.and_then(Some)` case, which is no-op.
-            hir::ExprKind::Path(QPath::Resolved(_, path)) if Self::is_variant(cx, path.res) => {
-                if let Some(msg) = Self::no_op_msg(cx) {
+            hir::ExprKind::Path(QPath::Resolved(_, path)) if self.is_variant(cx, path.res) => {
+                if let Some(msg) = self.no_op_msg(cx) {
                     span_lint_and_sugg(
                         cx,
                         BIND_INSTEAD_OF_MAP,
@@ -169,9 +190,9 @@ pub(crate) trait BindInsteadOfMap {
         }
     }
 
-    fn is_variant(cx: &LateContext<'_>, res: Res) -> bool {
+    fn is_variant(&self, cx: &LateContext<'_>, res: Res) -> bool {
         if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {
-            if let Some(variant_id) = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM) {
+            if let Some(variant_id) = cx.tcx.lang_items().get(self.variant_lang_item) {
                 return cx.tcx.parent(id) == variant_id;
             }
         }
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 12a3a36e8f6..aa4cad9342a 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -131,7 +131,6 @@ mod waker_clone_wake;
 mod wrong_self_convention;
 mod zst_offset;
 
-use bind_instead_of_map::BindInsteadOfMap;
 use clippy_config::msrvs::{self, Msrv};
 use clippy_config::Conf;
 use clippy_utils::consts::{constant, Constant};
@@ -4506,8 +4505,8 @@ impl Methods {
                     }
                 },
                 ("and_then", [arg]) => {
-                    let biom_option_linted = bind_instead_of_map::OptionAndThenSome::check(cx, expr, recv, arg);
-                    let biom_result_linted = bind_instead_of_map::ResultAndThenOk::check(cx, expr, recv, arg);
+                    let biom_option_linted = bind_instead_of_map::check_and_then_some(cx, expr, recv, arg);
+                    let biom_result_linted = bind_instead_of_map::check_and_then_ok(cx, expr, recv, arg);
                     if !biom_option_linted && !biom_result_linted {
                         unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
                     }
@@ -4847,7 +4846,7 @@ impl Methods {
                     open_options::check(cx, expr, recv);
                 },
                 ("or_else", [arg]) => {
-                    if !bind_instead_of_map::ResultOrElseErrInfo::check(cx, expr, recv, arg) {
+                    if !bind_instead_of_map::check_or_else_err(cx, expr, recv, arg) {
                         unnecessary_lazy_eval::check(cx, expr, recv, arg, "or");
                     }
                 },