Skip to content

Commit

Permalink
Use crate_inherent_impls in multiple_inhernet_impl lint
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarcho committed May 10, 2021
1 parent 9444fe4 commit d6841da
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 49 deletions.
128 changes: 80 additions & 48 deletions clippy_lints/src/inherent_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<Span> {
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<Span> {
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),
}
}
}
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit d6841da

Please sign in to comment.