Rust-clippy: Multiple inherent impls should consider implementations by macros

Created on 25 Sep 2019  路  5Comments  路  Source: rust-lang/rust-clippy


$ cargo clippy -V
clippy 0.0.212 (e3cb40e 2019-06-25)

I am using the https://github.com/nrc/derive-new crate to automatically generate struct constructors using #[derive(new)]. However, I also want to implement other associated methods with an impl Struct block, but doing this triggers the multiple_inherent_impl lint.

  • This lint should probably not be triggered when impl are derived from macros, as the concern about navigating multiple code does not apply?
  • I could not disable the warning locally. I tried applying #[allow(clippy::multiple_inherent_impl)] to the struct, to my impl Struct and also at the module level using #![allow(clippy::multiple_inherent_impl)], but clippy still throws the warning. Am I missing something?

The full warning I'm getting is:

warning: Multiple implementations of this structure
  --> path/mod.rs:59:34
   |
59 | #[derive(Serialize, Deserialize, new)]
   |                                  ^^^
   |
note: lint level defined here
  --> src/lib.rs:14:5
   |
14 |     clippy::restriction,
   |     ^^^^^^^^^^^^^^^^^^^
   = note: #[warn(clippy::multiple_inherent_impl)] implied by #[warn(clippy::restriction)]
note: First implementation here
  --> path/mod.rs:70:1
   |
70 | / impl MyStruct {
71 | |     pub fn my_method(&mut self) {
72 | |         ...
...  |
94 | |     }
95 | | }
   | |_^

Thanks.

L-bug T-macros good-first-issue

Most helpful comment

Hi, I'd like to work on this issue

All 5 comments

implied by #[warn(clippy::restriction)]

Enabling #[warn(clippy::restriction)] doesn't really make sense, since this category has lints, which contradict with other lints. From the restriction group only selected lints should be enabled. These lints are there to restrict the language in your specific project.


Regarding your question:
This is a crate level lint, so it can only be enabled/disabled on a crate level.

https://github.com/rust-lang/rust-clippy/blob/5a11ed7b92cc4cf40a4568a8fc1ff54b198c333b/clippy_lints/src/inherent_impl.rs#L61

The macro problem can be fixed, by inserting a macro check here:
https://github.com/rust-lang/rust-clippy/blob/5a11ed7b92cc4cf40a4568a8fc1ff54b198c333b/clippy_lints/src/inherent_impl.rs#L55

Hi, I'd like to work on this issue

Thanks @flip1995!

From the restriction group only selected lints should be enabled.

My idea is to enable warnings for everything and then explicitly disable the ones that I don't need. In this way, I will keep getting notified of any new lints that are added without having to go through the list at every release.

This is a crate level lint, so it can only be enabled/disabled on a crate level.

I did not know there was this categorization and didn't see it in any documentation yet. Perhaps you want to add this info to the README configuration section? Also, is it possible/desirable to add a meta warning on encountering a clippy lint config that will not take effect?

I will keep getting notified of any new lints that are added without having to go through

That's fair. :)

I did not know there was this categorization and didn't see it in any documentation yet

Yeah, because that is kind of an implementation detail and isn't really a problem for most lints. The thing with this lint is, that it has to do an analysis over the whole crate, before the lint can be emitted, so it is only possible to disable it crate wise. There are very few of these lints (I can't think of another lint like that) so adding this to the documentation would be more confusing IMO (most lints can be enabled/disabled where you would expect it).

Also, is it possible/desirable to add a meta warning on encountering a clippy lint config that will not take effect

This is actually a pretty old feature wish for the rust compiler to warn for useless/no-effect allows and is not that easy to implement. (Sadly I can't find the issue right now...)

Thanks for the explanation!

adding this to the documentation would be more confusing

How about adding a note to the lint description page (#multiple_inherent_impl)? These links appear in the clippy check messages, so a note saying "This lint can only be configured at the crate root" here should quickly help someone wondering why an allow/deny config is not working.

Was this page helpful?
0 / 5 - 0 ratings