Llogiq on stuff

Should Clippy Care From Whence They Came?

2019 was a great year for clippy. It’s available on stable, even installed by default in the Rust distribution and selectable as a rustup component. We have more than 300 lints, and the upwards trend is unbroken. The lints that we have also see a steady stream of improvements.

By the end of the year I found myself mulling a question that I thought should have a definite answer, but so far it seems to have eluded us:

Should clippy lint code expanded from macros? And if so, also from macros defined outside of the current crate?

Note that there are a few concerns to unpack here. First, macros often make it harder to get the code quite right and readability of the expanded code is likely a secondary concern relative to readability of the macro and the call. Also, giving a suggestion often requires a valid span, and macros (especially procedural ones) may break this. Finally, if the code is from an external macro the user may not even be able to do anything about it.

So far our response to this complexity has been to add macro checks where users have asked for it. This has led to an ad-hoc solution full of holes. I surveyed our code and found that 153 lints don’t care about macros at all, 44 will lint on internal macros only, and 129 will completely ignore macro generated code (The rest are either cargo lints or have their own special handling).

Now it might be a good idea to answer the question once and for all so we can update our lints to get consistent behavior when it comes to macros. So here’s my thinking:

  1. Style & Complexity lints should not lint in macros at all – while they may be useful on occasion, they often add more noise than value, especially because they trigger on every expansion of the macro instead of the definition. We may reconsider this once we find a way to lint the macro definition instead of the callsites.
  2. Correctness lints should always lint on internal macros, and ideally configurably also on external macros. Depending on the severity we may even decide to always lint (e.g. if the lint triggers on undefined behavior, we don’t want to omit it just because there’s a macro in the way.
  3. Perf lints should also always lint on internal macros. If we already have a way to configure linting on external macros, we may as well use it for them, too.
  4. Restriction, Pedantic and Nursery lints are allow by default anyway. They need further analysis to decide whether they warrant linting in macros. The default should be omitting linting in external macros (or all macros) unless it can be shown that linting in those cases is useful enough to warrant the noise.
  5. If the lint makes any sense at all even without suggestion, we should lint even in macros, but omit the suggestion.

Now we have to create the config plumbing for being able to meet points 2 & 3. Then it’s a matter of many small contributions to clippy to let it handle macros in a sane and consistent way. My hope is that we can crowdsource this effort in 2020 and attract new clippy contributors this way.