diff --git a/clippy_lints/src/inherent_impl.rs b/clippy_lints/src/inherent_impl.rs index c602db111769..ba838c1ef102 100644 --- a/clippy_lints/src/inherent_impl.rs +++ b/clippy_lints/src/inherent_impl.rs @@ -48,7 +48,10 @@ declare_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]); impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { fn check_crate_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Crate<'_>) { + // Map from a type to it's first impl block. Needed to distinguish generic arguments. + // e.g. `Foo` and `Foo` let mut type_map = FxHashMap::default(); + // List of spans to lint. (lint_span, first_span) let mut lint_spans = Vec::new(); for (_, impl_ids) in cx @@ -58,6 +61,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { .iter() .filter(|(id, impls)| { impls.len() > 1 + // Check for `#[allow]` on the type definition && !is_allowed( cx, MULTIPLE_INHERENT_IMPL, @@ -68,23 +72,38 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { 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) => { + // Store the id for the first impl block of this type. The span is retrieved lazily. 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); - } + let first_span = match *e.get() { + IdOrSpan::Span(s) => s, + IdOrSpan::Id(id) => match get_impl_span(cx, id) { + Some(s) => { + // Remember the span of the first block. + *e.get_mut() = IdOrSpan::Span(s); + s + }, + None => { + // The first impl block isn't considered by the lint. Replace it with the + // current one. + *e.get_mut() = IdOrSpan::Span(span); + continue; + }, + }, + }; + lint_spans.push((span, first_span)); } }, } } + // Switching to the next type definition, no need to keep the current entries around. type_map.clear(); } + // `TyCtxt::crate_inherent_impls` doesn't have a defined order. Sort the lint output first. lint_spans.sort_by_key(|x| x.0.lo()); for (span, first_span) in lint_spans { span_lint_and_note( @@ -99,6 +118,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { } } +/// Gets the span for the given impl block unless it's not being considered by the lint. 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 { @@ -118,14 +138,3 @@ enum IdOrSpan { Id(LocalDefId), Span(Span), } -impl IdOrSpan { - fn as_span(&mut self, cx: &LateContext<'_>) -> Option { - match *self { - Self::Id(id) => get_impl_span(cx, id).map(|span| { - *self = Self::Span(span); - span - }), - Self::Span(span) => Some(span), - } - } -}