From 11ef04728c90aa6105b20c681e3c12f231ba4f12 Mon Sep 17 00:00:00 2001 From: Jade Date: Thu, 29 Jul 2021 23:56:47 -0700 Subject: Add unwrap_or_else_default lint This will catch `unwrap_or_else(Default::default)` on Result and Option and suggest `unwrap_or_default()` instead. --- clippy_utils/src/source.rs | 2 +- clippy_utils/src/ty.rs | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) (limited to 'clippy_utils') diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 4d49b43bde9..789079510c5 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -168,7 +168,7 @@ pub fn snippet<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow< snippet_opt(cx, span).map_or_else(|| Cow::Borrowed(default), From::from) } -/// Same as `snippet`, but it adapts the applicability level by following rules: +/// Same as [`snippet`], but it adapts the applicability level by following rules: /// /// - Applicability level `Unspecified` will never be changed. /// - If the span is inside a macro, change the applicability level to `MaybeIncorrect`. diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index e914dc1c222..bc31bea8b9f 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -2,6 +2,7 @@ #![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; @@ -114,7 +115,7 @@ pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option< /// Checks whether a type implements a trait. /// The function returns false in case the type contains an inference variable. -/// See also `get_trait_def_id`. +/// See also [`get_trait_def_id`]. pub fn implements_trait<'tcx>( cx: &LateContext<'tcx>, ty: Ty<'tcx>, @@ -136,6 +137,21 @@ 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() { -- cgit 1.4.1-3-g733a5 From c78cc7ac1f1e89cd2e12d29374e1c5e6110d9217 Mon Sep 17 00:00:00 2001 From: Jade Date: Thu, 5 Aug 2021 20:08:52 -0700 Subject: 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 ++++----- clippy_lints/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(-) (limited to 'clippy_utils') 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() { -- cgit 1.4.1-3-g733a5 From 23d398b1848571709a0cbb8a08436f6de56d43a8 Mon Sep 17 00:00:00 2001 From: Jade Date: Thu, 5 Aug 2021 19:46:26 -0700 Subject: tree-wide: Fix all the rustdoc warnings --- clippy_lints/src/needless_continue.rs | 2 +- clippy_lints/src/needless_for_each.rs | 2 +- clippy_utils/src/diagnostics.rs | 6 +++--- clippy_utils/src/higher.rs | 6 +++--- clippy_utils/src/ty.rs | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) (limited to 'clippy_utils') diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index 5088b8bb0d3..5a50cc48d61 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -422,7 +422,7 @@ fn check_and_warn<'a>(cx: &EarlyContext<'_>, expr: &'a ast::Expr) { /// /// is transformed to /// -/// ```ignore +/// ```text /// { /// let x = 5; /// ``` diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs index d9aa42fe8ee..9a6ddc72ce5 100644 --- a/clippy_lints/src/needless_for_each.rs +++ b/clippy_lints/src/needless_for_each.rs @@ -122,7 +122,7 @@ impl LateLintPass<'_> for NeedlessForEach { /// 2. Detect use of `return` in `Loop` in the closure body. /// /// NOTE: The functionality of this type is similar to -/// [`crate::utilts::visitors::find_all_ret_expressions`], but we can't use +/// [`clippy_utils::visitors::find_all_ret_expressions`], but we can't use /// `find_all_ret_expressions` instead of this type. The reasons are: /// 1. `find_all_ret_expressions` passes the argument of `ExprKind::Ret` to a callback, but what we /// need here is `ExprKind::Ret` itself. diff --git a/clippy_utils/src/diagnostics.rs b/clippy_utils/src/diagnostics.rs index 7c94474cb35..71cfa196fc3 100644 --- a/clippy_utils/src/diagnostics.rs +++ b/clippy_utils/src/diagnostics.rs @@ -65,7 +65,7 @@ pub fn span_lint(cx: &T, lint: &'static Lint, sp: impl Into $DIR/zero_div_zero.rs:6:25 /// | @@ -103,7 +103,7 @@ pub fn span_lint_and_help<'a, T: LintContext>( /// /// # Example /// -/// ```ignore +/// ```text /// error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. /// --> $DIR/drop_forget_ref.rs:10:5 /// | @@ -189,7 +189,7 @@ pub fn span_lint_hir_and_then( /// /// # Example /// -/// ```ignore +/// ```text /// error: This `.fold` can be more succinctly expressed as `.any` /// --> $DIR/methods.rs:390:13 /// | diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index f32f1109b08..884180f0586 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -195,8 +195,8 @@ pub fn vec_macro<'e>(cx: &LateContext<'_>, expr: &'e hir::Expr<'_>) -> Option(e: &'tcx Expr<'tcx>) -> Option>> { /// Try to match the AST for a pattern that contains a match, for example when two args are /// compared @@ -283,7 +283,7 @@ pub struct FormatArgsExpn<'tcx> { /// String literal expressions which represent the format string split by "{}" pub format_string_parts: &'tcx [Expr<'tcx>], - /// Symbols corresponding to [`format_string_parts`] + /// Symbols corresponding to [`Self::format_string_parts`] pub format_string_symbols: Vec, /// Expressions like `ArgumentV1::new(arg0, Debug::fmt)` pub args: &'tcx [Expr<'tcx>], diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 536d0f006e3..4f9aaf396b8 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -114,7 +114,7 @@ pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option< /// Checks whether a type implements a trait. /// The function returns false in case the type contains an inference variable. -/// See also [`get_trait_def_id`]. +/// See also [`get_trait_def_id`](super::get_trait_def_id). pub fn implements_trait<'tcx>( cx: &LateContext<'tcx>, ty: Ty<'tcx>, -- cgit 1.4.1-3-g733a5 From 295df88986408c504eaa9982208dca0b505f1de1 Mon Sep 17 00:00:00 2001 From: Jade Date: Wed, 11 Aug 2021 18:28:42 -0700 Subject: Reword is_trait_item description --- clippy_utils/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'clippy_utils') diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index da9560f8ccf..075f1890c17 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -332,9 +332,9 @@ pub fn is_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) /// 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. +/// For example, this can be used to find if an expression like `u64::default` +/// refers to an item of the trait `Default`, which is associated with the +/// `diag_item` of `sym::Default`. 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) -- cgit 1.4.1-3-g733a5