Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert contructor arguments #703

Merged
merged 1 commit into from
Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion protoc_plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
## 21.0.0
## 21.0.0-dev

* Identifiers `fromBuffer`, `fromJson`, `$_defaultFor`, `initByValue` are no
longer reserved. Proto fields with those Dart names will no longer have a
suffix added. ([#679])

* Message constructor arguments removed. Constructors with arguments cause
increase in release binary sizes even when no arguments are passed to the
constructors. ([#703])

**Migration:**

Set the fields after construction, using cascade syntax. For example, if you
have:
```dart
MyMessage(a: 123, b: [1, 2, 3])
```
You can do:
```dart
MyMessage()
..a = 123
..b.addAll([1, 2, 3])
```

[#679]: https://github.com/google/protobuf.dart/pull/679
[#703]: https://github.com/google/protobuf.dart/pull/703

## 20.0.1

Expand Down
3 changes: 0 additions & 3 deletions protoc_plugin/lib/src/base_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ 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;

Expand Down
39 changes: 1 addition & 38 deletions protoc_plugin/lib/src/message_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -362,44 +362,7 @@ class MessageGenerator extends ProtobufContainer {
out.printlnAnnotated('$classname._() : super();', [
NamedLocation(name: classname, fieldPathSegment: fieldPath, start: 0)
]);
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();');
}
out.println('factory $classname() => create();');
out.println(
'factory $classname.fromBuffer($coreImportPrefix.List<$coreImportPrefix.int> i,'
' [$protobufImportPrefix.ExtensionRegistry r = $protobufImportPrefix.ExtensionRegistry.EMPTY])'
Expand Down
19 changes: 9 additions & 10 deletions protoc_plugin/test/freeze_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import '../out/protos/nested_message.pb.dart';

void main() {
test('testFreezingNestedFields', () {
var top = Top(
nestedMessageList: [Nested(a: 1)],
nestedMessageMap: {1: Nested(a: 2)},
nestedMessage: Nested(a: 3),
);
final top = Top()
..nestedMessageList.add(Nested()..a = 1)
..nestedMessageMap[1] = (Nested()..a = 2)
..nestedMessage = (Nested()..a = 3);

// Create aliases to lists, maps, nested messages
var list = top.nestedMessageList;
Expand All @@ -28,24 +27,24 @@ void main() {
// Check list field
expect(top.nestedMessageList.length, 1);
expect(top.nestedMessageList[0].isFrozen, true);
expect(() => top.nestedMessageList.add(Nested(a: 0)),
expect(() => top.nestedMessageList.add(Nested()..a = 0),
throwsA(const TypeMatcher<UnsupportedError>()));

// Check map field
expect(top.nestedMessageMap.length, 1);
expect(top.nestedMessageMap[1]!.isFrozen, true);
expect(() => top.nestedMessageMap[2] = Nested(a: 0),
expect(() => top.nestedMessageMap[2] = Nested()..a = 0,
throwsA(const TypeMatcher<UnsupportedError>()));
expect(() => map[0] = Nested(a: 0),
expect(() => map[0] = Nested()..a = 0,
throwsA(const TypeMatcher<UnsupportedError>()));

// Check message field
expect(top.nestedMessage.isFrozen, true);

// Check aliases
expect(() => list.add(Nested(a: 0)),
expect(() => list.add(Nested()..a = 0),
throwsA(const TypeMatcher<UnsupportedError>()));
expect(() => map[123] = Nested(a: 0),
expect(() => map[123] = Nested()..a = 0,
throwsA(const TypeMatcher<UnsupportedError>()));
expect(list[0].isFrozen, true);
expect(map[1]!.isFrozen, true);
Expand Down
84 changes: 0 additions & 84 deletions protoc_plugin/test/generated_message_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -876,88 +876,4 @@ void main() {
final value2 = value1.deepCopy();
assertAllExtensionsSet(value2);
});

test('named arguments in constructor', () {
final value = 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: TestAllTypes_OptionalGroup(a: 117),
optionalNestedMessage: TestAllTypes_NestedMessage(bb: 118),
optionalForeignMessage: ForeignMessage(c: 119),
optionalImportMessage: ImportMessage(d: 120),
optionalNestedEnum: TestAllTypes_NestedEnum.BAZ,
optionalForeignEnum: ForeignEnum.FOREIGN_BAZ,
optionalImportEnum: 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: [
TestAllTypes_RepeatedGroup(a: 217),
TestAllTypes_RepeatedGroup(a: 317)
],
repeatedNestedMessage: [
TestAllTypes_NestedMessage(bb: 218),
TestAllTypes_NestedMessage(bb: 318)
],
repeatedForeignMessage: [ForeignMessage(c: 219), ForeignMessage(c: 319)],
repeatedImportMessage: [ImportMessage(d: 220), ImportMessage(d: 320)],
repeatedNestedEnum: [
TestAllTypes_NestedEnum.BAR,
TestAllTypes_NestedEnum.BAZ
],
repeatedForeignEnum: [ForeignEnum.FOREIGN_BAR, ForeignEnum.FOREIGN_BAZ],
repeatedImportEnum: [ImportEnum.IMPORT_BAR, 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: TestAllTypes_NestedEnum.FOO,
defaultForeignEnum: ForeignEnum.FOREIGN_FOO,
defaultImportEnum: ImportEnum.IMPORT_FOO,
defaultStringPiece: '424',
defaultCord: '425',
);

assertAllFieldsSet(value);
});
}
18 changes: 1 addition & 17 deletions protoc_plugin/test/goldens/imports.pb
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,7 @@ class M extends $pb.GeneratedMessage {
;

M._() : super();
factory M({
M? m,
$1.M? m1,
$2.M? m2,
}) {
final _result = create();
if (m != null) {
_result.m = m;
}
if (m1 != null) {
_result.m1 = m1;
}
if (m2 != null) {
_result.m2 = m2;
}
return _result;
}
factory M() => create();
factory M.fromBuffer($core.List<$core.int> i, [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => create()..mergeFromBuffer(i, r);
factory M.fromJson($core.String i, [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => create()..mergeFromJson(i, r);
@$core.Deprecated(
Expand Down
10 changes: 1 addition & 9 deletions protoc_plugin/test/goldens/int64.pb
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,7 @@ class Int64 extends $pb.GeneratedMessage {
;

Int64._() : super();
factory Int64({
$fixnum.Int64? value,
}) {
final _result = create();
if (value != null) {
_result.value = value;
}
return _result;
}
factory Int64() => create();
factory Int64.fromBuffer($core.List<$core.int> i, [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => create()..mergeFromBuffer(i, r);
factory Int64.fromJson($core.String i, [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => create()..mergeFromJson(i, r);
@$core.Deprecated(
Expand Down
24 changes: 1 addition & 23 deletions protoc_plugin/test/goldens/messageGenerator
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,7 @@ class PhoneNumber extends $pb.GeneratedMessage {
;

PhoneNumber._() : super();
factory PhoneNumber({
$core.String? number,
PhoneNumber_PhoneType? type,
$core.String? name,
@$core.Deprecated('This field is deprecated.')
$core.String? deprecatedField,
}) {
final _result = create();
if (number != null) {
_result.number = number;
}
if (type != null) {
_result.type = type;
}
if (name != null) {
_result.name = name;
}
if (deprecatedField != null) {
// ignore: deprecated_member_use_from_same_package
_result.deprecatedField = deprecatedField;
}
return _result;
}
factory PhoneNumber() => create();
factory PhoneNumber.fromBuffer($core.List<$core.int> i, [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => create()..mergeFromBuffer(i, r);
factory PhoneNumber.fromJson($core.String i, [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => create()..mergeFromJson(i, r);
@$core.Deprecated(
Expand Down
Loading