Skip to content

Commit

Permalink
Improve multiple_inherent_impl
Browse files Browse the repository at this point in the history
Allow the lint to be ignored on the type definition, or any impl block
  • Loading branch information
Jarcho committed May 7, 2021
1 parent ca53b50 commit 9444fe4
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 6 deletions.
26 changes: 21 additions & 5 deletions clippy_lints/src/inherent_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,34 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
..
}) = item.kind
{
// Remember for each inherent implementation encountered its id and span,
// but filter out implementations that have generic params (type or lifetime)
// or are derived from a macro
if !in_macro(item.span) && generics.params.is_empty() {
// 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.iter().map(|&(did, span)| (cx.tcx.type_of(did), span)) {
for (ty, span) in self
.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(
cx,
MULTIPLE_INHERENT_IMPL,
cx.tcx.hir().local_def_id_to_hir_id(adt.did.expect_local()),
)
})
})
{
match type_map.entry(ty) {
Entry::Vacant(e) => {
e.insert(span);
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,23 @@ impl WithArgs<u64> {
fn f3() {}
}

// Ok, the struct is allowed to have multiple impls.
#[allow(clippy::multiple_inherent_impl)]
struct Allowed;
impl Allowed {}
impl Allowed {}
impl Allowed {}

struct AllowedImpl;
#[allow(clippy::multiple_inherent_impl)]
impl AllowedImpl {}
// Ok, the first block is skipped by this lint.
impl AllowedImpl {}

struct OneAllowedImpl;
impl OneAllowedImpl {}
#[allow(clippy::multiple_inherent_impl)]
impl OneAllowedImpl {}
impl OneAllowedImpl {} // Lint, only one of the three blocks is allowed.

fn main() {}
14 changes: 13 additions & 1 deletion tests/ui/impl.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,17 @@ LL | | fn f2() {}
LL | | }
| |_^

error: aborting due to 3 previous errors
error: multiple implementations of this structure
--> $DIR/impl.rs:65:1
|
LL | impl OneAllowedImpl {} // Lint, only one of the three blocks is allowed.
| ^^^^^^^^^^^^^^^^^^^^^^
|
note: first implementation here
--> $DIR/impl.rs:62:1
|
LL | impl OneAllowedImpl {}
| ^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 4 previous errors

0 comments on commit 9444fe4

Please sign in to comment.