-
Notifications
You must be signed in to change notification settings - Fork 1.6k
RFC: Autogenerated Attribute #3850
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
When writing tools that produce Rust code, they should add a `#[autogenerated]` attribute to the generated code. This attribute can be added onto any item, such as modules, but also functions, constants, etc. The attribute takes many parameters: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attribute takes many parameters:
What is the advantage of these parameters over separate, existing or new, tool attributes? It seems to me that combining them may have a slight benefit in readability, but has the large cost that it's a new mechanism that needs a schema and/or namespacing strategy. As you wrote yourself:
One problem that could arise is tools having their own parameters for the attribute that aren't specified, and it's unclear how to work around that.
I think it would make sense to keep the parameters down to ones which relate to the generation itself rather than setting any policy:
#[autogenerated(
by = "example tool", // The tool that created this item - required
regen_hint = "./regen_example_tool", // A hint for how to regenerate the item - strongly encouraged to be provided
)]
Everything else should be separate tool attributes (like rustfmt::skip
) possibly combined with a default behavior from those tools when they see any #[autogenerated]
attribute. If there’s some reason this can't work, it should at least be discussed “Rationale and alternatives” section.
|
||
Some things for individual tools do already exist such as `#[rustfmt::skip]`, however these of course apply only to individual tools meaning that a more general attribute would work well for this scenario. | ||
|
||
# Prior art |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A good addition to this section would be a summary of what effects these “generated” markers have on tools in other languages.
Consider alternative: use
The advantage of
This proposal RFC might be over-engineered a bit: in 99% of cases one just needs to know whether file is generated or not. |
|
||
Rust doesn't have a general attribute for this (some more specific attributes do exist, such as `#[automatically_derived]`), and this would be useful for scenarios such as: | ||
- `rust-analyzer`: Mark files as autogenerated and discourage users from editing them | ||
- `rustfmt`: Don't format files marked as autogenerated by default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto-generated code should get formatting to be readable to humans, because the generator sure doesn't need to duplicate all formatting logic into itself.
Rust doesn't have a general attribute for this (some more specific attributes do exist, such as `#[automatically_derived]`), and this would be useful for scenarios such as: | ||
- `rust-analyzer`: Mark files as autogenerated and discourage users from editing them | ||
- `rustfmt`: Don't format files marked as autogenerated by default | ||
- `clippy` and `rustc`: Suppress lints in autogenerated files by default - in other words, reset the list of lints to empty outside of lints marked explicitly in the file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better for lints to be "summarized" instead of suppressed entirely. Tell me one spot it appears (probably the first occurrence), and tell me it appears X many other times. Generators should generate code that is lint free, either because it does whatever to not fire the lint, or because it issues an allow
in an appropriate location.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generators should generate code that is lint free, either because it does whatever to not fire the lint, or because it issues an
allow
in an appropriate location.
As a data point, I tend to turn on all clippy lints and then opt out of the ones I don't want as triggering makes me aware of them, which generated code generally doesn't anticipate, and, when it runs afoul of that, I move the generated code into a submodule where I allow all to silence the lints because I don't think it's reasonable to bother dependency authors about that use-case.
(Which leaves the biggest annoyance as when I have to switch dependencies or move the generated code out into its own crate because it's violating the #![forbid(unsafe_code)]
that I put in my crates for the non-generated code.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like that sounds fine.
To elaborate a little more on why a "summary" is better than normal warnings: when working on the generator I want to generate some code, then have RA or Clippy or whatever tool inspect it. When doing this, I don't need to know all 4500 warnings and their line numbers. That's too much noise. Even with the most compact of current output options that's just too much. What I need to know is that one warning is triggered 1500 times and another warning is triggered 3000 times, and then I can go adjust the generator to fix what is actually just two problems, not 4500.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a specific example right now, but I have frequently run into code generators that used to be lint free, then you upgrade Rust and it's not anymore. That can happen because no maintenance or even just pinning Rust for an MSRV via rust-toolchain.toml.
I'm not sure what I think of the proposal, but it would definitely be nice if generated code didn't spam warnings because it e.g. doesn't use some new idiom. I feel like a real answer to that however would require having some sort of warnings level for generated code because it is indeed the case that many are important. I'll be honest that this specific point is why I took note that this RFC exists; literally everything else isn't a pain point I've managed to hit. I think it's also worth noting that the problem is slightly more general: you will see it e.g. sometimes with normal macro_rules.
It really feels like #[allow(autogenerated_safe)]
or something is the answer. But, the reason I'm replying to this subthread is that a summary doesn't actually fix the problem for downstream users, it just punts my specific problem with code generators as a consumer--I still have to go silence it and leave the ugly "because generated code" comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It really feels like #[allow(autogenerated_safe)] or something is the answer. But, the reason I'm replying to this subthread is that a summary doesn't actually fix the problem for downstream users, it just punts my specific problem with code generators as a consumer--I still have to go silence it and leave the ugly "because generated code" comments.
*nod* It reminds me of how I've become banner blind to the multiple screenfuls of "Please badger your upstream to bump their Flatpak runtime dependency. This one is no longer maintained" messages that I get when I run flatpak upgrade
. (Which are just silenced by frontends like KDE Discover.)
Are there other types of warnings buried in there? Probably. I know it used to be giving me "This Flatpak is unmaintained: Please install Nix and use our Nix flake for jstest-gtk" before I confirmed that KDE's Gamepads control panel is no longer lagging and dropped jstest-gtk as an app that'll eventually have to accept GTK 4's GNOME-isms.
(I generally trust my strategy for hand-tightening the sandbox on every flatpak'd app I install and applying a policy that an application can have network access or non-portalized access to filesystem locations outside its ~/.var/app/` folder but not both.)
What I do know is that I've got better things to do than play volunteer messenger and unpaid advocate.
Summary
This RFC introduces an attribute for marking files as autogenerated. This syntax can be used by everything from rustbindgen to protoc (protobuf compiler) and understood by humans and tools alike.
Rendered