Skip to content
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

[unstable option] format_generated_files #5080

Open
4 tasks
karyon opened this issue Nov 14, 2021 · 10 comments
Open
4 tasks

[unstable option] format_generated_files #5080

karyon opened this issue Nov 14, 2021 · 10 comments
Labels
unstable option tracking issue of an unstable option

Comments

@karyon
Copy link
Contributor

karyon commented Nov 14, 2021

Tracking issue for unstable option: format_generated_files.

See Processes.md, "Stabilising an Option":

  • Is the default value correct?
  • The design and implementation of the option are sound and clean.
  • The option is well tested, both in unit tests and, optimally, in real usage.
  • There is no open bug about the option that prevents its use.
@karyon karyon added the unstable option tracking issue of an unstable option label Nov 14, 2021
@Ten0
Copy link

Ten0 commented Jan 14, 2022

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
Copy link
Member

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
Copy link

Ten0 commented Jan 14, 2022

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:

  • Diesel CLI call to generate the file
  • rustfmt schema.rs
  • patch -i schema.patch ./src/schema.rs

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
Copy link
Member

calebcartwright commented Jan 14, 2022

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:

  • Use the command line override e.g. rustfmt schema.rs --config format_generated_files=true (yes, it'll work on stable)
  • Use nightly rustfmt for that specific step, rustfmt +nightly schema.rs
  • Use the 1.57 stable toolchain for whatever step/job is formatting the generated file
  • Adjust the generated file header such that the @generated marker appears below line 5 of the file (realize that may be problematic for other reasons, but mentioning because it would resolve the issue you are experiencing)

@Ten0
Copy link

Ten0 commented Jan 14, 2022

  • 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! :)

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 15, 2022
…es, r=Mark-Simulacrum

resolve rustfmt issue with generated files

Discussed in https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/1.2E58.20patch.20release

refs rust-lang/rustfmt#5080 (comment) and rust-lang/rustfmt#5172

updating in-tree vs. subtree sync to make backporting easier, would like to nominate backporting to both beta/1.59 as well as the 1.58.1 patch release
@captbaritone
Copy link
Contributor

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
Copy link
Member

jhpratt commented Apr 7, 2023

Is there a technical reason this is ignored for stdin?

@calebcartwright
Copy link
Member

Is there a technical reason this is ignored for stdin?

Yes, at least in so far as current state behavior. See #5172 and

// FIXME(calebcartwright) - we need to determine how we'll handle the
// `format_generated_files` option with stdin based input.
if !input_is_stdin && !config.format_generated_files() {

@DavisVaughan
Copy link

@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
Copy link
Member

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unstable option tracking issue of an unstable option
Projects
None yet
Development

No branches or pull requests

6 participants