X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fmethods%2Fbind_instead_of_map.rs;h=150bafc0f5db228f216127d3382c0be03e8ec353;hb=284b63a687cf9aef61ae4a1ef178ba2c4df60735;hp=0ba8a98a01851398f2558656ddb5f31cfcaba5d4;hpb=9f6b5de7deaf4dc9e7917370ad09ab85dc23997c;p=rust.git diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs index 0ba8a98a018..150bafc0f5d 100644 --- a/clippy_lints/src/methods/bind_instead_of_map.rs +++ b/clippy_lints/src/methods/bind_instead_of_map.rs @@ -1,101 +1,82 @@ 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::{peel_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( cx: &LateContext<'_>, expr: &hir::Expr<'_>, - args: &[hir::Expr<'_>], + recv: &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; + if let hir::ExprKind::Call(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 { @@ -103,13 +84,13 @@ fn lint_closure_autofixable( }; let closure_args_snip = snippet(cx, closure_args_span, ".."); - let option_snip = snippet(cx, args[0].span, ".."); + let option_snip = snippet(cx, recv.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(), + &msg, "try this", note, Applicability::MachineApplicable, @@ -125,69 +106,85 @@ fn lint_closure(cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir:: let mut suggs = Vec::new(); 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 !ret_expr.span.from_expansion(); + if let hir::ExprKind::Call(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<'_>, args: &[hir::Expr<'_>]) -> bool { - if !match_type(cx, cx.typeck_results().expr_ty(&args[0]), Self::TYPE_QPATH) { - return false; + fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool { + 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 args[1].kind { + match arg.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); + let closure_expr = peel_blocks(&closure_body.value); - if Self::lint_closure_autofixable(cx, expr, args, closure_expr, closure_args_span) { + if Self::lint_closure_autofixable(cx, expr, recv, closure_expr, closure_args_span) { true } else { 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, - ); + 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 + } }