rust-lang / rustfmt

Format Rust code
https://rust-lang.github.io/rustfmt/
Apache License 2.0
6.04k stars 889 forks source link

[unstable option] `format_generated_files` #5080

Open karyon opened 3 years ago

karyon commented 3 years ago

Tracking issue for unstable option: format_generated_files.

See Processes.md, "Stabilising an Option":

Ten0 commented 2 years ago

The latest rust version made false the default, but it's still unstable. This broke software that relied on being able to format generated file (e.g. ours, applying .patch to rustfmt-d generated code), and we can't even re-enable formatting of generated files because this option is marked as unstable.

I think it's inconsistent to have this as an unstable option if the extra code that this feature represents is now enabled by default. Please make this stable asap, or leave true as default as it was before.

calebcartwright commented 2 years ago

Thanks for reaching out @Ten0, could you expand a bit on your use case?

IIUC, you've been inserting @generated comments within the first few lines at the top of generated files historically, predating the recent changes to rustfmt that actually recognize this/utilize this new option. And your desire is to be able to use stable rustfmt to format those generated files?

Ten0 commented 2 years ago

We've been using Diesel CLI to generate schema.rs files automatically. This flags files as @generated (legitimately, because they are). We then need to make a few changes to these files. So the process to generate them is:

Because rustfmt now doesn't format these files anymore, all our patch files don't work anymore (they are based on formatted file and not un-formatted file, which is desired), hence our CI/CD broke.

Our current workaround will obviously be to patch the outputs of the diesel CLI to remove the @generated flag, but that is obviously incorrect semantically and pretty disgusting. (We also have other generated files related to e.g. gRPC which we occasionally need to read and that don't format anymore...)

calebcartwright commented 2 years ago

Gotcha, thanks for confirming.

We do need to get the default updated because we inverted the option itself to avoid double negatives but failed to make the corresponding update to the default. However, I'm not sure whether that'll warrant a 1.58.1 patch release (will check with the release team though)

Here's some tactical alternatives that should be available to you already:

Ten0 commented 2 years ago
  • Use the command line override e.g. rustfmt schema.rs --config format_generated_files=true (yes, it'll work on stable)

This will work great as a workaround for now. Thanks! :)

captbaritone commented 1 year ago

Perhaps another thing to consider in terms of promoting this to stable is if we want to bundle it with #5658, the ability to configure the number of header lines that rustfmt will look for for @generated in.

jhpratt commented 1 year ago

Is there a technical reason this is ignored for stdin?

calebcartwright commented 1 year ago

Is there a technical reason this is ignored for stdin?

Yes, at least in so far as current state behavior. See #5172 and https://github.com/rust-lang/rustfmt/blob/a3b2bfc2db33ec3bed266404e6391c145e000df0/src/formatting.rs#L79-L81

DavisVaughan commented 9 months ago

@calebcartwright is there an issue or something somewhere else that tracks the stdin support that I can upvote? It would be nice if format-on-save in VS Code would ignore files tagged with // @generated if I happen to hit save while inside them (a rare situation, but jarring nonetheless!). IIUC format-on-save there is run through stdin.

calebcartwright commented 9 months ago

@calebcartwright is there an issue or something somewhere else that tracks the stdin support that I can upvote? It would be nice if format-on-save in VS Code would ignore files tagged with // @generated if I happen to hit save while inside them (a rare situation, but jarring nonetheless!). IIUC format-on-save there is run through stdin.

Not that I'm aware of, I believe it's just a known issue that would need to be resolved in order for this option to potentially be stabilized