Add is_trait_item, refactor or_fun_call and unwrap_or_else_default

Author: lf-

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

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 {

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;
 

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"),

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 `<SomeTy as Trait>::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<DefId> {
-    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() {