From c78cc7ac1f1e89cd2e12d29374e1c5e6110d9217 Mon Sep 17 00:00:00 2001 From: Jade Date: Thu, 5 Aug 2021 20:08:52 -0700 Subject: [PATCH] Add is_trait_item, refactor or_fun_call and unwrap_or_else_default --- clippy_lints/src/methods/mod.rs | 8 ++++---- clippy_lints/src/methods/or_fun_call.rs | 9 ++++----- .../src/methods/unwrap_or_else_default.rs | 9 ++------- clippy_utils/src/lib.rs | 19 +++++++++++++++++++ clippy_utils/src/ty.rs | 16 ---------------- 5 files changed, 29 insertions(+), 32 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 12f7987fd3a..bf74cad039e 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -313,12 +313,12 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for usages of `_.unwrap_or_else(Default::default)` on Option and - /// Result values. + /// Checks for usages of `_.unwrap_or_else(Default::default)` on `Option` and + /// `Result` values. /// /// ### Why is this bad? - /// Readability, these can be written as `option.unwrap_or_default` or - /// `result.unwrap_or_default`. + /// Readability, these can be written as `_.unwrap_or_default`, which is + /// simpler and more concise. /// /// ### Examples /// ```rust diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 378b0724170..c1d22e5d72c 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -1,7 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::eager_or_lazy::is_lazyness_candidate; +use clippy_utils::is_trait_item; use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite}; -use clippy_utils::ty::{implements_trait, qpath_target_trait}; +use clippy_utils::ty::implements_trait; use clippy_utils::ty::{is_type_diagnostic_item, match_type}; use clippy_utils::{contains_return, last_path_segment, paths}; use if_chain::if_chain; @@ -35,9 +36,7 @@ pub(super) fn check<'tcx>( or_has_args: bool, span: Span, ) -> bool { - let is_default_default = |qpath, default_trait_id| { - qpath_target_trait(cx, qpath, fun.hir_id).map_or(false, |target_trait| target_trait == default_trait_id) - }; + let is_default_default = || is_trait_item(cx, fun, sym::Default); let implements_default = |arg, default_trait_id| { let arg_ty = cx.typeck_results().expr_ty(arg); @@ -52,7 +51,7 @@ pub(super) fn check<'tcx>( let path = last_path_segment(qpath).ident.name; // needs to target Default::default in particular or be *::new and have a Default impl // available - if (matches!(path, kw::Default) && is_default_default(qpath, default_trait_id)) + if (matches!(path, kw::Default) && is_default_default()) || (matches!(path, sym::new) && implements_default(arg, default_trait_id)); then { diff --git a/clippy_lints/src/methods/unwrap_or_else_default.rs b/clippy_lints/src/methods/unwrap_or_else_default.rs index f99ae6cae93..677aa80e1b7 100644 --- a/clippy_lints/src/methods/unwrap_or_else_default.rs +++ b/clippy_lints/src/methods/unwrap_or_else_default.rs @@ -2,9 +2,7 @@ use super::UNWRAP_OR_ELSE_DEFAULT; use clippy_utils::{ - diagnostics::span_lint_and_sugg, - source::snippet_with_applicability, - ty::{is_type_diagnostic_item, qpath_target_trait}, + diagnostics::span_lint_and_sugg, is_trait_item, source::snippet_with_applicability, ty::is_type_diagnostic_item, }; use rustc_errors::Applicability; use rustc_hir as hir; @@ -26,10 +24,7 @@ pub(super) fn check<'tcx>( if_chain! { if is_option || is_result; - if let hir::ExprKind::Path(ref qpath) = u_arg.kind; - if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default); - if let Some(target_trait) = qpath_target_trait(cx, qpath, u_arg.hir_id); - if target_trait == default_trait_id; + if is_trait_item(cx, u_arg, sym::Default); then { let mut applicability = Applicability::MachineApplicable; diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 59f878f8b20..da9560f8ccf 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -326,6 +326,25 @@ pub fn is_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) .map_or(false, |did| is_diag_trait_item(cx, did, diag_item)) } +/// Checks if the given expression is a path referring an item on the trait +/// that is marked with the given diagnostic item. +/// +/// For checking method call expressions instead of path expressions, use +/// [`is_trait_method`]. +/// +/// For example, to find if an expression like `u64::default` refers to an item +/// of the trait `Default`, which is marked `#[rustc_diagnostic_item = "Default"]`, +/// a `diag_item` of `sym::Default` should be used. +pub fn is_trait_item(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) -> bool { + if let hir::ExprKind::Path(ref qpath) = expr.kind { + cx.qpath_res(qpath, expr.hir_id) + .opt_def_id() + .map_or(false, |def_id| is_diag_trait_item(cx, def_id, diag_item)) + } else { + false + } +} + pub fn last_path_segment<'tcx>(path: &QPath<'tcx>) -> &'tcx PathSegment<'tcx> { match *path { QPath::Resolved(_, path) => path.segments.last().expect("A path must have at least one segment"), diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index bc31bea8b9f..536d0f006e3 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -2,7 +2,6 @@ #![allow(clippy::module_name_repetitions)] -use hir::{HirId, QPath}; use rustc_ast::ast::Mutability; use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; @@ -137,21 +136,6 @@ pub fn implements_trait<'tcx>( }) } -/// Gets the trait that a path targets. For example `::a` would return the -/// [`DefId`] for `Trait`. -/// -/// `cx` must be in a body. -pub fn qpath_target_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, expr_id: HirId) -> Option { - let method_res = cx.typeck_results().qpath_res(qpath, expr_id); - let method_id = match method_res { - hir::def::Res::Def(_kind, id) => Some(id), - _ => None, - }; - let method_id = method_id?; - - cx.tcx.trait_of_item(method_id) -} - /// Checks whether this type implements `Drop`. pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { match ty.ty_adt_def() {