From 61af2f4a2d48828bb1031aa63ed946d04d3c0332 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 10 Jun 2025 11:47:30 -0700 Subject: [PATCH 1/4] update the generator to emit formatted files --- .github/workflows/dart.yml | 12 ++--- protoc_plugin/CHANGELOG.md | 1 + protoc_plugin/Makefile | 3 -- protoc_plugin/lib/indenting_writer.dart | 51 ++++++++++++------- protoc_plugin/lib/src/file_generator.dart | 24 ++++++--- protoc_plugin/lib/src/formatter.dart | 14 +++++ protoc_plugin/lib/src/options.dart | 9 ++-- protoc_plugin/mono_pkg.yaml | 2 +- protoc_plugin/pubspec.yaml | 4 +- protoc_plugin/test/client_generator_test.dart | 2 +- protoc_plugin/test/const_generator_test.dart | 2 +- protoc_plugin/test/enum_generator_test.dart | 5 +- .../test/extension_generator_test.dart | 13 ++--- protoc_plugin/test/file_generator_test.dart | 38 ++++++++------ protoc_plugin/test/goldens/enum.pbenum.dart | 15 +++--- protoc_plugin/test/goldens/extension.pb.dart | 14 ++--- .../test/goldens/messageGenerator.pb.dart | 38 +++++--------- .../goldens/messageGeneratorEnums.pb.dart | 22 +++----- protoc_plugin/test/indenting_writer_test.dart | 11 ++-- .../test/message_generator_test.dart | 15 +++--- .../test/service_generator_test.dart | 3 +- protoc_plugin/test/src/golden_file.dart | 15 ------ tool/ci.sh | 6 ++- 23 files changed, 160 insertions(+), 159 deletions(-) create mode 100644 protoc_plugin/lib/src/formatter.dart diff --git a/.github/workflows/dart.yml b/.github/workflows/dart.yml index 970b7fdf8..f7c53e3f6 100644 --- a/.github/workflows/dart.yml +++ b/.github/workflows/dart.yml @@ -80,7 +80,7 @@ jobs: uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 with: path: "~/.pub-cache/hosted" - key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:benchmarks;commands:format-command_0-command_1-analyze_0" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:benchmarks;commands:format_0-command_0-command_1-analyze_0" restore-keys: | os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:benchmarks os:ubuntu-latest;pub-cache-hosted;sdk:dev @@ -122,7 +122,7 @@ jobs: uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 with: path: "~/.pub-cache/hosted" - key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protobuf;commands:format-analyze_0" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protobuf;commands:format_0-analyze_0" restore-keys: | os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protobuf os:ubuntu-latest;pub-cache-hosted;sdk:dev @@ -149,14 +149,14 @@ jobs: if: "always() && steps.protobuf_pub_upgrade.conclusion == 'success'" working-directory: protobuf job_005: - name: "format_analyze; linux; Dart dev; PKG: protoc_plugin; `dart format --output=none --set-exit-if-changed .`, `./../tool/setup.sh`, `make protos`, `dart analyze --fatal-infos`" + name: "format_analyze; linux; Dart dev; PKG: protoc_plugin; `dart format --output=none --set-exit-if-changed lib`, `./../tool/setup.sh`, `make protos`, `dart analyze --fatal-infos`" runs-on: ubuntu-latest steps: - name: Cache Pub hosted dependencies uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 with: path: "~/.pub-cache/hosted" - key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin;commands:format-command_0-command_2-analyze_0" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin;commands:format_1-command_0-command_2-analyze_0" restore-keys: | os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin os:ubuntu-latest;pub-cache-hosted;sdk:dev @@ -174,8 +174,8 @@ jobs: run: dart pub upgrade if: "always() && steps.checkout.conclusion == 'success'" working-directory: protoc_plugin - - name: "protoc_plugin; dart format --output=none --set-exit-if-changed ." - run: "dart format --output=none --set-exit-if-changed ." + - name: "protoc_plugin; dart format --output=none --set-exit-if-changed lib" + run: "dart format --output=none --set-exit-if-changed lib" if: "always() && steps.protoc_plugin_pub_upgrade.conclusion == 'success'" working-directory: protoc_plugin - name: protoc_plugin; ./../tool/setup.sh diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index 6860df557..7b342a52b 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -1,5 +1,6 @@ ## 22.4.0-wip +* Generated files are now formatted using the Dart formatter. * Update how we calculate import prefixes ([#1010]); import prefixes are now unique per-library instead of being unique across all generated libraries. * Ignore `unused_import` diagnostics for `*.pbjson.dart` files. diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile index 3be250119..217fdfd84 100644 --- a/protoc_plugin/Makefile +++ b/protoc_plugin/Makefile @@ -89,14 +89,11 @@ $(TEST_PROTO_LIBS): $(PLUGIN_SRC) $(TEST_PROTO_SRCS) --plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH))\ $(TEST_PROTO_SRCS) - dart format $(TEST_PROTO_DIR) - update-pregenerated: $(PLUGIN_PATH) $(PREGENERATED_SRCS) protoc --dart_out=lib/src/gen \ -Iprotos \ --plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH)) $(PREGENERATED_SRCS) find lib/src/gen -name '*.pbjson.dart' -delete - dart format lib/src/gen protos: $(PLUGIN_PATH) $(TEST_PROTO_LIBS) diff --git a/protoc_plugin/lib/indenting_writer.dart b/protoc_plugin/lib/indenting_writer.dart index b2b5808ff..d817fb77f 100644 --- a/protoc_plugin/lib/indenting_writer.dart +++ b/protoc_plugin/lib/indenting_writer.dart @@ -4,6 +4,7 @@ import 'dart:collection'; +import 'src/formatter.dart' as formatter; import 'src/gen/google/protobuf/descriptor.pb.dart'; /// Specifies code locations where metadata annotations should be attached and @@ -13,14 +14,18 @@ class NamedLocation { final List fieldPathSegment; final int start; - NamedLocation( - {required this.name, - required this.fieldPathSegment, - required this.start}); + NamedLocation({ + required this.name, + required this.fieldPathSegment, + required this.start, + }); } /// A buffer for writing indented source code. class IndentingWriter { + final String? fileName; + final bool generateMetadata; + final StringBuffer _buffer = StringBuffer(); final GeneratedCodeInfo sourceLocationInfo = GeneratedCodeInfo(); @@ -29,12 +34,14 @@ class IndentingWriter { // After writing any chunk, _previousOffset is the size of everything that was // written to the buffer before the latest call to print or addBlock. int _previousOffset = 0; - final String? _sourceFile; // Named text sections to write at the end of the file. final Map _suffixes = SplayTreeMap(); - IndentingWriter({String? filename}) : _sourceFile = filename; + IndentingWriter({ + this.fileName, + this.generateMetadata = false, + }); /// Appends a string indented to the current level. /// (Indentation will be added after newline characters where needed.) @@ -118,10 +125,11 @@ class IndentingWriter { _suffixes[suffixKey] = text; } - @override - String toString() { + /// Emit the generated source. + /// + /// This is safe to call multiple times. + String emitSource({required bool format}) { if (_suffixes.isNotEmpty) { - // TODO: We may want to introduce the notion of closing the writer. println(''); for (final key in _suffixes.keys) { println(_suffixes[key]!); @@ -129,7 +137,15 @@ class IndentingWriter { _suffixes.clear(); } - return _buffer.toString(); + var source = _buffer.toString(); + + // We don't always want to format the source (for example, we don't want to + // format if we're creating annotated locations of source elements). + if (format) { + source = formatter.format(source); + } + + return source; } /// Writes part of a line of text. @@ -155,15 +171,14 @@ class IndentingWriter { /// string that was passed to the previous [print]. Name should be the string /// that was written to file. void _addAnnotation(List fieldPath, String name, int start) { - if (_sourceFile == null) { - return; + if (generateMetadata) { + final annotation = GeneratedCodeInfo_Annotation() + ..path.addAll(fieldPath) + ..sourceFile = fileName! + ..begin = _previousOffset + start + ..end = _previousOffset + start + name.length; + sourceLocationInfo.annotation.add(annotation); } - final annotation = GeneratedCodeInfo_Annotation() - ..path.addAll(fieldPath) - ..sourceFile = _sourceFile - ..begin = _previousOffset + start - ..end = _previousOffset + start + name.length; - sourceLocationInfo.annotation.add(annotation); } } diff --git a/protoc_plugin/lib/src/file_generator.dart b/protoc_plugin/lib/src/file_generator.dart index a78495817..774714a8e 100644 --- a/protoc_plugin/lib/src/file_generator.dart +++ b/protoc_plugin/lib/src/file_generator.dart @@ -234,14 +234,17 @@ class FileGenerator extends ProtobufContainer { final mainWriter = generateMainFile(config); final enumWriter = generateEnumFile(config); + final generateMetadata = options.generateMetadata; + final files = [ - makeFile('.pb.dart', mainWriter.toString()), - makeFile('.pbenum.dart', enumWriter.toString()), + makeFile('.pb.dart', mainWriter.emitSource(format: !generateMetadata)), + makeFile( + '.pbenum.dart', enumWriter.emitSource(format: !generateMetadata)), // TODO(devoncarew): Consider not emitting empty json files. makeFile('.pbjson.dart', generateJsonFile(config)), ]; - if (options.generateMetadata) { + if (generateMetadata) { files.addAll([ makeFile('.pb.dart.meta', mainWriter.sourceLocationInfo.writeToJson().toString()), @@ -258,12 +261,17 @@ class FileGenerator extends ProtobufContainer { files.add(makeFile('.pbserver.dart', generateServerFile(config))); } } + return files; } /// Creates an IndentingWriter with metadata generation enabled or disabled. - IndentingWriter makeWriter() => IndentingWriter( - filename: options.generateMetadata ? descriptor.name : null); + IndentingWriter makeWriter() { + return IndentingWriter( + fileName: descriptor.name, + generateMetadata: options.generateMetadata, + ); + } /// Returns the contents of the .pb.dart file for this .proto file. IndentingWriter generateMainFile( @@ -509,7 +517,7 @@ class FileGenerator extends ProtobufContainer { s.generate(out); } - return out.toString(); + return out.emitSource(format: true); } /// Returns the contents of the .pbgrpc.dart file for this .proto file. @@ -545,7 +553,7 @@ class FileGenerator extends ProtobufContainer { generator.generate(out); } - return out.toString(); + return out.emitSource(format: true); } void writeBinaryDescriptor(IndentingWriter out, String identifierName, @@ -615,7 +623,7 @@ class FileGenerator extends ProtobufContainer { out.println(''); } - return out.toString(); + return out.emitSource(format: true); } /// Returns the generator for each .pbjson.dart file the generated diff --git a/protoc_plugin/lib/src/formatter.dart b/protoc_plugin/lib/src/formatter.dart new file mode 100644 index 000000000..200e8b9b4 --- /dev/null +++ b/protoc_plugin/lib/src/formatter.dart @@ -0,0 +1,14 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:dart_style/dart_style.dart'; +import 'package:pub_semver/pub_semver.dart'; + +final Version formatUsingVersion = Version(3, 6, 0); + +final DartFormatter _formatter = + DartFormatter(languageVersion: formatUsingVersion); + +/// Return the Dart formatted version of the given source. +String format(String source) => _formatter.format(source); diff --git a/protoc_plugin/lib/src/options.dart b/protoc_plugin/lib/src/options.dart index a923373b2..995e64ee0 100644 --- a/protoc_plugin/lib/src/options.dart +++ b/protoc_plugin/lib/src/options.dart @@ -52,10 +52,11 @@ class GenerationOptions { final bool generateMetadata; final bool disableConstructorArgs; - GenerationOptions( - {this.useGrpc = false, - this.generateMetadata = false, - this.disableConstructorArgs = false}); + GenerationOptions({ + this.useGrpc = false, + this.generateMetadata = false, + this.disableConstructorArgs = false, + }); } /// A parser for a name-value pair option. Options parsed in diff --git a/protoc_plugin/mono_pkg.yaml b/protoc_plugin/mono_pkg.yaml index d9625f261..01f1e8980 100644 --- a/protoc_plugin/mono_pkg.yaml +++ b/protoc_plugin/mono_pkg.yaml @@ -2,7 +2,7 @@ stages: - format_analyze: - group: - - format + - format: --output=none --set-exit-if-changed lib - command: ./../tool/setup.sh - command: make protos - analyze: --fatal-infos diff --git a/protoc_plugin/pubspec.yaml b/protoc_plugin/pubspec.yaml index 91d89a9f2..b5076fc95 100644 --- a/protoc_plugin/pubspec.yaml +++ b/protoc_plugin/pubspec.yaml @@ -10,14 +10,14 @@ resolution: workspace dependencies: collection: ^1.15.0 + dart_style: ^3.0.0 fixnum: ^1.0.0 path: ^1.8.0 protobuf: ^4.1.0 + pub_semver: ^2.2.0 dev_dependencies: - dart_style: ^3.0.0 lints: '>=5.0.0 <7.0.0' - pub_semver: ^2.2.0 test: ^1.16.0 executables: diff --git a/protoc_plugin/test/client_generator_test.dart b/protoc_plugin/test/client_generator_test.dart index 88f617444..a0d697c77 100644 --- a/protoc_plugin/test/client_generator_test.dart +++ b/protoc_plugin/test/client_generator_test.dart @@ -29,6 +29,6 @@ void main() { final writer = IndentingWriter(); cag.generate(writer); - expectGolden(writer.toString(), 'client.pb.dart'); + expectGolden(writer.emitSource(format: true), 'client.pb.dart'); }); } diff --git a/protoc_plugin/test/const_generator_test.dart b/protoc_plugin/test/const_generator_test.dart index 2e423a76a..831dc26b5 100644 --- a/protoc_plugin/test/const_generator_test.dart +++ b/protoc_plugin/test/const_generator_test.dart @@ -9,7 +9,7 @@ import 'package:test/test.dart'; String toConst(Object? val) { final out = IndentingWriter(); writeJsonConst(out, val); - return out.toString(); + return out.emitSource(format: false); } void main() { diff --git a/protoc_plugin/test/enum_generator_test.dart b/protoc_plugin/test/enum_generator_test.dart index 26a1d707a..84bc68a30 100644 --- a/protoc_plugin/test/enum_generator_test.dart +++ b/protoc_plugin/test/enum_generator_test.dart @@ -28,11 +28,12 @@ void main() { ..name = 'BUSINESS' ..number = 2 ]); - final writer = IndentingWriter(filename: 'sample.proto'); + final writer = + IndentingWriter(generateMetadata: true, fileName: 'sample.proto'); final fg = FileGenerator(FileDescriptorProto(), GenerationOptions()); final eg = EnumGenerator.topLevel(ed, fg, {}, 0); eg.generate(writer); - expectGolden(writer.toString(), 'enum.pbenum.dart'); + expectGolden(writer.emitSource(format: false), 'enum.pbenum.dart'); expectGolden(writer.sourceLocationInfo.toString(), 'enum.pbenum.dart.meta'); }); } diff --git a/protoc_plugin/test/extension_generator_test.dart b/protoc_plugin/test/extension_generator_test.dart index a87083b1a..7f3c5cba1 100644 --- a/protoc_plugin/test/extension_generator_test.dart +++ b/protoc_plugin/test/extension_generator_test.dart @@ -33,18 +33,11 @@ void main() { final options = parseGenerationOptions( pb.CodeGeneratorRequest(), pb.CodeGeneratorResponse()); link(options, [fileGenerator]); - final writer = IndentingWriter(filename: 'sample.proto'); + final writer = + IndentingWriter(generateMetadata: true, fileName: 'sample.proto'); fileGenerator.extensionGenerators.single.generate(writer); - // We wrap the output in a dummy class in order to create a valid - // (formattable) Dart file. - var actual = writer.toString(); - actual = ''' -class Card { - $actual -} -'''; - + final actual = writer.emitSource(format: false); expectGolden(actual, 'extension.pb.dart'); expectGolden( writer.sourceLocationInfo.toString(), 'extension.pb.dart.meta'); diff --git a/protoc_plugin/test/file_generator_test.dart b/protoc_plugin/test/file_generator_test.dart index 632f21444..150b39119 100644 --- a/protoc_plugin/test/file_generator_test.dart +++ b/protoc_plugin/test/file_generator_test.dart @@ -98,7 +98,8 @@ void main() { CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); - expectGolden(fg.generateMainFile().toString(), 'oneMessage.pb.dart'); + expectGolden( + fg.generateMainFile().emitSource(format: true), 'oneMessage.pb.dart'); }); test('FileGenerator outputs a .pb.dart file for an Int64 message', () { @@ -108,7 +109,8 @@ void main() { CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); - expectGolden(fg.generateMainFile().toString(), 'int64.pb.dart'); + expectGolden( + fg.generateMainFile().emitSource(format: true), 'int64.pb.dart'); }); test( @@ -144,8 +146,10 @@ void main() { final fg = FileGenerator(fd, options); link(options, [fg]); - expectGolden(fg.generateMainFile().toString(), 'topLevelEnum.pb.dart'); - expectGolden(fg.generateEnumFile().toString(), 'topLevelEnum.pbenum.dart'); + expectGolden( + fg.generateMainFile().emitSource(format: true), 'topLevelEnum.pb.dart'); + expectGolden(fg.generateEnumFile().emitSource(format: true), + 'topLevelEnum.pbenum.dart'); }); test('FileGenerator generates metadata files for a top-level enum', () { @@ -184,9 +188,9 @@ void main() { final fg = FileGenerator(fd, options); link(options, [fg]); - final writer = IndentingWriter(filename: ''); + final writer = IndentingWriter(); fg.writeMainHeader(writer); - expectGolden(writer.toString(), 'header_in_package.pb.dart'); + expectGolden(writer.emitSource(format: true), 'header_in_package.pb.dart'); }); test('FileGenerator outputs a fixnum import when needed', () { @@ -209,9 +213,9 @@ void main() { final fg = FileGenerator(fd, options); link(options, [fg]); - final writer = IndentingWriter(filename: ''); + final writer = IndentingWriter(); fg.writeMainHeader(writer); - expectGolden(writer.toString(), 'header_with_fixnum.pb.dart'); + expectGolden(writer.emitSource(format: true), 'header_with_fixnum.pb.dart'); }); test('FileGenerator outputs files for a service', () { @@ -236,9 +240,10 @@ void main() { final fg = FileGenerator(fd, options); link(options, [fg]); - final writer = IndentingWriter(filename: ''); + final writer = IndentingWriter(); fg.writeMainHeader(writer); - expectGolden(fg.generateMainFile().toString(), 'service.pb.dart'); + expectGolden( + fg.generateMainFile().emitSource(format: true), 'service.pb.dart'); expectGolden(fg.generateServerFile(), 'service.pbserver.dart'); }); @@ -263,9 +268,10 @@ void main() { final fg = FileGenerator(fd, options); link(options, [fg]); - final writer = IndentingWriter(filename: ''); + final writer = IndentingWriter(); fg.writeMainHeader(writer); - expectGolden(fg.generateMainFile().toString(), 'grpc_service.pb.dart'); + expectGolden( + fg.generateMainFile().emitSource(format: true), 'grpc_service.pb.dart'); }); test('FileGenerator outputs gRPC stubs if gRPC is selected', () { @@ -348,7 +354,7 @@ void main() { final fg = FileGenerator(fd, options); link(options, [fg]); - final writer = IndentingWriter(filename: ''); + final writer = IndentingWriter(); fg.writeMainHeader(writer); expectGolden(fg.generateGrpcFile(), 'grpc_service.pbgrpc.dart'); }); @@ -459,7 +465,9 @@ void main() { final fg = FileGenerator(fd, options); link(options, [fg, FileGenerator(fd1, options), FileGenerator(fd2, options)]); - expectGolden(fg.generateMainFile().toString(), 'imports.pb.dart'); - expectGolden(fg.generateEnumFile().toString(), 'imports.pbjson.dart'); + expectGolden( + fg.generateMainFile().emitSource(format: true), 'imports.pb.dart'); + expectGolden( + fg.generateEnumFile().emitSource(format: true), 'imports.pbjson.dart'); }); } diff --git a/protoc_plugin/test/goldens/enum.pbenum.dart b/protoc_plugin/test/goldens/enum.pbenum.dart index de7464f09..809bfedfe 100644 --- a/protoc_plugin/test/goldens/enum.pbenum.dart +++ b/protoc_plugin/test/goldens/enum.pbenum.dart @@ -1,24 +1,21 @@ class PhoneType extends $pb.ProtobufEnum { - static const PhoneType MOBILE = - PhoneType._(0, _omitEnumNames ? '' : 'MOBILE'); + static const PhoneType MOBILE = PhoneType._(0, _omitEnumNames ? '' : 'MOBILE'); static const PhoneType HOME = PhoneType._(1, _omitEnumNames ? '' : 'HOME'); static const PhoneType WORK = PhoneType._(2, _omitEnumNames ? '' : 'WORK'); static const PhoneType BUSINESS = WORK; - static const $core.List values = [ + static const $core.List values = [ MOBILE, HOME, WORK, ]; - static final $core.List _byValue = - $pb.ProtobufEnum.$_initByValueList(values, 2); - static PhoneType? valueOf($core.int value) => - value < 0 || value >= _byValue.length ? null : _byValue[value]; + static final $core.List _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2); + static PhoneType? valueOf($core.int value) => value < 0 || value >= _byValue.length ? null : _byValue[value]; const PhoneType._(super.value, super.name); } -const $core.bool _omitEnumNames = - $core.bool.fromEnvironment('protobuf.omit_enum_names'); + +const $core.bool _omitEnumNames = $core.bool.fromEnvironment('protobuf.omit_enum_names'); diff --git a/protoc_plugin/test/goldens/extension.pb.dart b/protoc_plugin/test/goldens/extension.pb.dart index 86888691b..32a5dcb75 100644 --- a/protoc_plugin/test/goldens/extension.pb.dart +++ b/protoc_plugin/test/goldens/extension.pb.dart @@ -1,12 +1,4 @@ -class Card { - static final clientInfo = $pb.Extension<$core.String>( - _omitMessageNames ? '' : 'Card', - _omitFieldNames ? '' : 'clientInfo', - 261486461, - $pb.PbFieldType.OS); +static final clientInfo = $pb.Extension<$core.String>(_omitMessageNames ? '' : 'Card', _omitFieldNames ? '' : 'clientInfo', 261486461, $pb.PbFieldType.OS); - const $core.bool _omitFieldNames = - $core.bool.fromEnvironment('protobuf.omit_field_names'); - const $core.bool _omitMessageNames = - $core.bool.fromEnvironment('protobuf.omit_message_names'); -} +const $core.bool _omitFieldNames = $core.bool.fromEnvironment('protobuf.omit_field_names'); +const $core.bool _omitMessageNames = $core.bool.fromEnvironment('protobuf.omit_message_names'); diff --git a/protoc_plugin/test/goldens/messageGenerator.pb.dart b/protoc_plugin/test/goldens/messageGenerator.pb.dart index 6871bed3b..15d05a340 100644 --- a/protoc_plugin/test/goldens/messageGenerator.pb.dart +++ b/protoc_plugin/test/goldens/messageGenerator.pb.dart @@ -3,32 +3,20 @@ class PhoneNumber extends $pb.GeneratedMessage { PhoneNumber._(); - factory PhoneNumber.fromBuffer($core.List<$core.int> data, - [$pb.ExtensionRegistry registry = $pb.ExtensionRegistry.EMPTY]) => - create()..mergeFromBuffer(data, registry); - factory PhoneNumber.fromJson($core.String json, - [$pb.ExtensionRegistry registry = $pb.ExtensionRegistry.EMPTY]) => - create()..mergeFromJson(json, registry); + factory PhoneNumber.fromBuffer($core.List<$core.int> data, [$pb.ExtensionRegistry registry = $pb.ExtensionRegistry.EMPTY]) => create()..mergeFromBuffer(data, registry); + factory PhoneNumber.fromJson($core.String json, [$pb.ExtensionRegistry registry = $pb.ExtensionRegistry.EMPTY]) => create()..mergeFromJson(json, registry); - static final $pb.BuilderInfo _i = $pb.BuilderInfo( - _omitMessageNames ? '' : 'PhoneNumber', - createEmptyInstance: create) + static final $pb.BuilderInfo _i = $pb.BuilderInfo(_omitMessageNames ? '' : 'PhoneNumber', createEmptyInstance: create) ..aQS(1, _omitFieldNames ? '' : 'number') - ..e( - 2, _omitFieldNames ? '' : 'type', $pb.PbFieldType.OE, - defaultOrMaker: PhoneNumber_PhoneType.MOBILE, - valueOf: PhoneNumber_PhoneType.valueOf, - enumValues: PhoneNumber_PhoneType.values) - ..a<$core.String>(3, _omitFieldNames ? '' : 'name', $pb.PbFieldType.OS, - defaultOrMaker: '\$') - ..aOS(4, _omitFieldNames ? '' : 'deprecatedField'); + ..e(2, _omitFieldNames ? '' : 'type', $pb.PbFieldType.OE, defaultOrMaker: PhoneNumber_PhoneType.MOBILE, valueOf: PhoneNumber_PhoneType.valueOf, enumValues: PhoneNumber_PhoneType.values) + ..a<$core.String>(3, _omitFieldNames ? '' : 'name', $pb.PbFieldType.OS, defaultOrMaker: '\$') + ..aOS(4, _omitFieldNames ? '' : 'deprecatedField') + ; @$core.Deprecated('See https://github.com/google/protobuf.dart/issues/998.') PhoneNumber clone() => PhoneNumber()..mergeFromMessage(this); @$core.Deprecated('See https://github.com/google/protobuf.dart/issues/998.') - PhoneNumber copyWith(void Function(PhoneNumber) updates) => - super.copyWith((message) => updates(message as PhoneNumber)) - as PhoneNumber; + PhoneNumber copyWith(void Function(PhoneNumber) updates) => super.copyWith((message) => updates(message as PhoneNumber)) as PhoneNumber; @$core.override $pb.BuilderInfo get info_ => _i; @@ -39,8 +27,7 @@ class PhoneNumber extends $pb.GeneratedMessage { PhoneNumber createEmptyInstance() => create(); static $pb.PbList createRepeated() => $pb.PbList(); @$core.pragma('dart2js:noInline') - static PhoneNumber getDefault() => _defaultInstance ??= - $pb.GeneratedMessage.$_defaultFor(create); + static PhoneNumber getDefault() => _defaultInstance ??= $pb.GeneratedMessage.$_defaultFor(create); static PhoneNumber? _defaultInstance; @$pb.TagNumber(1) @@ -84,7 +71,6 @@ class PhoneNumber extends $pb.GeneratedMessage { void clearDeprecatedField() => $_clearField(4); } -const $core.bool _omitFieldNames = - $core.bool.fromEnvironment('protobuf.omit_field_names'); -const $core.bool _omitMessageNames = - $core.bool.fromEnvironment('protobuf.omit_message_names'); + +const $core.bool _omitFieldNames = $core.bool.fromEnvironment('protobuf.omit_field_names'); +const $core.bool _omitMessageNames = $core.bool.fromEnvironment('protobuf.omit_message_names'); diff --git a/protoc_plugin/test/goldens/messageGeneratorEnums.pb.dart b/protoc_plugin/test/goldens/messageGeneratorEnums.pb.dart index 4ac231317..a9ba53433 100644 --- a/protoc_plugin/test/goldens/messageGeneratorEnums.pb.dart +++ b/protoc_plugin/test/goldens/messageGeneratorEnums.pb.dart @@ -1,27 +1,21 @@ class PhoneNumber_PhoneType extends $pb.ProtobufEnum { - static const PhoneNumber_PhoneType MOBILE = - PhoneNumber_PhoneType._(0, _omitEnumNames ? '' : 'MOBILE'); - static const PhoneNumber_PhoneType HOME = - PhoneNumber_PhoneType._(1, _omitEnumNames ? '' : 'HOME'); - static const PhoneNumber_PhoneType WORK = - PhoneNumber_PhoneType._(2, _omitEnumNames ? '' : 'WORK'); + static const PhoneNumber_PhoneType MOBILE = PhoneNumber_PhoneType._(0, _omitEnumNames ? '' : 'MOBILE'); + static const PhoneNumber_PhoneType HOME = PhoneNumber_PhoneType._(1, _omitEnumNames ? '' : 'HOME'); + static const PhoneNumber_PhoneType WORK = PhoneNumber_PhoneType._(2, _omitEnumNames ? '' : 'WORK'); static const PhoneNumber_PhoneType BUSINESS = WORK; - static const $core.List values = - [ + static const $core.List values = [ MOBILE, HOME, WORK, ]; - static final $core.List _byValue = - $pb.ProtobufEnum.$_initByValueList(values, 2); - static PhoneNumber_PhoneType? valueOf($core.int value) => - value < 0 || value >= _byValue.length ? null : _byValue[value]; + static final $core.List _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2); + static PhoneNumber_PhoneType? valueOf($core.int value) => value < 0 || value >= _byValue.length ? null : _byValue[value]; const PhoneNumber_PhoneType._(super.value, super.name); } -const $core.bool _omitEnumNames = - $core.bool.fromEnvironment('protobuf.omit_enum_names'); + +const $core.bool _omitEnumNames = $core.bool.fromEnvironment('protobuf.omit_enum_names'); diff --git a/protoc_plugin/test/indenting_writer_test.dart b/protoc_plugin/test/indenting_writer_test.dart index 6af479758..1c6c62a23 100644 --- a/protoc_plugin/test/indenting_writer_test.dart +++ b/protoc_plugin/test/indenting_writer_test.dart @@ -9,14 +9,14 @@ import 'package:test/test.dart'; void main() { group('IndentingWriter', () { test('IndentingWriter can indent a block', () { - final out = IndentingWriter(filename: ''); + final out = IndentingWriter(); out.addBlock('class test {', '}', () { out.println('first;'); out.println(); out.println('second;'); }); - expect(out.toString(), ''' + expect(out.emitSource(format: false), ''' class test { first; @@ -26,7 +26,8 @@ class test { }); test('IndentingWriter annotation tracks previous output', () { - final out = IndentingWriter(filename: 'sample.proto'); + final out = + IndentingWriter(generateMetadata: true, fileName: 'sample.proto'); out.print('13 characters'); out.printAnnotated('sample text', [ NamedLocation(name: 'text', fieldPathSegment: [1, 2, 3], start: 7) @@ -41,7 +42,7 @@ class test { }); test('IndentingWriter annotation counts indents correctly', () { - final out = IndentingWriter(filename: ''); + final out = IndentingWriter(generateMetadata: true, fileName: ''); out.addBlock('34 characters including newline {', '}', () { out.printlnAnnotated('sample text', [NamedLocation(name: 'sample', fieldPathSegment: [], start: 0)]); @@ -53,7 +54,7 @@ class test { }); test('IndentingWriter annotations counts multiline output correctly', () { - final out = IndentingWriter(filename: ''); + final out = IndentingWriter(generateMetadata: true, fileName: ''); out.print('20 characters\ntotal\n'); out.printlnAnnotated('20 characters before this', [NamedLocation(name: 'ch', fieldPathSegment: [], start: 3)]); diff --git a/protoc_plugin/test/message_generator_test.dart b/protoc_plugin/test/message_generator_test.dart index 749c2b1cb..5bbdb9675 100644 --- a/protoc_plugin/test/message_generator_test.dart +++ b/protoc_plugin/test/message_generator_test.dart @@ -19,6 +19,7 @@ void main() { late FileDescriptorProto fd; EnumDescriptorProto ed; late DescriptorProto md; + setUp(() async { fd = FileDescriptorProto(); ed = EnumDescriptorProto() @@ -72,6 +73,7 @@ void main() { ]) ..enumType.add(ed); }); + test('testMessageGenerator', () { final options = parseGenerationOptions( CodeGeneratorRequest()..parameter = 'disable_constructor_args', @@ -84,15 +86,16 @@ void main() { mg.register(ctx); mg.resolve(ctx); - var writer = IndentingWriter(filename: ''); + var writer = IndentingWriter(generateMetadata: true, fileName: ''); mg.generate(writer); - expectGolden(writer.toString(), 'messageGenerator.pb.dart'); + expectGolden(writer.emitSource(format: false), 'messageGenerator.pb.dart'); expectGolden( writer.sourceLocationInfo.toString(), 'messageGenerator.pb.dart.meta'); - writer = IndentingWriter(filename: ''); + writer = IndentingWriter(generateMetadata: true, fileName: ''); mg.generateEnums(writer); - expectGolden(writer.toString(), 'messageGeneratorEnums.pb.dart'); + expectGolden( + writer.emitSource(format: false), 'messageGeneratorEnums.pb.dart'); expectGolden(writer.sourceLocationInfo.toString(), 'messageGeneratorEnums.pb.dart.meta'); }); @@ -108,7 +111,7 @@ void main() { mg.register(ctx); mg.resolve(ctx); - final writer = IndentingWriter(filename: ''); + final writer = IndentingWriter(generateMetadata: true, fileName: ''); mg.generate(writer); final eq = ListEquality(); @@ -124,7 +127,7 @@ void main() { 'clearDeprecatedField' ]; - final generatedContents = writer.toString(); + final generatedContents = writer.emitSource(format: false); final metadata = writer.sourceLocationInfo; for (final annotation in metadata.annotation) { final annotatedName = diff --git a/protoc_plugin/test/service_generator_test.dart b/protoc_plugin/test/service_generator_test.dart index 706ccfa73..ee407b497 100644 --- a/protoc_plugin/test/service_generator_test.dart +++ b/protoc_plugin/test/service_generator_test.dart @@ -27,7 +27,8 @@ void main() { final serviceWriter = IndentingWriter(); fg.serviceGenerators[0].generate(serviceWriter); - expectGolden(serviceWriter.toString(), 'serviceGenerator.pb.dart'); + expectGolden( + serviceWriter.emitSource(format: true), 'serviceGenerator.pb.dart'); expectGolden(fg.generateJsonFile(), 'serviceGenerator.pbjson.dart'); }); } diff --git a/protoc_plugin/test/src/golden_file.dart b/protoc_plugin/test/src/golden_file.dart index 6b8d2b066..d3526ba86 100644 --- a/protoc_plugin/test/src/golden_file.dart +++ b/protoc_plugin/test/src/golden_file.dart @@ -4,9 +4,7 @@ import 'dart:io'; -import 'package:dart_style/dart_style.dart'; import 'package:path/path.dart' as path; -import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; /// Will test [actual] against the contests of the file at [file]. @@ -18,12 +16,6 @@ void expectGolden(String actual, String file) { goldens.createSync(); } - // TODO(devoncarew): We should move the formatting logic out of this test; - // perhaps into the generating code. - if (file.endsWith('.dart')) { - actual = format(actual); - } - var golden = File(path.join(goldens.path, file)); if (golden.existsSync()) { expect( @@ -40,10 +32,3 @@ void expectGolden(String actual, String file) { golden.writeAsStringSync(actual); } } - -String format(String source) { - // TODO(devoncarew): Move this language version to a central location. - // For tests, this version should match that of package:protoc_plugin. - final formatter = DartFormatter(languageVersion: Version(3, 6, 0)); - return formatter.format(source); -} diff --git a/tool/ci.sh b/tool/ci.sh index 709650627..1b123cce0 100755 --- a/tool/ci.sh +++ b/tool/ci.sh @@ -87,10 +87,14 @@ for PKG in ${PKGS}; do echo 'make protos' make protos || EXIT_CODE=$? ;; - format) + format_0) echo 'dart format --output=none --set-exit-if-changed .' dart format --output=none --set-exit-if-changed . || EXIT_CODE=$? ;; + format_1) + echo 'dart format --output=none --set-exit-if-changed lib' + dart format --output=none --set-exit-if-changed lib || EXIT_CODE=$? + ;; test) echo 'dart test' dart test || EXIT_CODE=$? From 09eaaa851b2e56e4e0f27d81f4ce779708d5e3c9 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 1 Jul 2025 12:46:46 -0700 Subject: [PATCH 2/4] codify how we choose what version to format with --- protoc_plugin/CHANGELOG.md | 5 ++++- protoc_plugin/lib/src/formatter.dart | 1 + protoc_plugin/pubspec.yaml | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index 334dd79aa..400b51cc4 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -1,6 +1,9 @@ -## 22.4.0 +## 22.5.0-wip * Generated files are now formatted using the Dart formatter. + +## 22.4.0 + * Update how we calculate import prefixes ([#1010]); import prefixes are now unique per-library instead of being unique across all generated libraries. * Ignore `unused_import` diagnostics for `*.pbjson.dart` files. ([#1013]) diff --git a/protoc_plugin/lib/src/formatter.dart b/protoc_plugin/lib/src/formatter.dart index 200e8b9b4..3b43b3f87 100644 --- a/protoc_plugin/lib/src/formatter.dart +++ b/protoc_plugin/lib/src/formatter.dart @@ -5,6 +5,7 @@ import 'package:dart_style/dart_style.dart'; import 'package:pub_semver/pub_semver.dart'; +// Note: keep this in sync with the SDK constraint in pubspec.yaml. final Version formatUsingVersion = Version(3, 6, 0); final DartFormatter _formatter = diff --git a/protoc_plugin/pubspec.yaml b/protoc_plugin/pubspec.yaml index abf5bdb7e..a07d79261 100644 --- a/protoc_plugin/pubspec.yaml +++ b/protoc_plugin/pubspec.yaml @@ -1,9 +1,10 @@ name: protoc_plugin -version: 22.4.0 +version: 22.5.0-wip description: A protobuf protoc compiler plugin used to generate Dart code. repository: https://github.com/google/protobuf.dart/tree/master/protoc_plugin environment: + # Note: keep this in sync with the SDK version in lib/src/formatter.dart. sdk: ^3.6.0 resolution: workspace From 8323194f1f8fb3350269524ebb1012dfcf0c2a8f Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 1 Jul 2025 12:56:14 -0700 Subject: [PATCH 3/4] update test goldens --- .../test/goldens/grpc_service.pbgrpc.~dart | 73 ++++++++++++------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/protoc_plugin/test/goldens/grpc_service.pbgrpc.~dart b/protoc_plugin/test/goldens/grpc_service.pbgrpc.~dart index a0968c478..107b83bb2 100644 --- a/protoc_plugin/test/goldens/grpc_service.pbgrpc.~dart +++ b/protoc_plugin/test/goldens/grpc_service.pbgrpc.~dart @@ -33,36 +33,55 @@ class TestClient extends $grpc.Client { TestClient(super.channel, {super.options, super.interceptors}); - $grpc.ResponseFuture<$0.Output> unary($0.Input request, {$grpc.CallOptions? options,}) { + $grpc.ResponseFuture<$0.Output> unary( + $0.Input request, { + $grpc.CallOptions? options, + }) { return $createUnaryCall(_$unary, request, options: options); } - $grpc.ResponseFuture<$0.Output> clientStreaming($async.Stream<$0.Input> request, {$grpc.CallOptions? options,}) { - return $createStreamingCall(_$clientStreaming, request, options: options).single; + $grpc.ResponseFuture<$0.Output> clientStreaming( + $async.Stream<$0.Input> request, { + $grpc.CallOptions? options, + }) { + return $createStreamingCall(_$clientStreaming, request, options: options) + .single; } - $grpc.ResponseStream<$0.Output> serverStreaming($0.Input request, {$grpc.CallOptions? options,}) { - return $createStreamingCall(_$serverStreaming, $async.Stream.fromIterable([request]), options: options); + $grpc.ResponseStream<$0.Output> serverStreaming( + $0.Input request, { + $grpc.CallOptions? options, + }) { + return $createStreamingCall( + _$serverStreaming, $async.Stream.fromIterable([request]), + options: options); } - $grpc.ResponseStream<$0.Output> bidirectional($async.Stream<$0.Input> request, {$grpc.CallOptions? options,}) { + $grpc.ResponseStream<$0.Output> bidirectional( + $async.Stream<$0.Input> request, { + $grpc.CallOptions? options, + }) { return $createStreamingCall(_$bidirectional, request, options: options); } - $grpc.ResponseFuture<$0.Output> call($0.Input request, {$grpc.CallOptions? options,}) { + $grpc.ResponseFuture<$0.Output> call( + $0.Input request, { + $grpc.CallOptions? options, + }) { return $createUnaryCall(_$call, request, options: options); } - $grpc.ResponseFuture<$0.Output> request($0.Input request, {$grpc.CallOptions? options,}) { + $grpc.ResponseFuture<$0.Output> request( + $0.Input request, { + $grpc.CallOptions? options, + }) { return $createUnaryCall(_$request, request, options: options); } - // method descriptors + // method descriptors - static final _$unary = $grpc.ClientMethod<$0.Input, $0.Output>( - '/Test/Unary', - ($0.Input value) => value.writeToBuffer(), - $0.Output.fromBuffer); + static final _$unary = $grpc.ClientMethod<$0.Input, $0.Output>('/Test/Unary', + ($0.Input value) => value.writeToBuffer(), $0.Output.fromBuffer); static final _$clientStreaming = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/ClientStreaming', ($0.Input value) => value.writeToBuffer(), @@ -75,10 +94,8 @@ class TestClient extends $grpc.Client { '/Test/Bidirectional', ($0.Input value) => value.writeToBuffer(), $0.Output.fromBuffer); - static final _$call = $grpc.ClientMethod<$0.Input, $0.Output>( - '/Test/Call', - ($0.Input value) => value.writeToBuffer(), - $0.Output.fromBuffer); + static final _$call = $grpc.ClientMethod<$0.Input, $0.Output>('/Test/Call', + ($0.Input value) => value.writeToBuffer(), $0.Output.fromBuffer); static final _$request = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/Request', ($0.Input value) => value.writeToBuffer(), @@ -134,32 +151,38 @@ abstract class TestServiceBase extends $grpc.Service { ($0.Output value) => value.writeToBuffer())); } - $async.Future<$0.Output> unary_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { + $async.Future<$0.Output> unary_Pre( + $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return unary($call, await $request); } $async.Future<$0.Output> unary($grpc.ServiceCall call, $0.Input request); - $async.Future<$0.Output> clientStreaming($grpc.ServiceCall call, $async.Stream<$0.Input> request); + $async.Future<$0.Output> clientStreaming( + $grpc.ServiceCall call, $async.Stream<$0.Input> request); - $async.Stream<$0.Output> serverStreaming_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async* { + $async.Stream<$0.Output> serverStreaming_Pre( + $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async* { yield* serverStreaming($call, await $request); } - $async.Stream<$0.Output> serverStreaming($grpc.ServiceCall call, $0.Input request); + $async.Stream<$0.Output> serverStreaming( + $grpc.ServiceCall call, $0.Input request); - $async.Stream<$0.Output> bidirectional($grpc.ServiceCall call, $async.Stream<$0.Input> request); + $async.Stream<$0.Output> bidirectional( + $grpc.ServiceCall call, $async.Stream<$0.Input> request); - $async.Future<$0.Output> call_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { + $async.Future<$0.Output> call_Pre( + $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return call($call, await $request); } $async.Future<$0.Output> call($grpc.ServiceCall call, $0.Input request); - $async.Future<$0.Output> request_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { + $async.Future<$0.Output> request_Pre( + $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return request($call, await $request); } $async.Future<$0.Output> request($grpc.ServiceCall call, $0.Input request); - } From 7c46e16f264c5d47e5c7f8023a9769d1dafda7f0 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 15 Jul 2025 09:33:26 -0700 Subject: [PATCH 4/4] update changelog w/ review feedback --- protoc_plugin/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index 400b51cc4..aa6278962 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -1,6 +1,7 @@ ## 22.5.0-wip -* Generated files are now formatted using the Dart formatter. +* Generated files are now formatted using the Dart formatter. The code is + formatted using the min. SDK for `package:protoc_plugin`; currently `3.6.0`. ## 22.4.0