trailofbits / dylint

Run Rust lints from dynamic libraries
https://blog.trailofbits.com/2021/11/09/write-rust-lints-without-forking-clippy/
Apache License 2.0
374 stars 21 forks source link

Sharing Clippy lints with 3rd party crates #1057

Open nyurik opened 7 months ago

nyurik commented 7 months ago

Thanks for a great tool! I created uninlined_format_args Clippy lint that works on the core library's format! and similar macros. There are also a few other similar format-focused lints, but they all focus just the core lib. The same lint should be usable on numerous 3rd party crates like log - crates with macros that mimic format! behavior. Sometimes 3rd party crates add a few arguments before the format string like info!(razor = razor; "Razor located {foo} {}", bar(10));

Is there a good way for 3rd party crates to support the built-in format lints without the complexities of implementing such lints from scratch? The code is obviously non-trivial, so it would be a huge task for each format-using crate to re-implement it.

smoelius commented 7 months ago

Thanks for your kind words. And I am a fan of uninlined_format_args. :)

Is there a good way for 3rd party crates to support the built-in format lints without the complexities of implementing such lints from scratch?

I'm not sure there is a good answer to this. One could publish a library that constitutes the bulk of the lint's logic, and then the third party could add whatever else was needed to complete it. But it sounds like you are looking for something more than that. Could you maybe share the kind of answer you were hoping for?

Would it not make sense to have just one lint that handles a set of known third-party crates? You mentioned log. anyhow is another one I can think of. But I presume there are too many to handle with just one lint?

Aside: I may send further replies from a different account.

nyurik commented 7 months ago

IntelliJ for C# has solved it with a custom attribute (message is the name of the formatting string arg that must be followed by any number of args):

[StringFormatMethod("message")]
void ShowError(string message, params object[] args) { /* do something */ }

void Foo() {
  ShowError("Failed: {0}"); // Warning: Non-existing argument in format string
}

In theory, all rust macros that behave like format! would be similarly annotated, but it won't be as easy. It works in C# because compiler ignores any unknown attribute, so a user can add an attribute specific to IntelliJ's tool to their code.

In Rust, we cannot have an attribute without declaration, so I am not sure what the good solution would be... Perhaps work with the core lib to add it there?

smoelius1 commented 7 months ago

To be sure I understand, you're imagining that a third party maintainer would an annotation each relevant macro in their package?

nyurik commented 7 months ago

@smoelius1 ideally, https://github.com/rust-lang/rust-clippy/pull/9948 should "just work". But people did bring up a lot of corner cases where suggestions might be incorrect... Hence, I was wondering if this is something each lib author can optionally help with. Still, not ideal, so just thinking out loud.

smoelius commented 7 months ago

I think I'm getting lost on the thread here. I see that there have been updates to https://github.com/rust-lang/rust-clippy/pull/9948 since this issue was opened. Is it still possible Dylint could help you? If so, how?