Add option to disable all linting in external macros

[jFransham]

I use Clippy a lot (even writing a Syntastic extension for it) but it seems overly harsh on code that is generated by macros. For example, the Oak parser generator uses closures to unroll tuple arguments into regular arguments, but if you have single-argument functions the linter will throw a redundant closure check - not something that could be fixed by the library author or the user (as far as I know). If there was an option to disable this, the pedant inside me who likes to get rid of all warnings would be happy and I could continue tapping keys and fighting crime.

[llogiq]

I'm entirely OK with this proposal, it's not even that hard – only someone needs to find the time to look through all passes and introduce the macro checks wherever they're missing.

Keep in mind that we have some lints that should not trigger within macros at all (that's why we have two methods in utils). Should macros deactivate all readability-related lints regardless of origin?

[jFransham]

Can we check the inputs to macros and see if the problem is in the input? This would, for example, catch println!("{}", (x | 1) > 2) and other problems that would be false negatives if all lints were disabled in macros. What I'm thinking is, parse each of the macro's variables as ASTs (sans type information etc.) and run the lints that only need an AST on those. If I get time over the next week I'll work on doing a pull request for this.

It won't catch problems in calls to macros that duplicate rust syntax - stuff like (fn $name($first, $second) => $expr) => {} (you see stuff like this a fair bit in DSL-type macros) or syntax extensions (maybe? I haven't written one so I don't know if they specify inputs like macros do) but it'd be a really nice feature to have.

EDIT: accidentally clicked "close and comment"

[Manishearth]

It's a bit hard to generalize that. Lints don't run pre-expansion

Currently we have a set of lints which are just turned off when in a macro. We could add a feature to turn off all lints in macros.

[llogiq]

Somehow rustc can usually report which macros expanded what, so the information is clearly available. It's just not reachable from the lint for now. Perhaps we should lobby to get access to it?

[Manishearth]

ExpnInfo knows about it. We have access, it's just hard to unroll and work with.

[leoyvens]

needless_return currently triggers on macro expansions, it shouldn't since it's readability related and "fixing it" can break code elsewhere.

[Manishearth]

Yes, that should be unconditionally off in macros.

I wonder if we can be smarter about our macro checks (as well as providing more control to the user about tweaking them). For example, the return lint should only trigger if the return and the surrounding fn item are from different macro contexts.

[llogiq]

Yeah, but even same_expansion_ctx(nodeid, nodeid) is a bit complex due to expansion chaining. It's not enough to have the same expansion somewhere (or everything in the same for loop would match).

[jFransham]

It's worth noting that IIRC the compiler team are planning an overhaul of the macro semantics and syntax, if this is true then it might be worth waiting to implement this.

[leoyvens]

@jFransham True, however the existing macros will take a while to migrate, so the current macro system will still be relevant.

[rcoh]

What's the status of this? I'm running into the same issue (although now with nom). Have the compiler internals caught up?

[oli-obk]

It's being worked on in the compiler: https://github.com/rust-lang/rust/pull/49755

[sbeyer]

Activity or mentions here in every year since 2015. Now let's add 2021 to the list: I stumbled over this issue yesterday and while checking if I could disable lints in macro expansions, I found this issue.

My case is very simple. The macro (from a crate that I use) calls into() on some input arguments. So cargo clippy brought up some useless conversion warnings. Because I did not want to disable that useful warning globally, I had to put #[allow(clippy::useless_conversion)] above every use of that macro.

I think this is a situation that is very natural to have in a macro and to make clear why it could be nicer for the users to ignore lints within macro expansions, at least if they come from a crate.

As a side note: having to add some #[allow(...)] lines was not the worst thing! The worst thing was the initial confusion the clippy warning gave me, because clippy did not show me the expanded macro... for me, the macro was simply a black-box. So, at first sight, I had no idea why there was a useless conversion. So providing a glance into the macro expansion could also have helped – maybe.

[oli-obk]

We could have a crate wide attribute that silences all clippy lints with spans from external macros and add a help explaining the macro situation (and the crate wide attr) if the attribute is not set

[sbeyer]

@oli-obk That would be the best solution (from my user perspective). I have no idea how hard that is to implement.

[jonas-schievink]

https://github.com/rust-lang/rust/pull/52467 has been merged a long time ago, but some Clippy lints still incorrectly fire for code generated by foreign macros (for example in https://github.com/rust-lang/rust-clippy/issues/4861 and https://github.com/knurling-rs/defmt/issues/642).