From fc7b5a81ca0370a91b9484cbdc2170647c25d540 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Tue, 27 Jun 2023 12:28:50 +0200
Subject: [PATCH 01/10] Add a flag to generate constructor arguments

Fixes #850.
---
 protoc_plugin/CHANGELOG.md                   | 12 +++++
 protoc_plugin/lib/src/base_type.dart         |  3 ++
 protoc_plugin/lib/src/message_generator.dart | 48 +++++++++++++++++++-
 protoc_plugin/lib/src/options.dart           | 32 +++++++++++--
 protoc_plugin/pubspec.yaml                   |  2 +-
 5 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md
index 9d417daca..7d7126f34 100644
--- a/protoc_plugin/CHANGELOG.md
+++ b/protoc_plugin/CHANGELOG.md
@@ -1,3 +1,15 @@
+## 21.0.3
+
+* 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>:.' ...
+  ```
+
 ## 21.0.2
 
 * Fix missing protobuf import in generated grpc files. ([#844])
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 8727a31a6..abf85f7d1 100644
--- a/protoc_plugin/lib/src/message_generator.dart
+++ b/protoc_plugin/lib/src/message_generator.dart
@@ -324,7 +324,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)
       ]);
@@ -424,6 +425,51 @@ class MessageGenerator extends ProtobufContainer {
     out.println();
   }
 
+  void _generateFactory(IndentingWriter out) {
+    if (fileGen.options.generateConstructorArguments) {
+      out.print('factory $classname(');
+      if (_fieldList.isNotEmpty) {
+        out.println('{');
+        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.print('}');
+      }
+      if (_fieldList.isNotEmpty) {
+        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(') => create();');
+      }
+    } 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 96ed3ebdc..fb0047712 100644
--- a/protoc_plugin/lib/src/options.dart
+++ b/protoc_plugin/lib/src/options.dart
@@ -50,8 +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
@@ -84,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.
@@ -102,13 +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);
+        generateMetadata: generateMetadataParser.generateKytheInfo,
+        generateConstructorArguments:
+            generateConstructorArgumentsParser.generateConstructorArguments);
   }
   return null;
 }
diff --git a/protoc_plugin/pubspec.yaml b/protoc_plugin/pubspec.yaml
index 3314fa43a..88bf2353d 100644
--- a/protoc_plugin/pubspec.yaml
+++ b/protoc_plugin/pubspec.yaml
@@ -1,5 +1,5 @@
 name: protoc_plugin
-version: 21.0.2
+version: 21.0.3
 description: A protobuf protoc compiler plugin used to generate Dart code.
 repository: https://github.com/google/protobuf.dart/tree/master/protoc_plugin
 

From 3b88cf4da89a8aa2e680caff07f8abe08dac7f8d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Tue, 27 Jun 2023 12:59:01 +0200
Subject: [PATCH 02/10] Remove underscore in function local

---
 protoc_plugin/lib/src/message_generator.dart | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/protoc_plugin/lib/src/message_generator.dart b/protoc_plugin/lib/src/message_generator.dart
index abf85f7d1..281f64eb4 100644
--- a/protoc_plugin/lib/src/message_generator.dart
+++ b/protoc_plugin/lib/src/message_generator.dart
@@ -444,7 +444,7 @@ class MessageGenerator extends ProtobufContainer {
       }
       if (_fieldList.isNotEmpty) {
         out.println(') {');
-        out.println('  final _result = create();');
+        out.println('  final result = create();');
         for (final field in _fieldList) {
           out.println('  if (${field.memberNames!.fieldName} != null) {');
           if (field.isDeprecated) {
@@ -453,14 +453,14 @@ class MessageGenerator extends ProtobufContainer {
           }
           if (field.isRepeated || field.isMapField) {
             out.println(
-                '    _result.${field.memberNames!.fieldName}.addAll(${field.memberNames!.fieldName});');
+                '    result.${field.memberNames!.fieldName}.addAll(${field.memberNames!.fieldName});');
           } else {
             out.println(
-                '    _result.${field.memberNames!.fieldName} = ${field.memberNames!.fieldName};');
+                '    result.${field.memberNames!.fieldName} = ${field.memberNames!.fieldName};');
           }
           out.println('  }');
         }
-        out.println('  return _result;');
+        out.println('  return result;');
         out.println('}');
       } else {
         out.println(') => create();');

From a5d337bc5db7401d379155e7f9c089a3d2718493 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Tue, 27 Jun 2023 13:02:52 +0200
Subject: [PATCH 03/10] Simplify factory generator

---
 protoc_plugin/lib/src/message_generator.dart | 61 +++++++++-----------
 1 file changed, 26 insertions(+), 35 deletions(-)

diff --git a/protoc_plugin/lib/src/message_generator.dart b/protoc_plugin/lib/src/message_generator.dart
index 281f64eb4..9081f4ffd 100644
--- a/protoc_plugin/lib/src/message_generator.dart
+++ b/protoc_plugin/lib/src/message_generator.dart
@@ -426,45 +426,36 @@ class MessageGenerator extends ProtobufContainer {
   }
 
   void _generateFactory(IndentingWriter out) {
-    if (fileGen.options.generateConstructorArguments) {
-      out.print('factory $classname(');
-      if (_fieldList.isNotEmpty) {
-        out.println('{');
-        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},');
-          }
+    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.print('}');
       }
-      if (_fieldList.isNotEmpty) {
-        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('}) {');
+      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');
         }
-        out.println('  return result;');
-        out.println('}');
-      } else {
-        out.println(') => create();');
+        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();');
     }

From 491bda64d3cce4ea0fd9f72b87e4ab12359033c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Wed, 26 Jul 2023 15:22:09 +0200
Subject: [PATCH 04/10] Start building test protos with constructor args, bring
 back tests

---
 protoc_plugin/Makefile                        |  13 +++
 .../test/generated_message_test.dart          | 105 ++++++++++++++++++
 2 files changed, 118 insertions(+)

diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile
index a563cf846..f5f6abd96 100644
--- a/protoc_plugin/Makefile
+++ b/protoc_plugin/Makefile
@@ -78,6 +78,7 @@ 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)\
@@ -85,8 +86,20 @@ $(TEST_PROTO_LIBS): $(PLUGIN_SRC) $(TEST_PROTO_SRCS)
 		-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)
 	protoc --dart_out=lib/src/generated -Iprotos --plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH)) $(PREGENERATED_SRCS)
 	rm lib/src/generated/descriptor.pb{json,server}.dart
diff --git a/protoc_plugin/test/generated_message_test.dart b/protoc_plugin/test/generated_message_test.dart
index c859b5b14..d75ba833c 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,105 @@ void main() {
     final value2 = value1.deepCopy();
     assertAllExtensionsSet(value2);
   });
+
+  test('named arguments in constructor', () {
+    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',
+    );
+
+    // TODO
+    // assertAllFieldsSet(value);
+  });
 }

From ce857b88e4c633a891859f37392aa4293fc1bf74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omeragacan@gmail.com>
Date: Wed, 26 Jul 2023 15:59:15 +0200
Subject: [PATCH 05/10] Test new message with old assertAllFieldsSet by
 converting it to the old message

---
 protoc_plugin/test/generated_message_test.dart | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/protoc_plugin/test/generated_message_test.dart b/protoc_plugin/test/generated_message_test.dart
index d75ba833c..a25f6a4d4 100644
--- a/protoc_plugin/test/generated_message_test.dart
+++ b/protoc_plugin/test/generated_message_test.dart
@@ -789,7 +789,7 @@ void main() {
     assertAllExtensionsSet(value2);
   });
 
-  test('named arguments in constructor', () {
+  test('Named arguments in constructors', () {
     final value = constructor_args_unittest.TestAllTypes(
       optionalInt32: 101,
       optionalInt64: make64(102),
@@ -886,7 +886,8 @@ void main() {
       defaultCord: '425',
     );
 
-    // TODO
-    // assertAllFieldsSet(value);
+    // Convert the message with constructor arguments to the message without
+    // constructor arguments, to be able to reuse `assertAllFieldsSet`.
+    assertAllFieldsSet(TestAllTypes.fromBuffer(value.writeToBuffer()));
   });
 }

From 32249f0c9ebbb187446496403896d27e86770977 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omeragacan@gmail.com>
Date: Wed, 26 Jul 2023 16:00:54 +0200
Subject: [PATCH 06/10] Simplify dart test command, revert whitespace in
 Makefile

---
 protoc_plugin/Makefile | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile
index f5f6abd96..5b8965bc1 100644
--- a/protoc_plugin/Makefile
+++ b/protoc_plugin/Makefile
@@ -99,7 +99,6 @@ $(TEST_PROTO_LIBS): $(PLUGIN_SRC) $(TEST_PROTO_SRCS)
 
 	dart format $(TEST_PROTO_DIR)
 
-
 update-pregenerated: $(PLUGIN_PATH) $(PREGENERATED_SRCS)
 	protoc --dart_out=lib/src/generated -Iprotos --plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH)) $(PREGENERATED_SRCS)
 	rm lib/src/generated/descriptor.pb{json,server}.dart
@@ -110,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)

From b1c48b3ba5039de2cd0067254304040f3043e5c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Mon, 14 Aug 2023 10:03:00 +0200
Subject: [PATCH 07/10] Rename flag to constructor_args

---
 protoc_plugin/CHANGELOG.md         | 8 ++++----
 protoc_plugin/Makefile             | 2 +-
 protoc_plugin/lib/src/options.dart | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md
index a92d8f84f..6c3fcd9d9 100644
--- a/protoc_plugin/CHANGELOG.md
+++ b/protoc_plugin/CHANGELOG.md
@@ -2,14 +2,14 @@
 
 * 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.
+* New protoc_plugin flag `constructor_args` 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>:.' ...
+  protoc --dart_out='constructor_args,<other options>:.' ...
   ```
 
 [#161]: https://github.com/google/protobuf.dart/issues/161
diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile
index 5b8965bc1..b3277c7c0 100644
--- a/protoc_plugin/Makefile
+++ b/protoc_plugin/Makefile
@@ -91,7 +91,7 @@ $(TEST_PROTO_LIBS): $(PLUGIN_SRC) $(TEST_PROTO_SRCS)
 
 	protoc\
 		--experimental_allow_proto3_optional\
-		--dart_out="generate_constructor_arguments:$(TEST_PROTO_DIR)/constructor_args"\
+		--dart_out="constructor_args:$(TEST_PROTO_DIR)/constructor_args"\
 		-Iprotos\
 		-I$(TEST_PROTO_SRC_DIR)\
 		--plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH))\
diff --git a/protoc_plugin/lib/src/options.dart b/protoc_plugin/lib/src/options.dart
index fb0047712..8f79d7932 100644
--- a/protoc_plugin/lib/src/options.dart
+++ b/protoc_plugin/lib/src/options.dart
@@ -102,7 +102,7 @@ class GenerateConstructorArgumentsParser implements SingleOptionParser {
   void parse(String name, String? value, OnError onError) {
     if (value != null) {
       onError(
-          'Invalid generate_constructor_arguments option. No value expected.');
+          'Invalid constructor_args option. No value expected.');
       return;
     }
     generateConstructorArguments = true;
@@ -126,7 +126,7 @@ GenerationOptions? parseGenerationOptions(
 
   final generateConstructorArgumentsParser =
       GenerateConstructorArgumentsParser();
-  newParsers['generate_constructor_arguments'] =
+  newParsers['constructor_args'] =
       generateConstructorArgumentsParser;
 
   if (genericOptionsParser(request, response, newParsers)) {

From 5dd3621732ac4bf69190ae370c2a84bf7f22a220 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Mon, 14 Aug 2023 10:42:56 +0200
Subject: [PATCH 08/10] Formatting

---
 protoc_plugin/lib/src/options.dart | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/protoc_plugin/lib/src/options.dart b/protoc_plugin/lib/src/options.dart
index 8f79d7932..c9df1cb81 100644
--- a/protoc_plugin/lib/src/options.dart
+++ b/protoc_plugin/lib/src/options.dart
@@ -101,8 +101,7 @@ class GenerateConstructorArgumentsParser implements SingleOptionParser {
   @override
   void parse(String name, String? value, OnError onError) {
     if (value != null) {
-      onError(
-          'Invalid constructor_args option. No value expected.');
+      onError('Invalid constructor_args option. No value expected.');
       return;
     }
     generateConstructorArguments = true;
@@ -126,8 +125,7 @@ GenerationOptions? parseGenerationOptions(
 
   final generateConstructorArgumentsParser =
       GenerateConstructorArgumentsParser();
-  newParsers['constructor_args'] =
-      generateConstructorArgumentsParser;
+  newParsers['constructor_args'] = generateConstructorArgumentsParser;
 
   if (genericOptionsParser(request, response, newParsers)) {
     return GenerationOptions(

From a24f91713ee4a248e37976e902d27c76bbf36435 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Mon, 14 Aug 2023 11:23:01 +0200
Subject: [PATCH 09/10] Generate constructor args by default

---
 protoc_plugin/CHANGELOG.md                    | 15 ++++++---
 protoc_plugin/Makefile                        |  4 +--
 protoc_plugin/lib/src/message_generator.dart  |  2 +-
 protoc_plugin/lib/src/options.dart            | 20 +++++------
 protoc_plugin/test/file_generator_test.dart   | 33 ++++++++++++-------
 .../test/message_generator_test.dart          |  6 ++--
 6 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md
index 6c3fcd9d9..5371b3bc3 100644
--- a/protoc_plugin/CHANGELOG.md
+++ b/protoc_plugin/CHANGELOG.md
@@ -1,18 +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` added to disable generating the arguments.
 
-* New protoc_plugin flag `constructor_args` 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.
+  Constructor arguments were removed in 21.0.0 as they increase dart2js binary
+  sizes even when the arguments are not used.
 
-  Example usage:
+  Example usage to disable constructor arguments:
 
   ```
-  protoc --dart_out='constructor_args,<other options>:.' ...
+  protoc --dart_out='disable_constructor_args,<other options>:.' ...
   ```
 
+  ([#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 b3277c7c0..69bca65d2 100644
--- a/protoc_plugin/Makefile
+++ b/protoc_plugin/Makefile
@@ -81,7 +81,7 @@ $(TEST_PROTO_LIBS): $(PLUGIN_SRC) $(TEST_PROTO_SRCS)
 
 	protoc\
 		--experimental_allow_proto3_optional\
-		--dart_out=$(TEST_PROTO_DIR)\
+		--dart_out="disable_constructor_args:$(TEST_PROTO_DIR)"\
 		-Iprotos\
 		-I$(TEST_PROTO_SRC_DIR)\
 		--plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH))\
@@ -91,7 +91,7 @@ $(TEST_PROTO_LIBS): $(PLUGIN_SRC) $(TEST_PROTO_SRCS)
 
 	protoc\
 		--experimental_allow_proto3_optional\
-		--dart_out="constructor_args:$(TEST_PROTO_DIR)/constructor_args"\
+		--dart_out="$(TEST_PROTO_DIR)/constructor_args"\
 		-Iprotos\
 		-I$(TEST_PROTO_SRC_DIR)\
 		--plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH))\
diff --git a/protoc_plugin/lib/src/message_generator.dart b/protoc_plugin/lib/src/message_generator.dart
index fae311a23..fca4a33c4 100644
--- a/protoc_plugin/lib/src/message_generator.dart
+++ b/protoc_plugin/lib/src/message_generator.dart
@@ -432,7 +432,7 @@ class MessageGenerator extends ProtobufContainer {
   }
 
   void _generateFactory(IndentingWriter out) {
-    if (fileGen.options.generateConstructorArguments && _fieldList.isNotEmpty) {
+    if (!fileGen.options.disableConstructorArgs && _fieldList.isNotEmpty) {
       out.println('factory $classname({');
       for (final field in _fieldList) {
         _emitDeprecatedIf(field.isDeprecated, out);
diff --git a/protoc_plugin/lib/src/options.dart b/protoc_plugin/lib/src/options.dart
index c9df1cb81..25b960b26 100644
--- a/protoc_plugin/lib/src/options.dart
+++ b/protoc_plugin/lib/src/options.dart
@@ -50,12 +50,12 @@ bool genericOptionsParser(CodeGeneratorRequest request,
 class GenerationOptions {
   final bool useGrpc;
   final bool generateMetadata;
-  final bool generateConstructorArguments;
+  final bool disableConstructorArgs;
 
   GenerationOptions(
       {this.useGrpc = false,
       this.generateMetadata = false,
-      this.generateConstructorArguments = false});
+      this.disableConstructorArgs = false});
 }
 
 /// A parser for a name-value pair option. Options parsed in
@@ -95,16 +95,16 @@ class GenerateMetadataParser implements SingleOptionParser {
   }
 }
 
-class GenerateConstructorArgumentsParser implements SingleOptionParser {
-  bool generateConstructorArguments = false;
+class DisableConstructorArgsParser implements SingleOptionParser {
+  bool value = false;
 
   @override
   void parse(String name, String? value, OnError onError) {
     if (value != null) {
-      onError('Invalid constructor_args option. No value expected.');
+      onError('Invalid disable_constructor_args option. No value expected.');
       return;
     }
-    generateConstructorArguments = true;
+    this.value = true;
   }
 }
 
@@ -123,16 +123,14 @@ GenerationOptions? parseGenerationOptions(
   final generateMetadataParser = GenerateMetadataParser();
   newParsers['generate_kythe_info'] = generateMetadataParser;
 
-  final generateConstructorArgumentsParser =
-      GenerateConstructorArgumentsParser();
-  newParsers['constructor_args'] = generateConstructorArgumentsParser;
+  final disableConstructorArgsParser = DisableConstructorArgsParser();
+  newParsers['disable_constructor_args'] = disableConstructorArgsParser;
 
   if (genericOptionsParser(request, response, newParsers)) {
     return GenerationOptions(
         useGrpc: grpcOptionParser.grpcEnabled,
         generateMetadata: generateMetadataParser.generateKytheInfo,
-        generateConstructorArguments:
-            generateConstructorArgumentsParser.generateConstructorArguments);
+        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/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, <String>{}, 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, <String>{}, 0);
 

From 5621bcc921943db325454dbaba932ef991d5f96f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= <omersa@google.com>
Date: Mon, 14 Aug 2023 11:49:56 +0200
Subject: [PATCH 10/10] Reword changelog

---
 protoc_plugin/CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md
index 5371b3bc3..15a7c6207 100644
--- a/protoc_plugin/CHANGELOG.md
+++ b/protoc_plugin/CHANGELOG.md
@@ -2,7 +2,7 @@
 
 * Generate code comments for annotated protobuf inputs. ([#161])
 * Generate message constructor arguments by default again. New flag
-  `disable_constructor_args` added to disable generating the arguments.
+  `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.