Open
Description
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.
Metadata
Metadata
Assignees
Labels
Type
Projects
Milestone
Relationships
Development
No branches or pull requests
Participants
Activity
llogiq commentedon Oct 22, 2015
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 commentedon Oct 23, 2015
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 commentedon Oct 23, 2015
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 commentedon Oct 23, 2015
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 commentedon Oct 23, 2015
ExpnInfo knows about it. We have access, it's just hard to unroll and work with.
leoyvens commentedon Dec 12, 2015
needless_returncurrently triggers on macro expansions, it shouldn't since it's readability related and "fixing it" can break code elsewhere.Manishearth commentedon Dec 12, 2015
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 commentedon Dec 13, 2015
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 commentedon Dec 13, 2015
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 commentedon Dec 13, 2015
@jFransham True, however the existing macros will take a while to migrate, so the current macro system will still be relevant.
rcoh commentedon Apr 9, 2018
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 commentedon Apr 9, 2018
It's being worked on in the compiler: rust-lang/rust#49755
sbeyer commentedon Oct 11, 2021
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. Socargo clippybrought up someuseless conversionwarnings. 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 commentedon Oct 11, 2021
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 commentedon Oct 11, 2021
@oli-obk That would be the best solution (from my user perspective). I have no idea how hard that is to implement.
jonas-schievink commentedon Nov 29, 2021
rust-lang/rust#52467 has been merged a long time ago, but some Clippy lints still incorrectly fire for code generated by foreign macros (for example in #4861 and knurling-rs/defmt#642).
Solve clippy::forget-non-drop lint error
Solve clippy::forget-non-drop lint error
clippy::panic_in_result_fnlint for functions returningResulttokio-rs/tracing#2498