From e1d47cd5f1c8158671d4ae766235afd8a58783cb Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Thu, 7 Mar 2019 07:42:38 +0100 Subject: [PATCH 1/4] Refactor: Extract `trait_ref_of_method` function --- clippy_lints/src/assign_ops.rs | 9 ++------- clippy_lints/src/missing_const_for_fn.rs | 17 ++--------------- clippy_lints/src/suspicious_trait_impl.rs | 7 ++----- clippy_lints/src/utils/mod.rs | 21 +++++++++++++++++++++ 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index b24e0cdfc..b0bf25002 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -1,4 +1,4 @@ -use crate::utils::{get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, SpanlessEq}; +use crate::utils::{get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, trait_ref_of_method, SpanlessEq}; use crate::utils::{higher, sugg}; use if_chain::if_chain; use rustc::hir; @@ -140,13 +140,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps { }; // check that we are not inside an `impl AssignOp` of this exact operation let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id); - let parent_impl = cx.tcx.hir().get_parent_item(parent_fn); - // the crate node is the only one that is not in the map if_chain! { - if parent_impl != hir::CRATE_HIR_ID; - if let hir::Node::Item(item) = cx.tcx.hir().get_by_hir_id(parent_impl); - if let hir::ItemKind::Impl(_, _, _, _, Some(trait_ref), _, _) = - &item.node; + if let Some(trait_ref) = trait_ref_of_method(cx, parent_fn); if trait_ref.path.def.def_id() == trait_id; then { return; } } diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index 19757f32e..5bc949a66 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -1,5 +1,4 @@ -use crate::utils::{is_entrypoint_fn, span_lint}; -use if_chain::if_chain; +use crate::utils::{is_entrypoint_fn, span_lint, trait_ref_of_method}; use rustc::hir; use rustc::hir::intravisit::FnKind; use rustc::hir::{Body, Constness, FnDecl, HirId}; @@ -96,7 +95,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn { } }, FnKind::Method(_, sig, ..) => { - if is_trait_method(cx, hir_id) || already_const(sig.header) { + if trait_ref_of_method(cx, hir_id).is_some() || already_const(sig.header) { return; } }, @@ -115,18 +114,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn { } } -fn is_trait_method(cx: &LateContext<'_, '_>, hir_id: HirId) -> bool { - // Get the implemented trait for the current function - let parent_impl = cx.tcx.hir().get_parent_item(hir_id); - if_chain! { - if parent_impl != hir::CRATE_HIR_ID; - if let hir::Node::Item(item) = cx.tcx.hir().get_by_hir_id(parent_impl); - if let hir::ItemKind::Impl(_, _, _, _, Some(_trait_ref), _, _) = &item.node; - then { return true; } - } - false -} - // We don't have to lint on something that's already `const` fn already_const(header: hir::FnHeader) -> bool { header.constness == Constness::Const diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 0b242d614..76ca1dc28 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -1,4 +1,4 @@ -use crate::utils::{get_trait_def_id, span_lint}; +use crate::utils::{get_trait_def_id, span_lint, trait_ref_of_method}; use if_chain::if_chain; use rustc::hir; use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; @@ -177,12 +177,9 @@ fn check_binop<'a>( // Get the actually implemented trait let parent_fn = cx.tcx.hir().get_parent_item(expr.hir_id); - let parent_impl = cx.tcx.hir().get_parent_item(parent_fn); if_chain! { - if parent_impl != hir::CRATE_HIR_ID; - if let hir::Node::Item(item) = cx.tcx.hir().get_by_hir_id(parent_impl); - if let hir::ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, _) = item.node; + if let Some(trait_ref) = trait_ref_of_method(cx, parent_fn); if let Some(idx) = trait_ids.iter().position(|&tid| tid == trait_ref.path.def.def_id()); if binop != expected_ops[idx]; then{ diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 70ba0592c..6713f4769 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -301,6 +301,27 @@ pub fn implements_trait<'a, 'tcx>( .enter(|infcx| infcx.predicate_must_hold_modulo_regions(&obligation)) } +/// Get the `hir::TraitRef` of the trait the given method is implemented for +/// +/// Use this if you want to find the `TraitRef` of the `Point` trait in this example: +/// +/// ```rust +/// trait Point; +/// +/// impl std::ops::Add for Point {} +/// ``` +pub fn trait_ref_of_method(cx: &LateContext<'_, '_>, hir_id: HirId) -> Option { + // Get the implemented trait for the current function + let parent_impl = cx.tcx.hir().get_parent_item(hir_id); + if_chain! { + if parent_impl != hir::CRATE_HIR_ID; + if let hir::Node::Item(item) = cx.tcx.hir().get_by_hir_id(parent_impl); + if let hir::ItemKind::Impl(_, _, _, _, trait_ref, _, _) = &item.node; + then { return trait_ref.clone(); } + } + None +} + /// Check whether this type implements Drop. pub fn has_drop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool { match ty.ty_adt_def() { From 65694cc6c8df092353a3dec046d7101857e979e7 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 8 Mar 2019 09:10:41 +0100 Subject: [PATCH 2/4] Fix doctest --- clippy_lints/src/utils/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 6713f4769..7d370b475 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -306,9 +306,15 @@ pub fn implements_trait<'a, 'tcx>( /// Use this if you want to find the `TraitRef` of the `Point` trait in this example: /// /// ```rust -/// trait Point; +/// struct Point(isize, isize); /// -/// impl std::ops::Add for Point {} +/// impl std::ops::Add for Point { +/// type Output = Self; +/// +/// fn add(self, other: Self) -> Self { +/// Point(0, 0) +/// } +/// } /// ``` pub fn trait_ref_of_method(cx: &LateContext<'_, '_>, hir_id: HirId) -> Option { // Get the implemented trait for the current function From 837d675afd3219155b429947d71458de53614b1d Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Fri, 8 Mar 2019 09:40:12 +0100 Subject: [PATCH 3/4] Update clippy_lints/src/utils/mod.rs Co-Authored-By: phansch --- clippy_lints/src/utils/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 7d370b475..1a5b4f12b 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -303,7 +303,7 @@ pub fn implements_trait<'a, 'tcx>( /// Get the `hir::TraitRef` of the trait the given method is implemented for /// -/// Use this if you want to find the `TraitRef` of the `Point` trait in this example: +/// Use this if you want to find the `TraitRef` of the `Add` trait in this example: /// /// ```rust /// struct Point(isize, isize); From 131b89b54ecdefcf7576f556c637441aa81f2da0 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 8 Mar 2019 09:42:09 +0100 Subject: [PATCH 4/4] fmt --- clippy_lints/src/assign_ops.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index b0bf25002..87e92c2bd 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -1,4 +1,6 @@ -use crate::utils::{get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, trait_ref_of_method, SpanlessEq}; +use crate::utils::{ + get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, trait_ref_of_method, SpanlessEq, +}; use crate::utils::{higher, sugg}; use if_chain::if_chain; use rustc::hir;