update the generator to emit formatted files

[devoncarew]

• update the generator to emit formatted files

This uses package:dart_style to format the generated proto files.

There is additional complexity here as we support an option to generate metadata about the source location of symbols for .pb.dart and .pbenum.dart files. I see a reference to this being in support of the Kythe indexer. I'm not sure why it needs us to generate metadata files for it to use - it can just parse dart source directly. Perhaps this option is no longer needed? It would be nice to be able to remove it for general simplification.

[devoncarew]

@osa1, once this lands (after review and such), I may have one more small style PR, then would likely want to do another publish.

[osa1]

Why do you want the plugin to generate formatted files?

The problem with formatting generated files in protoc plugin is that the plugin and the project that uses generated files can have different formatting settings and foramtter versions. This may even be the common case for non-Google packages, or if you rarely update your protoc plugin as you update your Dart versions.

So you have to format generated files anyway with your project's formatter.

I don't remember if adding dart_style as a dependency was a problem for g3, but we should check that as well.

(I don't have access to g3 right now, I can check tomorrow.)

FWIW, grpc generator used to format generated files in the open source version but not in the g3 version. That may suggest that there's a problem with adding dart_style dependency to the g3 version. #702 then removed formatting in the open source version as well.

Maybe an alternative could be formatting the files with dart format (as a shell command) rather than with a formatter built into the plugin. That way the formatting would be done using the project's settings rather than the plugin's.

[devoncarew]

Why do you want the plugin to generate formatted files?

For general readability, and so the generated code looks and feels like regular dart code. I think its more common then not to have your generated code be formatted. See the readme example for our code_builder package: https://github.com/dart-lang/tools/tree/main/pkgs/code_builder.

The problem with formatting generated files in protoc plugin is that the plugin and the project that uses generated files can have different formatting settings and foramtter versions. This may even be the common case for non-Google packages, or if you rarely update your protoc plugin as you update your Dart versions.

So you have to format generated files anyway with your project's formatter.

Yes, this is a good point. I think less of an issue for quite a while, but we've had big formatter changes from 3.6 => 3.7, and additional changes from 3.7 => 3.8.

I don't remember if adding dart_style as a dependency was a problem for g3, but we should check that as well.

FWIW, grpc generator used to format generated files in the open source version but not in the g3 version. That may suggest that there's a problem with adding dart_style dependency to the g3 version. #702 then removed formatting in the open source version as well.

I don't know if the dep is an issue; worth digging into through.

Separately, I don't know that formatting generated files matters as much for google3. I think generated files there are treated more ephemerally; they're almost never committed, and just available after a blaze build. cc @srawlins

Maybe an alternative could be formatting the files with dart format (as a shell command) rather than with a formatter built into the plugin. That way the formatting would be done using the project's settings rather than the plugin's.

I think using the formatter as a library will have fewer moving parts - we won't have to shell out to anything. But aside from that, I think we would want a deterministic way for the user to indicate which dart version to format with (in this PR, see: protoc_plugin/lib/src/formatter.dart).

• we could hard-code the version in the source code, and update that as the min. sdk version of package:protoc_plugin changes
• we could assume that package:protoc_plugin has been added as a dev dependency of the generation target, and read the min. sdk version from that pubspec. that would mean we'd get the same formatting version as the rest of the target package's source
• format using 'dart format' w/ a cwd of the generation output directory
• find the nearest pubspec to the generation output directory, read the min sdk version, and use that version for the formatter

[devoncarew]

After thinking about this a bit more, I updated this PR (09eaaa8) to always format using the Dart version from the min. sdk constraint in the protoc_plugin pubspec. This:

• is predictable for users; a particular version of protoc_plugin will always format the same
• doesn't require us to plumb a new parameter through the dart plugin options, or, require the user to know about a new option
• means we don't have to do something potentially unpredictable, like looking around the filesystem in the output directory for a parent pubspec file, and reading the sdk constraint version from that

If its necessary for a particular use of protoc_plugin to have the files formatted at a specific version, they can always generate+format as part of their regular regeneration process.

[osa1]

Btw, I asked around a little bit and found this prior art on formatting generated codes:

• dart-lang/build#3887
• dart-lang/build#3812

There's also a build_runner package for Dart protos: https://github.com/pikaju/dart-protoc-builder

I don't know if it does formatting itself.

[sigurdm]

This is an interesting problem. It would be cool if we had some way of "anchoring" the annotated locations in the source.

Something like https://gcc.gnu.org/onlinedocs/cpp/Line-Control.html#Line-Control-1 ...

@lrhn wdyt?

[devoncarew]

Hmm, I think that would be interesting. I have in the past (when absolutely necessary) 'recovered' locations post format by:

• generating code and recording the location something was generated at
• formatting the code
• recognizing that the formatter is only ever changing whitespace; recover the new post-format location by counting the number of non-whitespace chars from the start of the file

Separately, I think this location annotation code only exists for generating files used by kythe. I don't know if those are used anymore? Kythe has a dart parser, so not sure why we need to generate separate symbol location files for it. 🤷