Skip to content

Commit 959477e

Browse files
borsbors
committed
Auto merge of rust-lang#13153 - Alexendoo:bind-instead-of-map-struct, r=Manishearth
Make `BindInsteadOfMap` a struct Makes it codegen once instead of three times changelog: none
2 parents 615284a + 493498b commit 959477e

File tree

3 files changed

+68
-49
lines changed

3 files changed

+68
-49
lines changed

‎clippy.toml

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ reason = "this function does not add a link to our documentation, please use the
88
path = "rustc_lint::context::LintContext::span_lint"
99
reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint*` functions instead"
1010

11-
1211
[[disallowed-methods]]
1312
path = "rustc_middle::ty::context::TyCtxt::node_span_lint"
1413
reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint_hir*` functions instead"

‎clippy_lints/src/methods/bind_instead_of_map.rs

+65-44
Original file line numberDiff line numberDiff line change
@@ -10,59 +10,80 @@ use rustc_hir::{LangItem, QPath};
1010
use rustc_lint::LateContext;
1111
use rustc_span::Span;
1212

13-
pub(crate) struct OptionAndThenSome;
14-
15-
impl BindInsteadOfMap for OptionAndThenSome {
16-
const VARIANT_LANG_ITEM: LangItem = LangItem::OptionSome;
17-
const BAD_METHOD_NAME: &'static str = "and_then";
18-
const GOOD_METHOD_NAME: &'static str = "map";
13+
pub(super) fn check_and_then_some(
14+
cx: &LateContext<'_>,
15+
expr: &hir::Expr<'_>,
16+
recv: &hir::Expr<'_>,
17+
arg: &hir::Expr<'_>,
18+
) -> bool {
19+
BindInsteadOfMap {
20+
variant_lang_item: LangItem::OptionSome,
21+
bad_method_name: "and_then",
22+
good_method_name: "map",
23+
}
24+
.check(cx, expr, recv, arg)
1925
}
2026

21-
pub(crate) struct ResultAndThenOk;
22-
23-
impl BindInsteadOfMap for ResultAndThenOk {
24-
const VARIANT_LANG_ITEM: LangItem = LangItem::ResultOk;
25-
const BAD_METHOD_NAME: &'static str = "and_then";
26-
const GOOD_METHOD_NAME: &'static str = "map";
27+
pub(super) fn check_and_then_ok(
28+
cx: &LateContext<'_>,
29+
expr: &hir::Expr<'_>,
30+
recv: &hir::Expr<'_>,
31+
arg: &hir::Expr<'_>,
32+
) -> bool {
33+
BindInsteadOfMap {
34+
variant_lang_item: LangItem::ResultOk,
35+
bad_method_name: "and_then",
36+
good_method_name: "map",
37+
}
38+
.check(cx, expr, recv, arg)
2739
}
2840

29-
pub(crate) struct ResultOrElseErrInfo;
30-
31-
impl BindInsteadOfMap for ResultOrElseErrInfo {
32-
const VARIANT_LANG_ITEM: LangItem = LangItem::ResultErr;
33-
const BAD_METHOD_NAME: &'static str = "or_else";
34-
const GOOD_METHOD_NAME: &'static str = "map_err";
41+
pub(super) fn check_or_else_err(
42+
cx: &LateContext<'_>,
43+
expr: &hir::Expr<'_>,
44+
recv: &hir::Expr<'_>,
45+
arg: &hir::Expr<'_>,
46+
) -> bool {
47+
BindInsteadOfMap {
48+
variant_lang_item: LangItem::ResultErr,
49+
bad_method_name: "or_else",
50+
good_method_name: "map_err",
51+
}
52+
.check(cx, expr, recv, arg)
3553
}
3654

37-
pub(crate) trait BindInsteadOfMap {
38-
const VARIANT_LANG_ITEM: LangItem;
39-
const BAD_METHOD_NAME: &'static str;
40-
const GOOD_METHOD_NAME: &'static str;
55+
struct BindInsteadOfMap {
56+
variant_lang_item: LangItem,
57+
bad_method_name: &'static str,
58+
good_method_name: &'static str,
59+
}
4160

42-
fn no_op_msg(cx: &LateContext<'_>) -> Option<String> {
43-
let variant_id = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)?;
61+
impl BindInsteadOfMap {
62+
fn no_op_msg(&self, cx: &LateContext<'_>) -> Option<String> {
63+
let variant_id = cx.tcx.lang_items().get(self.variant_lang_item)?;
4464
let item_id = cx.tcx.parent(variant_id);
4565
Some(format!(
4666
"using `{}.{}({})`, which is a no-op",
4767
cx.tcx.item_name(item_id),
48-
Self::BAD_METHOD_NAME,
68+
self.bad_method_name,
4969
cx.tcx.item_name(variant_id),
5070
))
5171
}
5272

53-
fn lint_msg(cx: &LateContext<'_>) -> Option<String> {
54-
let variant_id = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)?;
73+
fn lint_msg(&self, cx: &LateContext<'_>) -> Option<String> {
74+
let variant_id = cx.tcx.lang_items().get(self.variant_lang_item)?;
5575
let item_id = cx.tcx.parent(variant_id);
5676
Some(format!(
5777
"using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`",
5878
cx.tcx.item_name(item_id),
59-
Self::BAD_METHOD_NAME,
79+
self.bad_method_name,
6080
cx.tcx.item_name(variant_id),
61-
Self::GOOD_METHOD_NAME
81+
self.good_method_name,
6282
))
6383
}
6484

6585
fn lint_closure_autofixable(
86+
&self,
6687
cx: &LateContext<'_>,
6788
expr: &hir::Expr<'_>,
6889
recv: &hir::Expr<'_>,
@@ -71,9 +92,9 @@ pub(crate) trait BindInsteadOfMap {
7192
) -> bool {
7293
if let hir::ExprKind::Call(some_expr, [inner_expr]) = closure_expr.kind
7394
&& let hir::ExprKind::Path(QPath::Resolved(_, path)) = some_expr.kind
74-
&& Self::is_variant(cx, path.res)
95+
&& self.is_variant(cx, path.res)
7596
&& !contains_return(inner_expr)
76-
&& let Some(msg) = Self::lint_msg(cx)
97+
&& let Some(msg) = self.lint_msg(cx)
7798
{
7899
let mut app = Applicability::MachineApplicable;
79100
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 {
82103
let option_snip = snippet(cx, recv.span, "..");
83104
let note = format!(
84105
"{option_snip}.{}({closure_args_snip} {some_inner_snip})",
85-
Self::GOOD_METHOD_NAME
106+
self.good_method_name
86107
);
87108
span_lint_and_sugg(cx, BIND_INSTEAD_OF_MAP, expr.span, msg, "try", note, app);
88109
true
@@ -91,13 +112,13 @@ pub(crate) trait BindInsteadOfMap {
91112
}
92113
}
93114

94-
fn lint_closure(cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) -> bool {
115+
fn lint_closure(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) -> bool {
95116
let mut suggs = Vec::new();
96117
let can_sugg: bool = find_all_ret_expressions(cx, closure_expr, |ret_expr| {
97118
if !ret_expr.span.from_expansion()
98119
&& let hir::ExprKind::Call(func_path, [arg]) = ret_expr.kind
99120
&& let hir::ExprKind::Path(QPath::Resolved(_, path)) = func_path.kind
100-
&& Self::is_variant(cx, path.res)
121+
&& self.is_variant(cx, path.res)
101122
&& !contains_return(arg)
102123
{
103124
suggs.push((ret_expr.span, arg.span.source_callsite()));
@@ -108,7 +129,7 @@ pub(crate) trait BindInsteadOfMap {
108129
});
109130
let (span, msg) = if can_sugg
110131
&& let hir::ExprKind::MethodCall(segment, ..) = expr.kind
111-
&& let Some(msg) = Self::lint_msg(cx)
132+
&& let Some(msg) = self.lint_msg(cx)
112133
{
113134
(segment.ident.span, msg)
114135
} else {
@@ -119,7 +140,7 @@ pub(crate) trait BindInsteadOfMap {
119140
diag,
120141
"try",
121142
Applicability::MachineApplicable,
122-
std::iter::once((span, Self::GOOD_METHOD_NAME.into())).chain(
143+
std::iter::once((span, self.good_method_name.into())).chain(
123144
suggs
124145
.into_iter()
125146
.map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())),
@@ -130,9 +151,9 @@ pub(crate) trait BindInsteadOfMap {
130151
}
131152

132153
/// Lint use of `_.and_then(|x| Some(y))` for `Option`s
133-
fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool {
154+
fn check(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool {
134155
if let Some(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def()
135-
&& let Some(vid) = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)
156+
&& let Some(vid) = cx.tcx.lang_items().get(self.variant_lang_item)
136157
&& adt.did() == cx.tcx.parent(vid)
137158
{
138159
} else {
@@ -144,15 +165,15 @@ pub(crate) trait BindInsteadOfMap {
144165
let closure_body = cx.tcx.hir().body(body);
145166
let closure_expr = peel_blocks(closure_body.value);
146167

147-
if Self::lint_closure_autofixable(cx, expr, recv, closure_expr, fn_decl_span) {
168+
if self.lint_closure_autofixable(cx, expr, recv, closure_expr, fn_decl_span) {
148169
true
149170
} else {
150-
Self::lint_closure(cx, expr, closure_expr)
171+
self.lint_closure(cx, expr, closure_expr)
151172
}
152173
},
153174
// `_.and_then(Some)` case, which is no-op.
154-
hir::ExprKind::Path(QPath::Resolved(_, path)) if Self::is_variant(cx, path.res) => {
155-
if let Some(msg) = Self::no_op_msg(cx) {
175+
hir::ExprKind::Path(QPath::Resolved(_, path)) if self.is_variant(cx, path.res) => {
176+
if let Some(msg) = self.no_op_msg(cx) {
156177
span_lint_and_sugg(
157178
cx,
158179
BIND_INSTEAD_OF_MAP,
@@ -169,9 +190,9 @@ pub(crate) trait BindInsteadOfMap {
169190
}
170191
}
171192

172-
fn is_variant(cx: &LateContext<'_>, res: Res) -> bool {
193+
fn is_variant(&self, cx: &LateContext<'_>, res: Res) -> bool {
173194
if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {
174-
if let Some(variant_id) = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM) {
195+
if let Some(variant_id) = cx.tcx.lang_items().get(self.variant_lang_item) {
175196
return cx.tcx.parent(id) == variant_id;
176197
}
177198
}

‎clippy_lints/src/methods/mod.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ mod waker_clone_wake;
131131
mod wrong_self_convention;
132132
mod zst_offset;
133133

134-
use bind_instead_of_map::BindInsteadOfMap;
135134
use clippy_config::msrvs::{self, Msrv};
136135
use clippy_config::Conf;
137136
use clippy_utils::consts::{constant, Constant};
@@ -4506,8 +4505,8 @@ impl Methods {
45064505
}
45074506
},
45084507
("and_then", [arg]) => {
4509-
let biom_option_linted = bind_instead_of_map::OptionAndThenSome::check(cx, expr, recv, arg);
4510-
let biom_result_linted = bind_instead_of_map::ResultAndThenOk::check(cx, expr, recv, arg);
4508+
let biom_option_linted = bind_instead_of_map::check_and_then_some(cx, expr, recv, arg);
4509+
let biom_result_linted = bind_instead_of_map::check_and_then_ok(cx, expr, recv, arg);
45114510
if !biom_option_linted && !biom_result_linted {
45124511
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
45134512
}
@@ -4847,7 +4846,7 @@ impl Methods {
48474846
open_options::check(cx, expr, recv);
48484847
},
48494848
("or_else", [arg]) => {
4850-
if !bind_instead_of_map::ResultOrElseErrInfo::check(cx, expr, recv, arg) {
4849+
if !bind_instead_of_map::check_or_else_err(cx, expr, recv, arg) {
48514850
unnecessary_lazy_eval::check(cx, expr, recv, arg, "or");
48524851
}
48534852
},

0 commit comments

Comments
 (0)