Generate constructor arguments again, add a flag to disable

protoc_plugin/CHANGELOG.md

@@ -2,6 +2,16 @@
 
 * Generate code comments for annotated protobuf inputs. ([#161])
 
+* New protoc_plugin flag `generate_constructor_arguments` added to bring back
+  the old message factory methods with arguments. These arguments were removed
+  in 21.0.0 ([#703]) as they caused bloat in generated binaries in some cases.
+
+  Example usage:
+
+  ```
+  protoc --dart_out='generate_constructor_arguments,<other options>:.' ...
+  ```
+
 [#161]: https://github.com/google/protobuf.dart/issues/161
 
 ## 21.0.2

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=$(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="generate_constructor_arguments:$(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)

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;
 

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.generateConstructorArguments && _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.

protoc_plugin/lib/src/options.dart

@@ -50,11 +50,12 @@ bool genericOptionsParser(CodeGeneratorRequest request,
 class GenerationOptions {
   final bool useGrpc;
   final bool generateMetadata;
+  final bool generateConstructorArguments;
 
-  GenerationOptions({
-    this.useGrpc = false,
-    this.generateMetadata = false,
-  });
+  GenerationOptions(
+      {this.useGrpc = false,
+      this.generateMetadata = false,
+      this.generateConstructorArguments = false});
 }
 
 /// A parser for a name-value pair option. Options parsed in
@@ -87,13 +88,27 @@ 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 GenerateConstructorArgumentsParser implements SingleOptionParser {
+  bool generateConstructorArguments = false;
+
+  @override
+  void parse(String name, String? value, OnError onError) {
+    if (value != null) {
+      onError(
+          'Invalid generate_constructor_arguments option. No value expected.');
+      return;
+    }
+    generateConstructorArguments = 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 +120,21 @@ GenerationOptions? parseGenerationOptions(
 
   final grpcOptionParser = GrpcOptionParser();
   newParsers['grpc'] = grpcOptionParser;
+
   final generateMetadataParser = GenerateMetadataParser();
   newParsers['generate_kythe_info'] = generateMetadataParser;
 
+  final generateConstructorArgumentsParser =
+      GenerateConstructorArgumentsParser();
+  newParsers['generate_constructor_arguments'] =
+      generateConstructorArgumentsParser;
+
   if (genericOptionsParser(request, response, newParsers)) {
     return GenerationOptions(
-      useGrpc: grpcOptionParser.grpcEnabled,
-      generateMetadata: generateMetadataParser.generateKytheInfo,
-    );
+        useGrpc: grpcOptionParser.grpcEnabled,
+        generateMetadata: generateMetadataParser.generateKytheInfo,
+        generateConstructorArguments:
+            generateConstructorArgumentsParser.generateConstructorArguments);
   }
   return null;
 }

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()));
+  });
 }