From e7682a499db30d86b804d1f70c465fecd62c3f14 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sat, 1 Jan 2022 16:08:32 -0500 Subject: [PATCH] Provide different suggestions for constructors. --- clippy_lints/src/derive.rs | 4 +- clippy_lints/src/drop_forget_ref.rs | 58 +++++++------------ clippy_lints/src/infinite_iter.rs | 4 +- .../src/loops/while_let_on_iterator.rs | 9 ++- clippy_lints/src/mem_forget.rs | 3 +- clippy_lints/src/mem_replace.rs | 4 +- .../src/methods/inefficient_to_string.rs | 4 +- clippy_lints/src/minmax.rs | 14 ++--- clippy_lints/src/redundant_clone.rs | 4 +- clippy_lints/src/size_of_in_element_count.rs | 5 +- clippy_lints/src/to_string_in_display.rs | 4 +- clippy_lints/src/types/borrowed_box.rs | 3 +- clippy_lints/src/useless_conversion.rs | 4 +- clippy_lints/src/utils/internal_lints.rs | 43 ++++++++++++-- clippy_utils/src/lib.rs | 44 +++++++++++--- tests/ui-internal/unnecessary_def_path.fixed | 17 +++--- tests/ui-internal/unnecessary_def_path.rs | 5 +- tests/ui-internal/unnecessary_def_path.stderr | 44 +++++++------- tests/ui/drop_forget_copy.stderr | 12 ++-- 19 files changed, 161 insertions(+), 124 deletions(-) diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 64dbd8df1cac..f19779d62b33 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_then}; use clippy_utils::paths; use clippy_utils::ty::{implements_trait, is_copy}; -use clippy_utils::{get_trait_def_id, is_automatically_derived, is_diagnostic_item, is_lint_allowed, match_def_path}; +use clippy_utils::{get_trait_def_id, is_automatically_derived, is_lint_allowed, match_def_path}; use if_chain::if_chain; use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::{ @@ -197,7 +197,7 @@ fn check_hash_peq<'tcx>( if_chain! { if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait(); if let Some(def_id) = trait_ref.trait_def_id(); - if is_diagnostic_item(cx, def_id, sym::Hash); + if cx.tcx.is_diagnostic_item(sym::Hash, def_id); then { // Look for the PartialEq implementations for `ty` cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| { diff --git a/clippy_lints/src/drop_forget_ref.rs b/clippy_lints/src/drop_forget_ref.rs index c2664a84c02e..daf43f33c87a 100644 --- a/clippy_lints/src/drop_forget_ref.rs +++ b/clippy_lints/src/drop_forget_ref.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint_and_note; -use clippy_utils::is_diagnostic_item; use clippy_utils::ty::is_copy; use if_chain::if_chain; use rustc_hir::{Expr, ExprKind}; @@ -118,49 +117,36 @@ declare_lint_pass!(DropForgetRef => [DROP_REF, FORGET_REF, DROP_COPY, FORGET_COP impl<'tcx> LateLintPass<'tcx> for DropForgetRef { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if_chain! { - if let ExprKind::Call(path, args) = expr.kind; + if let ExprKind::Call(path, [arg]) = expr.kind; if let ExprKind::Path(ref qpath) = path.kind; - if args.len() == 1; if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id(); then { - let lint; - let msg; - let arg = &args[0]; let arg_ty = cx.typeck_results().expr_ty(arg); - if let ty::Ref(..) = arg_ty.kind() { - if is_diagnostic_item(cx, def_id, sym::mem_drop) { - lint = DROP_REF; - msg = DROP_REF_SUMMARY.to_string(); - } else if is_diagnostic_item(cx, def_id, sym::mem_forget) { - lint = FORGET_REF; - msg = FORGET_REF_SUMMARY.to_string(); - } else { - return; + let (lint, msg) = if let ty::Ref(..) = arg_ty.kind() { + match cx.tcx.get_diagnostic_name(def_id) { + Some(sym::mem_drop) => (DROP_REF, DROP_REF_SUMMARY), + Some(sym::mem_forget) => (FORGET_REF, FORGET_REF_SUMMARY), + _ => return, } - span_lint_and_note(cx, - lint, - expr.span, - &msg, - Some(arg.span), - &format!("argument has type `{}`", arg_ty)); } else if is_copy(cx, arg_ty) { - if is_diagnostic_item(cx, def_id, sym::mem_drop) { - lint = DROP_COPY; - msg = DROP_COPY_SUMMARY.to_string(); - } else if is_diagnostic_item(cx, def_id, sym::mem_forget) { - lint = FORGET_COPY; - msg = FORGET_COPY_SUMMARY.to_string(); - } else { - return; + match cx.tcx.get_diagnostic_name(def_id) { + Some(sym::mem_drop) => (DROP_COPY, DROP_COPY_SUMMARY), + Some(sym::mem_forget) => (FORGET_COPY, FORGET_COPY_SUMMARY), + _ => return, } - span_lint_and_note(cx, - lint, - expr.span, - &msg, - Some(arg.span), - &format!("argument has type {}", arg_ty)); - } + } else { + return; + }; + + span_lint_and_note( + cx, + lint, + expr.span, + msg, + Some(arg.span), + &format!("argument has type `{}`", arg_ty) + ); } } } diff --git a/clippy_lints/src/infinite_iter.rs b/clippy_lints/src/infinite_iter.rs index b5d080564594..da5e72b8b4d3 100644 --- a/clippy_lints/src/infinite_iter.rs +++ b/clippy_lints/src/infinite_iter.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; -use clippy_utils::{get_trait_def_id, higher, is_diagnostic_item, paths}; +use clippy_utils::{get_trait_def_id, higher, paths}; use rustc_hir::{BorrowKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -171,7 +171,7 @@ fn is_infinite(cx: &LateContext<'_>, expr: &Expr<'_>) -> Finiteness { if let ExprKind::Path(ref qpath) = path.kind { cx.qpath_res(qpath, path.hir_id) .opt_def_id() - .map_or(false, |id| is_diagnostic_item(cx, id, sym::iter_repeat)) + .map_or(false, |id| cx.tcx.is_diagnostic_item(sym::iter_repeat, id)) .into() } else { Finite diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index 29a836c42423..53765e0c9c93 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -2,11 +2,11 @@ use super::WHILE_LET_ON_ITERATOR; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::higher; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{get_enclosing_loop_or_closure, is_lang_item, is_refutable, is_trait_method, visitors::is_res_used}; +use clippy_utils::{get_enclosing_loop_or_closure, is_lang_ctor, is_refutable, is_trait_method, visitors::is_res_used}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; -use rustc_hir::{def::Res, Expr, ExprKind, HirId, LangItem, Local, PatKind, QPath, UnOp}; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, LangItem, Local, PatKind, UnOp}; use rustc_lint::LateContext; use rustc_span::{symbol::sym, Span, Symbol}; @@ -14,9 +14,8 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { let (scrutinee_expr, iter_expr, some_pat, loop_expr) = if_chain! { if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr); // check for `Some(..)` pattern - if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = let_pat.kind; - if let Res::Def(_, pat_did) = pat_path.res; - if is_lang_item(cx, pat_did, LangItem::OptionSome); + if let PatKind::TupleStruct(ref pat_path, some_pat, _) = let_pat.kind; + if is_lang_ctor(cx, pat_path, LangItem::OptionSome); // check for call to `Iterator::next` if let ExprKind::MethodCall(method_name, _, [iter_expr], _) = let_expr.kind; if method_name.ident.name == sym::next; diff --git a/clippy_lints/src/mem_forget.rs b/clippy_lints/src/mem_forget.rs index bdcba661d85c..d6c235b5a693 100644 --- a/clippy_lints/src/mem_forget.rs +++ b/clippy_lints/src/mem_forget.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::is_diagnostic_item; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -33,7 +32,7 @@ impl<'tcx> LateLintPass<'tcx> for MemForget { if let ExprKind::Call(path_expr, [ref first_arg, ..]) = e.kind { if let ExprKind::Path(ref qpath) = path_expr.kind { if let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id() { - if is_diagnostic_item(cx, def_id, sym::mem_forget) { + if cx.tcx.is_diagnostic_item(sym::mem_forget, def_id) { let forgot_ty = cx.typeck_results().expr_ty(first_arg); if forgot_ty.ty_adt_def().map_or(false, |def| def.has_dtor(cx.tcx)) { diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs index 3ae037bd2a71..f843bcd616d5 100644 --- a/clippy_lints/src/mem_replace.rs +++ b/clippy_lints/src/mem_replace.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::ty::is_non_aggregate_primitive_type; -use clippy_utils::{is_default_equivalent, is_diagnostic_item, is_lang_ctor, meets_msrv, msrvs}; +use clippy_utils::{is_default_equivalent, is_lang_ctor, meets_msrv, msrvs}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::LangItem::OptionNone; @@ -249,7 +249,7 @@ impl<'tcx> LateLintPass<'tcx> for MemReplace { if let ExprKind::Call(func, func_args) = expr.kind; if let ExprKind::Path(ref func_qpath) = func.kind; if let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id(); - if is_diagnostic_item(cx, def_id, sym::mem_replace); + if cx.tcx.is_diagnostic_item(sym::mem_replace, def_id); if let [dest, src] = func_args; then { check_replace_option_with_none(cx, src, dest, expr.span); diff --git a/clippy_lints/src/methods/inefficient_to_string.rs b/clippy_lints/src/methods/inefficient_to_string.rs index 8fec19f5fedd..799f59e4c17c 100644 --- a/clippy_lints/src/methods/inefficient_to_string.rs +++ b/clippy_lints/src/methods/inefficient_to_string.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::{is_type_diagnostic_item, walk_ptrs_ty_depth}; -use clippy_utils::{is_diagnostic_item, match_def_path, paths}; +use clippy_utils::{match_def_path, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; @@ -60,7 +60,7 @@ fn specializes_tostring(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { } if let ty::Adt(adt, substs) = ty.kind() { - is_diagnostic_item(cx, adt.did, sym::Cow) && substs.type_at(1).is_str() + cx.tcx.is_diagnostic_item(sym::Cow, adt.did) && substs.type_at(1).is_str() } else { false } diff --git a/clippy_lints/src/minmax.rs b/clippy_lints/src/minmax.rs index 411a44fc3d97..eef30a4547ae 100644 --- a/clippy_lints/src/minmax.rs +++ b/clippy_lints/src/minmax.rs @@ -1,6 +1,6 @@ use clippy_utils::consts::{constant_simple, Constant}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::{is_diagnostic_item, is_trait_method}; +use clippy_utils::is_trait_method; use if_chain::if_chain; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -74,14 +74,10 @@ fn min_max<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(MinMax, Cons cx.typeck_results() .qpath_res(qpath, path.hir_id) .opt_def_id() - .and_then(|def_id| { - if is_diagnostic_item(cx, def_id, sym::cmp_min) { - fetch_const(cx, args, MinMax::Min) - } else if is_diagnostic_item(cx, def_id, sym::cmp_max) { - fetch_const(cx, args, MinMax::Max) - } else { - None - } + .and_then(|def_id| match cx.tcx.get_diagnostic_name(def_id) { + Some(sym::cmp_min) => fetch_const(cx, args, MinMax::Min), + Some(sym::cmp_max) => fetch_const(cx, args, MinMax::Max), + _ => None, }) } else { None diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index 367c15b8094c..e2963b8babc0 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{has_drop, is_copy, is_type_diagnostic_item, walk_ptrs_ty_depth}; -use clippy_utils::{fn_has_unsatisfiable_preds, is_lang_item, match_def_path, paths}; +use clippy_utils::{fn_has_unsatisfiable_preds, match_def_path, paths}; use if_chain::if_chain; use rustc_data_structures::{fx::FxHashMap, transitive_relation::TransitiveRelation}; use rustc_errors::Applicability; @@ -135,7 +135,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone { } if let ty::Adt(def, _) = arg_ty.kind() { - if is_lang_item(cx, def.did, LangItem::ManuallyDrop) { + if cx.tcx.lang_items().require(LangItem::ManuallyDrop).ok() == Some(def.did) { continue; } } diff --git a/clippy_lints/src/size_of_in_element_count.rs b/clippy_lints/src/size_of_in_element_count.rs index 8ee71e040ce9..8cd09decf419 100644 --- a/clippy_lints/src/size_of_in_element_count.rs +++ b/clippy_lints/src/size_of_in_element_count.rs @@ -2,7 +2,7 @@ //! expecting a count of T use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::{is_diagnostic_item, match_def_path, paths}; +use clippy_utils::{match_def_path, paths}; use if_chain::if_chain; use rustc_hir::BinOpKind; use rustc_hir::{Expr, ExprKind}; @@ -45,8 +45,7 @@ fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, inverted: bool) if !inverted; if let ExprKind::Path(ref count_func_qpath) = count_func.kind; if let Some(def_id) = cx.qpath_res(count_func_qpath, count_func.hir_id).opt_def_id(); - if is_diagnostic_item(cx, def_id, sym::mem_size_of) - || is_diagnostic_item(cx, def_id, sym::mem_size_of_val); + if matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::mem_size_of | sym::mem_size_of_val)); then { cx.typeck_results().node_substs(count_func.hir_id).types().next() } else { diff --git a/clippy_lints/src/to_string_in_display.rs b/clippy_lints/src/to_string_in_display.rs index d220b3f65309..b376d1eba4d0 100644 --- a/clippy_lints/src/to_string_in_display.rs +++ b/clippy_lints/src/to_string_in_display.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::{is_diag_trait_item, is_diagnostic_item, path_to_local_id}; +use clippy_utils::{is_diag_trait_item, path_to_local_id}; use if_chain::if_chain; use rustc_hir::{Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -115,7 +115,7 @@ fn is_display_impl(cx: &LateContext<'_>, item: &Item<'_>) -> bool { if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), .. }) = &item.kind; if let Some(did) = trait_ref.trait_def_id(); then { - is_diagnostic_item(cx, did, sym::Display) + cx.tcx.is_diagnostic_item(sym::Display, did) } else { false } diff --git a/clippy_lints/src/types/borrowed_box.rs b/clippy_lints/src/types/borrowed_box.rs index a4b7b00b49f9..79306c77e862 100644 --- a/clippy_lints/src/types/borrowed_box.rs +++ b/clippy_lints/src/types/borrowed_box.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_diagnostic_item; use clippy_utils::source::snippet; use if_chain::if_chain; @@ -91,7 +90,7 @@ fn is_any_trait(cx: &LateContext<'_>, t: &hir::Ty<'_>) -> bool { if let Some(trait_did) = traits[0].trait_ref.trait_def_id(); // Only Send/Sync can be used as additional traits, so it is enough to // check only the first trait. - if is_diagnostic_item(cx, trait_did, sym::Any); + if cx.tcx.is_diagnostic_item(sym::Any, trait_did); then { return true; } diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 215977e98776..25011287319e 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; use clippy_utils::source::{snippet, snippet_with_macro_callsite}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts}; -use clippy_utils::{get_parent_expr, is_lang_item, is_trait_method, match_def_path, paths}; +use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, HirId, LangItem, MatchSource}; @@ -154,7 +154,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { } if_chain! { - if is_lang_item(cx, def_id, LangItem::FromFrom); + if cx.tcx.lang_items().require(LangItem::FromFrom).ok() == Some(def_id); if same_type_and_consts(a, b); then { diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 1db5ce609bf2..a7313169b460 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -23,7 +23,7 @@ use rustc_hir::{ use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; use rustc_middle::mir::interpret::{Allocation, ConstValue, GlobalAlloc}; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, DefIdTree, Ty}; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Spanned; @@ -897,15 +897,32 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath { return; }; + let has_ctor = match cx.tcx.def_kind(def_id) { + DefKind::Struct => { + let variant = cx.tcx.adt_def(def_id).non_enum_variant(); + variant.ctor_def_id.is_some() && variant.fields.iter().all(|f| f.vis.is_public()) + } + DefKind::Variant => + cx.tcx.parent(def_id).map_or(false, |adt_id| { + let variant = cx.tcx.adt_def(adt_id).variant_with_id(def_id); + variant.ctor_def_id.is_some() && variant.fields.iter().all(|f| f.vis.is_public()) + }), + _ => false, + }; + let mut app = Applicability::MachineApplicable; let cx_snip = snippet_with_applicability(cx, cx_arg.span, "..", &mut app); let def_snip = snippet_with_applicability(cx, def_arg.span, "..", &mut app); let sugg = match (which_path, item) { // match_def_path + (0, Item::DiagnosticItem(item)) if has_ctor => + format!("is_diagnostic_item_or_ctor({}, {}, sym::{})", cx_snip, def_snip, item), + (0, Item::LangItem(item)) if has_ctor => + format!("is_lang_item_or_ctor({}, {}, LangItem::{})", cx_snip, def_snip, item), (0, Item::DiagnosticItem(item)) => - format!("is_diagnostic_item({}, {}, sym::{})", cx_snip, def_snip, item), + format!("{}.tcx.is_diagnostic_item(sym::{}, {})", cx_snip, item, def_snip), (0, Item::LangItem(item)) => - format!("is_lang_item({}, {}, LangItem::{})", cx_snip, def_snip, item), + format!("{}.tcx.lang_items().require(LangItem::{}).ok() == Some({})", cx_snip, item, def_snip), // match_trait_method (1, Item::DiagnosticItem(item)) => format!("is_trait_method({}, {}, sym::{})", cx_snip, def_snip, item), @@ -915,20 +932,34 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath { (2, Item::LangItem(item)) => format!("is_type_lang_item({}, {}, LangItem::{})", cx_snip, def_snip, item), // is_expr_path_def_path + (3, Item::DiagnosticItem(item)) if has_ctor => + format!( + "is_res_diagnostic_item_ctor({}, expr_path_res({}, {}), sym::{})", + cx_snip, cx_snip, def_snip, item, + ), + (3, Item::LangItem(item)) if has_ctor => + format!( + "is_res_lang_item_ctor({}, expr_path_res({}, {}), LangItem::{})", + cx_snip, cx_snip, def_snip, item, + ), (3, Item::DiagnosticItem(item)) => format!("is_expr_diagnostic_item({}, {}, sym::{})", cx_snip, def_snip, item), (3, Item::LangItem(item)) => format!( "expr_path_res({}, {}).opt_def_id()\ - .map_or(false, |id| is_lang_item({}, id, LangItem::{}))", + .map_or(false, |id| {}.tcx.lang_items().require(LangItem::{}).ok() == Some(id))", cx_snip, def_snip, cx_snip, item, ), // is_qpath_def_path + (4, Item::DiagnosticItem(item)) if has_ctor => + format!("is_diagnostic_item_ctor({}, {}, sym::{})", cx_snip, def_snip, item), + (4, Item::LangItem(item)) if has_ctor => + format!("is_lang_ctor({}, {}, LangItem::{})", cx_snip, def_snip, item), (4, Item::DiagnosticItem(item)) => { let id_snip = snippet_with_applicability(cx, args[0].span, "..", &mut app); format!( "{}.qpath_res({}, {}).opt_def_id()\ - .map_or(false, |id| is_diagnostic_item({}, id, sym::{}))", + .map_or(false, |id| {}.tcx.is_diagnostic_item(sym::{}, id))", cx_snip, def_snip, id_snip, cx_snip, item, ) } @@ -936,7 +967,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath { let id_snip = snippet_with_applicability(cx, args[0].span, "..", &mut app); format!( "{}.qpath_res({}, {}).opt_def_id()\ - .map_or(false, |id| is_lang_item({}, id, LangItem::{}))", + .map_or(false, |id| {}.tcx.lang_items().require(LangItem::{}).ok() == Some(id))", cx_snip, def_snip, id_snip, cx_snip, item, ) } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 50bfda597ed6..0a53d3b2264a 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -254,17 +254,43 @@ pub fn in_constant(cx: &LateContext<'_>, id: HirId) -> bool { /// For example, use this to check whether a function call or a pattern is `Some(..)`. pub fn is_lang_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, lang_item: LangItem) -> bool { if let QPath::Resolved(_, path) = qpath { - if let Res::Def(DefKind::Ctor(..), ctor_id) = path.res { - if let Ok(item_id) = cx.tcx.lang_items().require(lang_item) { - return cx.tcx.parent(ctor_id) == Some(item_id); - } + is_res_lang_item_ctor(cx, path.res, lang_item) + } else { + false + } +} + +/// Checks if a `QPath` resolves to a constructor of a diagnostic item. +pub fn is_diagnostic_item_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, diagnostic_item: Symbol) -> bool { + if let QPath::Resolved(_, path) = qpath { + is_res_diagnostic_item_ctor(cx, path.res, diagnostic_item) + } else { + false + } +} + +/// Checks if a resolved path is a constructor of a `LangItem` +pub fn is_res_lang_item_ctor(cx: &LateContext<'_>, res: Res, lang_item: LangItem) -> bool { + if let Res::Def(DefKind::Ctor(..), ctor_id) = res { + if let Ok(item_id) = cx.tcx.lang_items().require(lang_item) { + return cx.tcx.parent(ctor_id) == Some(item_id); + } + } + false +} + +/// Checks if a resolved path is a constructor of a diagnostic item. +pub fn is_res_diagnostic_item_ctor(cx: &LateContext<'_>, res: Res, diagnostic_item: Symbol) -> bool { + if let Res::Def(DefKind::Ctor(..), ctor_id) = res { + if let Some(variant_id) = cx.tcx.parent(ctor_id) { + return cx.tcx.is_diagnostic_item(diagnostic_item, variant_id); } } false } -/// Checks if the `DefId` matches the given diagnostic item. -pub fn is_diagnostic_item(cx: &LateContext<'_>, did: DefId, item: Symbol) -> bool { +/// Checks if the `DefId` matches the given diagnostic item or it's constructor. +pub fn is_diagnostic_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: Symbol) -> bool { let did = match cx.tcx.def_kind(did) { DefKind::Ctor(..) => match cx.tcx.parent(did) { Some(did) => did, @@ -281,8 +307,8 @@ pub fn is_diagnostic_item(cx: &LateContext<'_>, did: DefId, item: Symbol) -> boo cx.tcx.is_diagnostic_item(item, did) } -/// Checks if the `DefId` matches the given `LangItem`. -pub fn is_lang_item(cx: &LateContext<'_>, did: DefId, item: LangItem) -> bool { +/// Checks if the `DefId` matches the given `LangItem` or it's constructor. +pub fn is_lang_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: LangItem) -> bool { let did = match cx.tcx.def_kind(did) { DefKind::Ctor(..) => match cx.tcx.parent(did) { Some(did) => did, @@ -502,7 +528,7 @@ pub fn is_expr_path_def_path(cx: &LateContext<'_>, expr: &Expr<'_>, segments: &[ pub fn is_expr_diagnostic_item(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) -> bool { expr_path_res(cx, expr) .opt_def_id() - .map_or(false, |id| is_diagnostic_item(cx, id, diag_item)) + .map_or(false, |id| is_diagnostic_item_or_ctor(cx, id, diag_item)) } /// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the diff --git a/tests/ui-internal/unnecessary_def_path.fixed b/tests/ui-internal/unnecessary_def_path.fixed index 52ff927381b3..811cec6705bf 100644 --- a/tests/ui-internal/unnecessary_def_path.fixed +++ b/tests/ui-internal/unnecessary_def_path.fixed @@ -14,8 +14,9 @@ extern crate rustc_span; use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type}; #[allow(unused)] use clippy_utils::{ - expr_path_res, is_diagnostic_item, is_expr_diagnostic_item, is_expr_path_def_path, is_lang_ctor, is_lang_item, - is_qpath_def_path, is_trait_method, match_def_path, match_trait_method, + expr_path_res, is_diagnostic_item_ctor, is_diagnostic_item_or_ctor, is_expr_diagnostic_item, is_expr_path_def_path, + is_lang_ctor, is_lang_item_or_ctor, is_qpath_def_path, is_res_diagnostic_item_ctor, is_res_lang_item_ctor, + is_trait_method, match_def_path, match_trait_method, }; #[allow(unused)] @@ -48,17 +49,17 @@ fn _f<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, did: DefId, expr: &Expr<'_>, q let _ = is_type_lang_item(cx, ty, LangItem::OwnedBox); let _ = is_type_diagnostic_item(cx, ty, sym::maybe_uninit_uninit); - let _ = is_lang_item(cx, did, LangItem::OwnedBox); - let _ = is_diagnostic_item(cx, did, sym::Option); - let _ = is_lang_item(cx, did, LangItem::OptionSome); + let _ = cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(did); + let _ = cx.tcx.is_diagnostic_item(sym::Option, did); + let _ = is_lang_item_or_ctor(cx, did, LangItem::OptionSome); let _ = is_trait_method(cx, expr, sym::AsRef); let _ = is_expr_diagnostic_item(cx, expr, sym::Option); - let _ = expr_path_res(cx, expr).opt_def_id().map_or(false, |id| is_lang_item(cx, id, LangItem::IteratorNext)); + let _ = expr_path_res(cx, expr).opt_def_id().map_or(false, |id| cx.tcx.lang_items().require(LangItem::IteratorNext).ok() == Some(id)); - let _ = cx.qpath_res(qpath, hid).opt_def_id().map_or(false, |id| is_diagnostic_item(cx, id, sym::Option)); - let _ = cx.qpath_res(qpath, hid).opt_def_id().map_or(false, |id| is_lang_item(cx, id, LangItem::OwnedBox)); + let _ = cx.qpath_res(qpath, hid).opt_def_id().map_or(false, |id| cx.tcx.is_diagnostic_item(sym::Option, id)); + let _ = cx.qpath_res(qpath, hid).opt_def_id().map_or(false, |id| cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(id)); } fn main() {} diff --git a/tests/ui-internal/unnecessary_def_path.rs b/tests/ui-internal/unnecessary_def_path.rs index 55f6c6dc59cf..40f6ec00eb2a 100644 --- a/tests/ui-internal/unnecessary_def_path.rs +++ b/tests/ui-internal/unnecessary_def_path.rs @@ -14,8 +14,9 @@ extern crate rustc_span; use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type}; #[allow(unused)] use clippy_utils::{ - expr_path_res, is_diagnostic_item, is_expr_diagnostic_item, is_expr_path_def_path, is_lang_ctor, is_lang_item, - is_qpath_def_path, is_trait_method, match_def_path, match_trait_method, + expr_path_res, is_diagnostic_item_ctor, is_diagnostic_item_or_ctor, is_expr_diagnostic_item, is_expr_path_def_path, + is_lang_ctor, is_lang_item_or_ctor, is_qpath_def_path, is_res_diagnostic_item_ctor, is_res_lang_item_ctor, + is_trait_method, match_def_path, match_trait_method, }; #[allow(unused)] diff --git a/tests/ui-internal/unnecessary_def_path.stderr b/tests/ui-internal/unnecessary_def_path.stderr index c84262476e17..c0c042f2bd41 100644 --- a/tests/ui-internal/unnecessary_def_path.stderr +++ b/tests/ui-internal/unnecessary_def_path.stderr @@ -1,5 +1,5 @@ error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:37:13 + --> $DIR/unnecessary_def_path.rs:38:13 | LL | let _ = match_type(cx, ty, &OPTION); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Option)` @@ -12,94 +12,94 @@ LL | #![deny(clippy::internal)] = note: `#[deny(clippy::unnecessary_def_path)]` implied by `#[deny(clippy::internal)]` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:38:13 + --> $DIR/unnecessary_def_path.rs:39:13 | LL | let _ = match_type(cx, ty, RESULT); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Result)` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:39:13 + --> $DIR/unnecessary_def_path.rs:40:13 | LL | let _ = match_type(cx, ty, &["core", "result", "Result"]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Result)` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:43:13 + --> $DIR/unnecessary_def_path.rs:44:13 | LL | let _ = clippy_utils::ty::match_type(cx, ty, rc_path); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Rc)` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:45:13 + --> $DIR/unnecessary_def_path.rs:46:13 | LL | let _ = match_type(cx, ty, &paths::OPTION); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Option)` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:46:13 + --> $DIR/unnecessary_def_path.rs:47:13 | LL | let _ = match_type(cx, ty, paths::RESULT); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Result)` error: use of a def path to a `LangItem` - --> $DIR/unnecessary_def_path.rs:48:13 + --> $DIR/unnecessary_def_path.rs:49:13 | LL | let _ = match_type(cx, ty, &["alloc", "boxed", "Box"]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_lang_item(cx, ty, LangItem::OwnedBox)` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:49:13 + --> $DIR/unnecessary_def_path.rs:50:13 | LL | let _ = match_type(cx, ty, &["core", "mem", "maybe_uninit", "MaybeUninit", "uninit"]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::maybe_uninit_uninit)` error: use of a def path to a `LangItem` - --> $DIR/unnecessary_def_path.rs:51:13 + --> $DIR/unnecessary_def_path.rs:52:13 | LL | let _ = match_def_path(cx, did, &["alloc", "boxed", "Box"]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_lang_item(cx, did, LangItem::OwnedBox)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(did)` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:52:13 + --> $DIR/unnecessary_def_path.rs:53:13 | LL | let _ = match_def_path(cx, did, &["core", "option", "Option"]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_diagnostic_item(cx, did, sym::Option)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.is_diagnostic_item(sym::Option, did)` error: use of a def path to a `LangItem` - --> $DIR/unnecessary_def_path.rs:53:13 + --> $DIR/unnecessary_def_path.rs:54:13 | LL | let _ = match_def_path(cx, did, &["core", "option", "Option", "Some"]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_lang_item(cx, did, LangItem::OptionSome)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_lang_item_or_ctor(cx, did, LangItem::OptionSome)` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:55:13 + --> $DIR/unnecessary_def_path.rs:56:13 | LL | let _ = match_trait_method(cx, expr, &["core", "convert", "AsRef"]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_trait_method(cx, expr, sym::AsRef)` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:57:13 + --> $DIR/unnecessary_def_path.rs:58:13 | LL | let _ = is_expr_path_def_path(cx, expr, &["core", "option", "Option"]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_expr_diagnostic_item(cx, expr, sym::Option)` error: use of a def path to a `LangItem` - --> $DIR/unnecessary_def_path.rs:58:13 + --> $DIR/unnecessary_def_path.rs:59:13 | LL | let _ = is_expr_path_def_path(cx, expr, &["core", "iter", "traits", "Iterator", "next"]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `expr_path_res(cx, expr).opt_def_id().map_or(false, |id| is_lang_item(cx, id, LangItem::IteratorNext))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `expr_path_res(cx, expr).opt_def_id().map_or(false, |id| cx.tcx.lang_items().require(LangItem::IteratorNext).ok() == Some(id))` error: use of a def path to a diagnostic item - --> $DIR/unnecessary_def_path.rs:60:13 + --> $DIR/unnecessary_def_path.rs:61:13 | LL | let _ = is_qpath_def_path(cx, qpath, hid, &["core", "option", "Option"]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.qpath_res(qpath, hid).opt_def_id().map_or(false, |id| is_diagnostic_item(cx, id, sym::Option))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.qpath_res(qpath, hid).opt_def_id().map_or(false, |id| cx.tcx.is_diagnostic_item(sym::Option, id))` error: use of a def path to a `LangItem` - --> $DIR/unnecessary_def_path.rs:61:13 + --> $DIR/unnecessary_def_path.rs:62:13 | LL | let _ = is_qpath_def_path(cx, qpath, hid, &["alloc", "boxed", "Box"]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.qpath_res(qpath, hid).opt_def_id().map_or(false, |id| is_lang_item(cx, id, LangItem::OwnedBox))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.qpath_res(qpath, hid).opt_def_id().map_or(false, |id| cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(id))` error: aborting due to 16 previous errors diff --git a/tests/ui/drop_forget_copy.stderr b/tests/ui/drop_forget_copy.stderr index 01de0be7caea..88228afae89c 100644 --- a/tests/ui/drop_forget_copy.stderr +++ b/tests/ui/drop_forget_copy.stderr @@ -5,7 +5,7 @@ LL | drop(s1); | ^^^^^^^^ | = note: `-D clippy::drop-copy` implied by `-D warnings` -note: argument has type SomeStruct +note: argument has type `SomeStruct` --> $DIR/drop_forget_copy.rs:33:10 | LL | drop(s1); @@ -17,7 +17,7 @@ error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a LL | drop(s2); | ^^^^^^^^ | -note: argument has type SomeStruct +note: argument has type `SomeStruct` --> $DIR/drop_forget_copy.rs:34:10 | LL | drop(s2); @@ -29,7 +29,7 @@ error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a LL | drop(s4); | ^^^^^^^^ | -note: argument has type SomeStruct +note: argument has type `SomeStruct` --> $DIR/drop_forget_copy.rs:36:10 | LL | drop(s4); @@ -42,7 +42,7 @@ LL | forget(s1); | ^^^^^^^^^^ | = note: `-D clippy::forget-copy` implied by `-D warnings` -note: argument has type SomeStruct +note: argument has type `SomeStruct` --> $DIR/drop_forget_copy.rs:39:12 | LL | forget(s1); @@ -54,7 +54,7 @@ error: calls to `std::mem::forget` with a value that implements `Copy`. Forgetti LL | forget(s2); | ^^^^^^^^^^ | -note: argument has type SomeStruct +note: argument has type `SomeStruct` --> $DIR/drop_forget_copy.rs:40:12 | LL | forget(s2); @@ -66,7 +66,7 @@ error: calls to `std::mem::forget` with a value that implements `Copy`. Forgetti LL | forget(s4); | ^^^^^^^^^^ | -note: argument has type SomeStruct +note: argument has type `SomeStruct` --> $DIR/drop_forget_copy.rs:42:12 | LL | forget(s4);