mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 07:04:18 +00:00
Make BindInsteadOfMap
a struct
Makes it codegen once instead of three times
This commit is contained in:
parent
bd1224d8aa
commit
493498b65f
3 changed files with 68 additions and 49 deletions
|
@ -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"
|
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"
|
reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint*` functions instead"
|
||||||
|
|
||||||
|
|
||||||
[[disallowed-methods]]
|
[[disallowed-methods]]
|
||||||
path = "rustc_middle::ty::context::TyCtxt::node_span_lint"
|
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"
|
reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint_hir*` functions instead"
|
||||||
|
|
|
@ -10,59 +10,80 @@ use rustc_hir::{LangItem, QPath};
|
||||||
use rustc_lint::LateContext;
|
use rustc_lint::LateContext;
|
||||||
use rustc_span::Span;
|
use rustc_span::Span;
|
||||||
|
|
||||||
pub(crate) struct OptionAndThenSome;
|
pub(super) fn check_and_then_some(
|
||||||
|
cx: &LateContext<'_>,
|
||||||
impl BindInsteadOfMap for OptionAndThenSome {
|
expr: &hir::Expr<'_>,
|
||||||
const VARIANT_LANG_ITEM: LangItem = LangItem::OptionSome;
|
recv: &hir::Expr<'_>,
|
||||||
const BAD_METHOD_NAME: &'static str = "and_then";
|
arg: &hir::Expr<'_>,
|
||||||
const GOOD_METHOD_NAME: &'static str = "map";
|
) -> 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;
|
pub(super) fn check_and_then_ok(
|
||||||
|
cx: &LateContext<'_>,
|
||||||
impl BindInsteadOfMap for ResultAndThenOk {
|
expr: &hir::Expr<'_>,
|
||||||
const VARIANT_LANG_ITEM: LangItem = LangItem::ResultOk;
|
recv: &hir::Expr<'_>,
|
||||||
const BAD_METHOD_NAME: &'static str = "and_then";
|
arg: &hir::Expr<'_>,
|
||||||
const GOOD_METHOD_NAME: &'static str = "map";
|
) -> 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;
|
pub(super) fn check_or_else_err(
|
||||||
|
cx: &LateContext<'_>,
|
||||||
impl BindInsteadOfMap for ResultOrElseErrInfo {
|
expr: &hir::Expr<'_>,
|
||||||
const VARIANT_LANG_ITEM: LangItem = LangItem::ResultErr;
|
recv: &hir::Expr<'_>,
|
||||||
const BAD_METHOD_NAME: &'static str = "or_else";
|
arg: &hir::Expr<'_>,
|
||||||
const GOOD_METHOD_NAME: &'static str = "map_err";
|
) -> 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 {
|
struct BindInsteadOfMap {
|
||||||
const VARIANT_LANG_ITEM: LangItem;
|
variant_lang_item: LangItem,
|
||||||
const BAD_METHOD_NAME: &'static str;
|
bad_method_name: &'static str,
|
||||||
const GOOD_METHOD_NAME: &'static str;
|
good_method_name: &'static str,
|
||||||
|
}
|
||||||
|
|
||||||
fn no_op_msg(cx: &LateContext<'_>) -> Option<String> {
|
impl BindInsteadOfMap {
|
||||||
let variant_id = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)?;
|
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);
|
let item_id = cx.tcx.parent(variant_id);
|
||||||
Some(format!(
|
Some(format!(
|
||||||
"using `{}.{}({})`, which is a no-op",
|
"using `{}.{}({})`, which is a no-op",
|
||||||
cx.tcx.item_name(item_id),
|
cx.tcx.item_name(item_id),
|
||||||
Self::BAD_METHOD_NAME,
|
self.bad_method_name,
|
||||||
cx.tcx.item_name(variant_id),
|
cx.tcx.item_name(variant_id),
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
fn lint_msg(cx: &LateContext<'_>) -> Option<String> {
|
fn lint_msg(&self, cx: &LateContext<'_>) -> Option<String> {
|
||||||
let variant_id = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)?;
|
let variant_id = cx.tcx.lang_items().get(self.variant_lang_item)?;
|
||||||
let item_id = cx.tcx.parent(variant_id);
|
let item_id = cx.tcx.parent(variant_id);
|
||||||
Some(format!(
|
Some(format!(
|
||||||
"using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`",
|
"using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`",
|
||||||
cx.tcx.item_name(item_id),
|
cx.tcx.item_name(item_id),
|
||||||
Self::BAD_METHOD_NAME,
|
self.bad_method_name,
|
||||||
cx.tcx.item_name(variant_id),
|
cx.tcx.item_name(variant_id),
|
||||||
Self::GOOD_METHOD_NAME
|
self.good_method_name,
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
fn lint_closure_autofixable(
|
fn lint_closure_autofixable(
|
||||||
|
&self,
|
||||||
cx: &LateContext<'_>,
|
cx: &LateContext<'_>,
|
||||||
expr: &hir::Expr<'_>,
|
expr: &hir::Expr<'_>,
|
||||||
recv: &hir::Expr<'_>,
|
recv: &hir::Expr<'_>,
|
||||||
|
@ -71,9 +92,9 @@ pub(crate) trait BindInsteadOfMap {
|
||||||
) -> bool {
|
) -> bool {
|
||||||
if let hir::ExprKind::Call(some_expr, [inner_expr]) = closure_expr.kind
|
if let hir::ExprKind::Call(some_expr, [inner_expr]) = closure_expr.kind
|
||||||
&& let hir::ExprKind::Path(QPath::Resolved(_, path)) = some_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)
|
&& !contains_return(inner_expr)
|
||||||
&& let Some(msg) = Self::lint_msg(cx)
|
&& let Some(msg) = self.lint_msg(cx)
|
||||||
{
|
{
|
||||||
let mut app = Applicability::MachineApplicable;
|
let mut app = Applicability::MachineApplicable;
|
||||||
let some_inner_snip = snippet_with_context(cx, inner_expr.span, closure_expr.span.ctxt(), "_", &mut app).0;
|
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 option_snip = snippet(cx, recv.span, "..");
|
||||||
let note = format!(
|
let note = format!(
|
||||||
"{option_snip}.{}({closure_args_snip} {some_inner_snip})",
|
"{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);
|
span_lint_and_sugg(cx, BIND_INSTEAD_OF_MAP, expr.span, msg, "try", note, app);
|
||||||
true
|
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 mut suggs = Vec::new();
|
||||||
let can_sugg: bool = find_all_ret_expressions(cx, closure_expr, |ret_expr| {
|
let can_sugg: bool = find_all_ret_expressions(cx, closure_expr, |ret_expr| {
|
||||||
if !ret_expr.span.from_expansion()
|
if !ret_expr.span.from_expansion()
|
||||||
&& let hir::ExprKind::Call(func_path, [arg]) = ret_expr.kind
|
&& let hir::ExprKind::Call(func_path, [arg]) = ret_expr.kind
|
||||||
&& let hir::ExprKind::Path(QPath::Resolved(_, path)) = func_path.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)
|
&& !contains_return(arg)
|
||||||
{
|
{
|
||||||
suggs.push((ret_expr.span, arg.span.source_callsite()));
|
suggs.push((ret_expr.span, arg.span.source_callsite()));
|
||||||
|
@ -108,7 +129,7 @@ pub(crate) trait BindInsteadOfMap {
|
||||||
});
|
});
|
||||||
let (span, msg) = if can_sugg
|
let (span, msg) = if can_sugg
|
||||||
&& let hir::ExprKind::MethodCall(segment, ..) = expr.kind
|
&& 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)
|
(segment.ident.span, msg)
|
||||||
} else {
|
} else {
|
||||||
|
@ -119,7 +140,7 @@ pub(crate) trait BindInsteadOfMap {
|
||||||
diag,
|
diag,
|
||||||
"try",
|
"try",
|
||||||
Applicability::MachineApplicable,
|
Applicability::MachineApplicable,
|
||||||
std::iter::once((span, Self::GOOD_METHOD_NAME.into())).chain(
|
std::iter::once((span, self.good_method_name.into())).chain(
|
||||||
suggs
|
suggs
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())),
|
.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
|
/// 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()
|
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)
|
&& adt.did() == cx.tcx.parent(vid)
|
||||||
{
|
{
|
||||||
} else {
|
} else {
|
||||||
|
@ -144,15 +165,15 @@ pub(crate) trait BindInsteadOfMap {
|
||||||
let closure_body = cx.tcx.hir().body(body);
|
let closure_body = cx.tcx.hir().body(body);
|
||||||
let closure_expr = peel_blocks(closure_body.value);
|
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
|
true
|
||||||
} else {
|
} else {
|
||||||
Self::lint_closure(cx, expr, closure_expr)
|
self.lint_closure(cx, expr, closure_expr)
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
// `_.and_then(Some)` case, which is no-op.
|
// `_.and_then(Some)` case, which is no-op.
|
||||||
hir::ExprKind::Path(QPath::Resolved(_, path)) if Self::is_variant(cx, path.res) => {
|
hir::ExprKind::Path(QPath::Resolved(_, path)) if self.is_variant(cx, path.res) => {
|
||||||
if let Some(msg) = Self::no_op_msg(cx) {
|
if let Some(msg) = self.no_op_msg(cx) {
|
||||||
span_lint_and_sugg(
|
span_lint_and_sugg(
|
||||||
cx,
|
cx,
|
||||||
BIND_INSTEAD_OF_MAP,
|
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 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;
|
return cx.tcx.parent(id) == variant_id;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -131,7 +131,6 @@ mod waker_clone_wake;
|
||||||
mod wrong_self_convention;
|
mod wrong_self_convention;
|
||||||
mod zst_offset;
|
mod zst_offset;
|
||||||
|
|
||||||
use bind_instead_of_map::BindInsteadOfMap;
|
|
||||||
use clippy_config::msrvs::{self, Msrv};
|
use clippy_config::msrvs::{self, Msrv};
|
||||||
use clippy_config::Conf;
|
use clippy_config::Conf;
|
||||||
use clippy_utils::consts::{constant, Constant};
|
use clippy_utils::consts::{constant, Constant};
|
||||||
|
@ -4506,8 +4505,8 @@ impl Methods {
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
("and_then", [arg]) => {
|
("and_then", [arg]) => {
|
||||||
let biom_option_linted = bind_instead_of_map::OptionAndThenSome::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::ResultAndThenOk::check(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 {
|
if !biom_option_linted && !biom_result_linted {
|
||||||
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
|
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
|
||||||
}
|
}
|
||||||
|
@ -4847,7 +4846,7 @@ impl Methods {
|
||||||
open_options::check(cx, expr, recv);
|
open_options::check(cx, expr, recv);
|
||||||
},
|
},
|
||||||
("or_else", [arg]) => {
|
("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");
|
unnecessary_lazy_eval::check(cx, expr, recv, arg, "or");
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
Loading…
Reference in a new issue