diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs index 8ccb8f4268ce9..46d4c67464811 100644 --- a/clippy_lints/src/methods/bind_instead_of_map.rs +++ b/clippy_lints/src/methods/bind_instead_of_map.rs @@ -1,80 +1,66 @@ use super::{contains_return, BIND_INSTEAD_OF_MAP}; use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::{snippet, snippet_with_macro_callsite}; -use clippy_utils::ty::match_type; -use clippy_utils::{in_macro, match_qpath, method_calls, paths, remove_blocks, visitors::find_all_ret_expressions}; +use clippy_utils::{in_macro, remove_blocks, visitors::find_all_ret_expressions}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; +use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; +use rustc_hir::{LangItem, QPath}; use rustc_lint::LateContext; +use rustc_middle::ty::DefIdTree; 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 VARIANT_LANG_ITEM: LangItem = LangItem::OptionSome; 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 VARIANT_LANG_ITEM: LangItem = LangItem::ResultOk; 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 VARIANT_LANG_ITEM: LangItem = LangItem::ResultErr; 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 VARIANT_LANG_ITEM: LangItem; 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!( + fn no_op_msg(cx: &LateContext<'_>) -> Option { + let variant_id = cx.tcx.lang_items().require(Self::VARIANT_LANG_ITEM).ok()?; + let item_id = cx.tcx.parent(variant_id)?; + Some(format!( "using `{}.{}({})`, which is a no-op", - Self::TYPE_NAME, + cx.tcx.item_name(item_id), Self::BAD_METHOD_NAME, - Self::BAD_VARIANT_NAME - ) + cx.tcx.item_name(variant_id), + )) } - fn lint_msg() -> String { - format!( + fn lint_msg(cx: &LateContext<'_>) -> Option { + let variant_id = cx.tcx.lang_items().require(Self::VARIANT_LANG_ITEM).ok()?; + let item_id = cx.tcx.parent(variant_id)?; + Some(format!( "using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`", - Self::TYPE_NAME, + cx.tcx.item_name(item_id), Self::BAD_METHOD_NAME, - Self::BAD_VARIANT_NAME, + cx.tcx.item_name(variant_id), Self::GOOD_METHOD_NAME - ) + )) } fn lint_closure_autofixable( @@ -85,17 +71,12 @@ pub(crate) trait BindInsteadOfMap { 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; + if let hir::ExprKind::Call(ref some_expr, [inner_expr]) = closure_expr.kind; + if let hir::ExprKind::Path(QPath::Resolved(_, path)) = some_expr.kind; + if Self::is_variant(cx, path.res); + if !contains_return(inner_expr); + if let Some(msg) = Self::lint_msg(cx); 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 { @@ -109,7 +90,7 @@ pub(crate) trait BindInsteadOfMap { cx, BIND_INSTEAD_OF_MAP, expr.span, - Self::lint_msg().as_ref(), + &msg, "try this", note, Applicability::MachineApplicable, @@ -126,41 +107,46 @@ pub(crate) trait BindInsteadOfMap { let can_sugg: bool = 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]); + if let hir::ExprKind::Call(ref func_path, [arg]) = ret_expr.kind; + if let hir::ExprKind::Path(QPath::Resolved(_, path)) = func_path.kind; + if Self::is_variant(cx, path.res); + if !contains_return(arg); then { - suggs.push((ret_expr.span, args[0].span.source_callsite())); + suggs.push((ret_expr.span, arg.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())), - ), - ) - }); - } - can_sugg + let (span, msg) = if_chain! { + if can_sugg; + if let hir::ExprKind::MethodCall(_, span, ..) = expr.kind; + if let Some(msg) = Self::lint_msg(cx); + then { (span, msg) } else { return false; } + }; + span_lint_and_then(cx, BIND_INSTEAD_OF_MAP, expr.span, &msg, |diag| { + multispan_sugg_with_applicability( + diag, + "try this", + Applicability::MachineApplicable, + std::iter::once((span, Self::GOOD_METHOD_NAME.into())).chain( + suggs + .into_iter() + .map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())), + ), + ) + }); + true } /// 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 { - if !match_type(cx, cx.typeck_results().expr_ty(recv), Self::TYPE_QPATH) { - return false; + if_chain! { + if let Some(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def(); + if let Ok(vid) = cx.tcx.lang_items().require(Self::VARIANT_LANG_ITEM); + if Some(adt.did) == cx.tcx.parent(vid); + then {} else { return false; } } match arg.kind { @@ -175,19 +161,30 @@ pub(crate) trait BindInsteadOfMap { } }, // `_.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, recv.span, "..").into(), - Applicability::MachineApplicable, - ); + 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, + expr.span, + &msg, + "use the expression directly", + snippet(cx, recv.span, "..").into(), + Applicability::MachineApplicable, + ); + } true }, _ => false, } } + + fn is_variant(cx: &LateContext<'_>, res: Res) -> bool { + if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res { + if let Ok(variant_id) = cx.tcx.lang_items().require(Self::VARIANT_LANG_ITEM) { + return cx.tcx.parent(id) == Some(variant_id); + } + } + false + } }