diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index 0bda720d5..15a7c6207 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -1,8 +1,23 @@ ## 21.1.0-dev * Generate code comments for annotated protobuf inputs. ([#161]) +* Generate message constructor arguments by default again. New flag + `disable_constructor_args` disables generating the arguments. + + Constructor arguments were removed in 21.0.0 as they increase dart2js binary + sizes even when the arguments are not used. + + Example usage to disable constructor arguments: + + ``` + protoc --dart_out='disable_constructor_args,:.' ... + ``` + + ([#850], [#855]) [#161]: https://github.com/google/protobuf.dart/issues/161 +[#850]: https://github.com/google/protobuf.dart/issues/850 +[#855]: https://github.com/google/protobuf.dart/pull/855 ## 21.0.2 diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile index a563cf846..69bca65d2 100644 --- a/protoc_plugin/Makefile +++ b/protoc_plugin/Makefile @@ -78,13 +78,25 @@ PREGENERATED_SRCS=protos/descriptor.proto protos/plugin.proto protos/dart_option $(TEST_PROTO_LIBS): $(PLUGIN_SRC) $(TEST_PROTO_SRCS) mkdir -p $(TEST_PROTO_DIR) + + protoc\ + --experimental_allow_proto3_optional\ + --dart_out="disable_constructor_args:$(TEST_PROTO_DIR)"\ + -Iprotos\ + -I$(TEST_PROTO_SRC_DIR)\ + --plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH))\ + $(TEST_PROTO_SRCS) + + mkdir -p $(TEST_PROTO_DIR)/constructor_args + protoc\ --experimental_allow_proto3_optional\ - --dart_out=$(TEST_PROTO_DIR)\ + --dart_out="$(TEST_PROTO_DIR)/constructor_args"\ -Iprotos\ -I$(TEST_PROTO_SRC_DIR)\ --plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH))\ $(TEST_PROTO_SRCS) + dart format $(TEST_PROTO_DIR) update-pregenerated: $(PLUGIN_PATH) $(PREGENERATED_SRCS) @@ -97,11 +109,11 @@ update-pregenerated: $(PLUGIN_PATH) $(PREGENERATED_SRCS) protos: $(PLUGIN_PATH) $(TEST_PROTO_LIBS) run-tests: protos - dart run test + dart test update-goldens: protos rm -rf test/goldens - dart run test + dart test clean: rm -rf $(OUTPUT_DIR) diff --git a/protoc_plugin/lib/src/base_type.dart b/protoc_plugin/lib/src/base_type.dart index 8a0080aaa..6cc7ae5b0 100644 --- a/protoc_plugin/lib/src/base_type.dart +++ b/protoc_plugin/lib/src/base_type.dart @@ -56,6 +56,9 @@ class BaseType { String getRepeatedDartType(FileGenerator fileGen) => '$coreImportPrefix.List<${getDartType(fileGen)}>'; + String getRepeatedDartTypeIterable(FileGenerator fileGen) => + '$coreImportPrefix.Iterable<${getDartType(fileGen)}>'; + factory BaseType(FieldDescriptorProto field, GenerationContext ctx) { String constSuffix; diff --git a/protoc_plugin/lib/src/message_generator.dart b/protoc_plugin/lib/src/message_generator.dart index 832c4e840..fca4a33c4 100644 --- a/protoc_plugin/lib/src/message_generator.dart +++ b/protoc_plugin/lib/src/message_generator.dart @@ -330,7 +330,8 @@ class MessageGenerator extends ProtobufContainer { NamedLocation( name: classname, fieldPathSegment: fieldPath, start: 'class '.length) ], () { - out.println('factory $classname() => create();'); + _generateFactory(out); + out.printlnAnnotated('$classname._() : super();', [ NamedLocation(name: classname, fieldPathSegment: fieldPath, start: 0) ]); @@ -430,6 +431,42 @@ class MessageGenerator extends ProtobufContainer { out.println(); } + void _generateFactory(IndentingWriter out) { + if (!fileGen.options.disableConstructorArgs && _fieldList.isNotEmpty) { + out.println('factory $classname({'); + for (final field in _fieldList) { + _emitDeprecatedIf(field.isDeprecated, out); + if (field.isRepeated && !field.isMapField) { + out.println( + ' ${field.baseType.getRepeatedDartTypeIterable(fileGen)}? ${field.memberNames!.fieldName},'); + } else { + out.println( + ' ${field.getDartType()}? ${field.memberNames!.fieldName},'); + } + } + out.println('}) {'); + out.println(' final result = create();'); + for (final field in _fieldList) { + out.println(' if (${field.memberNames!.fieldName} != null) {'); + if (field.isDeprecated) { + out.println(' // ignore: deprecated_member_use_from_same_package'); + } + if (field.isRepeated || field.isMapField) { + out.println( + ' result.${field.memberNames!.fieldName}.addAll(${field.memberNames!.fieldName});'); + } else { + out.println( + ' result.${field.memberNames!.fieldName} = ${field.memberNames!.fieldName};'); + } + out.println(' }'); + } + out.println(' return result;'); + out.println('}'); + } else { + out.println('factory $classname() => create();'); + } + } + // Returns true if the message type has any required fields. If it doesn't, // we can optimize out calls to its isInitialized()/_findInvalidFields() // methods. diff --git a/protoc_plugin/lib/src/options.dart b/protoc_plugin/lib/src/options.dart index c66969503..25b960b26 100644 --- a/protoc_plugin/lib/src/options.dart +++ b/protoc_plugin/lib/src/options.dart @@ -50,11 +50,12 @@ bool genericOptionsParser(CodeGeneratorRequest request, class GenerationOptions { final bool useGrpc; final bool generateMetadata; + final bool disableConstructorArgs; - GenerationOptions({ - this.useGrpc = false, - this.generateMetadata = false, - }); + GenerationOptions( + {this.useGrpc = false, + this.generateMetadata = false, + this.disableConstructorArgs = false}); } /// A parser for a name-value pair option. Options parsed in @@ -87,13 +88,26 @@ class GenerateMetadataParser implements SingleOptionParser { @override void parse(String name, String? value, OnError onError) { if (value != null) { - onError('Invalid metadata option. No value expected.'); + onError('Invalid generate_kythe_info option. No value expected.'); return; } generateKytheInfo = true; } } +class DisableConstructorArgsParser implements SingleOptionParser { + bool value = false; + + @override + void parse(String name, String? value, OnError onError) { + if (value != null) { + onError('Invalid disable_constructor_args option. No value expected.'); + return; + } + this.value = true; + } +} + /// Parser used by the compiler, which supports the `rpc` option (see /// [GrpcOptionParser]) and any additional option added in [parsers]. If /// [parsers] has a key for `rpc`, it will be ignored. @@ -105,14 +119,18 @@ GenerationOptions? parseGenerationOptions( final grpcOptionParser = GrpcOptionParser(); newParsers['grpc'] = grpcOptionParser; + final generateMetadataParser = GenerateMetadataParser(); newParsers['generate_kythe_info'] = generateMetadataParser; + final disableConstructorArgsParser = DisableConstructorArgsParser(); + newParsers['disable_constructor_args'] = disableConstructorArgsParser; + if (genericOptionsParser(request, response, newParsers)) { return GenerationOptions( - useGrpc: grpcOptionParser.grpcEnabled, - generateMetadata: generateMetadataParser.generateKytheInfo, - ); + useGrpc: grpcOptionParser.grpcEnabled, + generateMetadata: generateMetadataParser.generateKytheInfo, + disableConstructorArgs: disableConstructorArgsParser.value); } return null; } diff --git a/protoc_plugin/test/file_generator_test.dart b/protoc_plugin/test/file_generator_test.dart index f739cbc52..f26d8d5de 100644 --- a/protoc_plugin/test/file_generator_test.dart +++ b/protoc_plugin/test/file_generator_test.dart @@ -93,7 +93,8 @@ void main() { () { final fd = buildFileDescriptor(); final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); expectMatchesGoldenFile( @@ -103,7 +104,8 @@ void main() { test('FileGenerator outputs a .pb.dart file for an Int64 message', () { final fd = createInt64Proto(); final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); expectMatchesGoldenFile( @@ -115,7 +117,8 @@ void main() { () { final fd = buildFileDescriptor(); final options = parseGenerationOptions( - CodeGeneratorRequest()..parameter = 'generate_kythe_info', + CodeGeneratorRequest() + ..parameter = 'generate_kythe_info,disable_constructor_args', CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); @@ -127,7 +130,8 @@ void main() { () { final fd = buildFileDescriptor(); final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); expectMatchesGoldenFile( @@ -137,7 +141,8 @@ void main() { test('FileGenerator generates files for a top-level enum', () { final fd = buildFileDescriptor(phoneNumber: false, topLevelEnum: true); final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); @@ -150,7 +155,8 @@ void main() { test('FileGenerator generates metadata files for a top-level enum', () { final fd = buildFileDescriptor(phoneNumber: false, topLevelEnum: true); final options = parseGenerationOptions( - CodeGeneratorRequest()..parameter = 'generate_kythe_info', + CodeGeneratorRequest() + ..parameter = 'generate_kythe_info,disable_constructor_args', CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); @@ -164,7 +170,8 @@ void main() { test('FileGenerator generates a .pbjson.dart file for a top-level enum', () { final fd = buildFileDescriptor(phoneNumber: false, topLevelEnum: true); final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); @@ -176,7 +183,8 @@ void main() { final fd = buildFileDescriptor(); fd.package = 'pb_library'; final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); @@ -201,7 +209,8 @@ void main() { ])); final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); @@ -228,7 +237,8 @@ void main() { ..service.add(sd); final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); link(options, [fg]); @@ -415,7 +425,8 @@ void main() { ..name = 'test.proto' ..messageType.add(md); fd.dependency.addAll(['package1.proto', 'package2.proto']); - final request = CodeGeneratorRequest(); + final request = CodeGeneratorRequest() + ..parameter = 'disable_constructor_args'; final response = CodeGeneratorResponse(); final options = parseGenerationOptions(request, response)!; diff --git a/protoc_plugin/test/generated_message_test.dart b/protoc_plugin/test/generated_message_test.dart index c859b5b14..a25f6a4d4 100644 --- a/protoc_plugin/test/generated_message_test.dart +++ b/protoc_plugin/test/generated_message_test.dart @@ -5,6 +5,10 @@ import 'package:protobuf/protobuf.dart'; import 'package:test/test.dart'; +import '../out/protos/constructor_args/google/protobuf/unittest.pb.dart' + as constructor_args_unittest; +import '../out/protos/constructor_args/google/protobuf/unittest_import.pb.dart' + as constructor_args_unittest_import; import '../out/protos/duplicate_names_import.pb.dart'; import '../out/protos/google/protobuf/unittest.pb.dart'; import '../out/protos/google/protobuf/unittest_import.pb.dart'; @@ -784,4 +788,106 @@ void main() { final value2 = value1.deepCopy(); assertAllExtensionsSet(value2); }); + + test('Named arguments in constructors', () { + final value = constructor_args_unittest.TestAllTypes( + optionalInt32: 101, + optionalInt64: make64(102), + optionalUint32: 103, + optionalUint64: make64(104), + optionalSint32: 105, + optionalSint64: make64(106), + optionalFixed32: 107, + optionalFixed64: make64(108), + optionalSfixed32: 109, + optionalSfixed64: make64(110), + optionalFloat: 111.0, + optionalDouble: 112.0, + optionalBool: true, + optionalString: '115', + optionalBytes: '116'.codeUnits, + optionalGroup: + constructor_args_unittest.TestAllTypes_OptionalGroup(a: 117), + optionalNestedMessage: + constructor_args_unittest.TestAllTypes_NestedMessage(bb: 118), + optionalForeignMessage: constructor_args_unittest.ForeignMessage(c: 119), + optionalImportMessage: + constructor_args_unittest_import.ImportMessage(d: 120), + optionalNestedEnum: constructor_args_unittest.TestAllTypes_NestedEnum.BAZ, + optionalForeignEnum: constructor_args_unittest.ForeignEnum.FOREIGN_BAZ, + optionalImportEnum: + constructor_args_unittest_import.ImportEnum.IMPORT_BAZ, + optionalStringPiece: '124', + optionalCord: '125', + repeatedInt32: [201, 301], + repeatedInt64: [make64(202), make64(302)], + repeatedUint32: [203, 303], + repeatedUint64: [make64(204), make64(304)], + repeatedSint32: [205, 305], + repeatedSint64: [make64(206), make64(306)], + repeatedFixed32: [207, 307], + repeatedFixed64: [make64(208), make64(308)], + repeatedSfixed32: [209, 309], + repeatedSfixed64: [make64(210), make64(310)], + repeatedFloat: [211.0, 311.0], + repeatedDouble: [212.0, 312.0], + repeatedBool: [true, false], + repeatedString: ['215', '315'], + repeatedBytes: ['216'.codeUnits, '316'.codeUnits], + repeatedGroup: [ + constructor_args_unittest.TestAllTypes_RepeatedGroup(a: 217), + constructor_args_unittest.TestAllTypes_RepeatedGroup(a: 317) + ], + repeatedNestedMessage: [ + constructor_args_unittest.TestAllTypes_NestedMessage(bb: 218), + constructor_args_unittest.TestAllTypes_NestedMessage(bb: 318) + ], + repeatedForeignMessage: [ + constructor_args_unittest.ForeignMessage(c: 219), + constructor_args_unittest.ForeignMessage(c: 319) + ], + repeatedImportMessage: [ + constructor_args_unittest_import.ImportMessage(d: 220), + constructor_args_unittest_import.ImportMessage(d: 320) + ], + repeatedNestedEnum: [ + constructor_args_unittest.TestAllTypes_NestedEnum.BAR, + constructor_args_unittest.TestAllTypes_NestedEnum.BAZ + ], + repeatedForeignEnum: [ + constructor_args_unittest.ForeignEnum.FOREIGN_BAR, + constructor_args_unittest.ForeignEnum.FOREIGN_BAZ + ], + repeatedImportEnum: [ + constructor_args_unittest_import.ImportEnum.IMPORT_BAR, + constructor_args_unittest_import.ImportEnum.IMPORT_BAZ + ], + repeatedStringPiece: ['224', '324'], + repeatedCord: ['225', '325'], + defaultInt32: 401, + defaultInt64: make64(402), + defaultUint32: 403, + defaultUint64: make64(404), + defaultSint32: 405, + defaultSint64: make64(406), + defaultFixed32: 407, + defaultFixed64: make64(408), + defaultSfixed32: 409, + defaultSfixed64: make64(410), + defaultFloat: 411.0, + defaultDouble: 412.0, + defaultBool: false, + defaultString: '415', + defaultBytes: '416'.codeUnits, + defaultNestedEnum: constructor_args_unittest.TestAllTypes_NestedEnum.FOO, + defaultForeignEnum: constructor_args_unittest.ForeignEnum.FOREIGN_FOO, + defaultImportEnum: constructor_args_unittest_import.ImportEnum.IMPORT_FOO, + defaultStringPiece: '424', + defaultCord: '425', + ); + + // Convert the message with constructor arguments to the message without + // constructor arguments, to be able to reuse `assertAllFieldsSet`. + assertAllFieldsSet(TestAllTypes.fromBuffer(value.writeToBuffer())); + }); } diff --git a/protoc_plugin/test/message_generator_test.dart b/protoc_plugin/test/message_generator_test.dart index 0728e1498..c25fa0ec0 100644 --- a/protoc_plugin/test/message_generator_test.dart +++ b/protoc_plugin/test/message_generator_test.dart @@ -74,7 +74,8 @@ void main() { }); test('testMessageGenerator', () { final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); final mg = MessageGenerator.topLevel(md, fg, {}, null, {}, 0); @@ -99,7 +100,8 @@ void main() { test('testMetadataIndices', () { final options = parseGenerationOptions( - CodeGeneratorRequest(), CodeGeneratorResponse())!; + CodeGeneratorRequest()..parameter = 'disable_constructor_args', + CodeGeneratorResponse())!; final fg = FileGenerator(fd, options); final mg = MessageGenerator.topLevel(md, fg, {}, null, {}, 0);