From d6841daf0fc174de2cb3928deed4e67c87675cbc Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 10 May 2021 10:58:22 -0400 Subject: [PATCH] Use `crate_inherent_impls` in `multiple_inhernet_impl` lint --- clippy_lints/src/inherent_impl.rs | 128 +++++++++++++++++++----------- clippy_lints/src/lib.rs | 2 +- 2 files changed, 81 insertions(+), 49 deletions(-) diff --git a/clippy_lints/src/inherent_impl.rs b/clippy_lints/src/inherent_impl.rs index ae4e0eae7031..c8c61b76ded3 100644 --- a/clippy_lints/src/inherent_impl.rs +++ b/clippy_lints/src/inherent_impl.rs @@ -3,9 +3,12 @@ use clippy_utils::diagnostics::span_lint_and_note; use clippy_utils::{in_macro, is_allowed}; use rustc_data_structures::fx::FxHashMap; -use rustc_hir::{def_id::LocalDefId, Crate, Impl, Item, ItemKind}; +use rustc_hir::{ + def_id::{LocalDefId, LOCAL_CRATE}, + Crate, Item, ItemKind, Node, +}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::Span; use std::collections::hash_map::Entry; @@ -41,63 +44,92 @@ declare_clippy_lint! { "Multiple inherent impl that could be grouped" } -#[allow(clippy::module_name_repetitions)] -#[derive(Default)] -pub struct MultipleInherentImpl { - impls: Vec<(LocalDefId, Span)>, -} - -impl_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]); +declare_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]); impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { - fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Impl(Impl { - ref generics, - of_trait: None, - .. - }) = item.kind - { - // Remember the order inherent implementations are defined. Note `TyCtxt::crate_inherent_impls` does - // not have a defined order. TODO: Check if the generic parameters are the same. - if !in_macro(item.span) - && generics.params.is_empty() - && !is_allowed(cx, MULTIPLE_INHERENT_IMPL, item.hir_id()) - { - self.impls.push((item.def_id, item.span)); - } - } - } - fn check_crate_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Crate<'_>) { let mut type_map = FxHashMap::default(); - for (ty, span) in self - .impls + let mut lint_spans = Vec::new(); + + for (_, impl_ids) in cx + .tcx + .crate_inherent_impls(LOCAL_CRATE) + .inherent_impls .iter() - .map(|&(did, span)| (cx.tcx.type_of(did), span)) - .filter(|&(ty, _)| { - // Filter out any type which has `#[allow(clippy::multiple_inherent_impl)]` - ty.ty_adt_def().map_or(false, |adt| { - !is_allowed( + .filter(|(id, impls)| { + impls.len() > 1 + && !is_allowed( cx, MULTIPLE_INHERENT_IMPL, - cx.tcx.hir().local_def_id_to_hir_id(adt.did.expect_local()), + cx.tcx.hir().local_def_id_to_hir_id(id.expect_local()), ) - }) }) { - match type_map.entry(ty) { - Entry::Vacant(e) => { - e.insert(span); - }, - Entry::Occupied(e) => span_lint_and_note( - cx, - MULTIPLE_INHERENT_IMPL, - span, - "multiple implementations of this structure", - Some(*e.get()), - "first implementation here", - ), + for impl_id in impl_ids.iter().map(|id| id.expect_local()) { + match type_map.entry(cx.tcx.type_of(impl_id)) { + Entry::Vacant(e) => { + e.insert(IdOrSpan::Id(impl_id)); + }, + Entry::Occupied(mut e) => { + if let Some(span) = get_impl_span(cx, impl_id) { + if let Some(first_span) = e.get_mut().as_span(cx) { + lint_spans.push((span, first_span)); + } else { + *e.get_mut() = IdOrSpan::Span(span); + } + } + }, + } } + + type_map.clear(); + } + + lint_spans.sort_by_key(|x| x.0.lo()); + for (span, first_span) in lint_spans { + span_lint_and_note( + cx, + MULTIPLE_INHERENT_IMPL, + span, + "multiple implementations of this structure", + Some(first_span), + "first implementation here", + ); + } + } +} + +fn get_impl_span(cx: &LateContext<'_>, id: LocalDefId) -> Option { + let id = cx.tcx.hir().local_def_id_to_hir_id(id); + if let Node::Item(&Item { + kind: ItemKind::Impl(ref impl_item), + span, + .. + }) = cx.tcx.hir().get(id) + { + (!in_macro(span) && impl_item.generics.params.is_empty() && !is_allowed(cx, MULTIPLE_INHERENT_IMPL, id)) + .then(|| span) + } else { + None + } +} + +enum IdOrSpan { + Id(LocalDefId), + Span(Span), +} +impl IdOrSpan { + fn as_span(&mut self, cx: &LateContext<'_>) -> Option { + match *self { + Self::Id(id) => { + if let Some(span) = get_impl_span(cx, id) { + *self = Self::Span(span); + Some(span) + } else { + None + } + }, + Self::Span(span) => Some(span), } } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 78a95b004034..47c576bac486 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1154,7 +1154,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box suspicious_operation_groupings::SuspiciousOperationGroupings); store.register_late_pass(|| box suspicious_trait_impl::SuspiciousImpl); store.register_late_pass(|| box map_unit_fn::MapUnit); - store.register_late_pass(|| box inherent_impl::MultipleInherentImpl::default()); + store.register_late_pass(|| box inherent_impl::MultipleInherentImpl); store.register_late_pass(|| box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd); store.register_late_pass(|| box unwrap::Unwrap); store.register_late_pass(|| box duration_subsec::DurationSubsec);