Fix proto3 repeated field encoding without packed option (#635)
proto3 repeated fields are packed by default, see
https://developers.google.com/protocol-buffers/docs/encoding#packed.
Fixes #345
diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile
index 651302a..3a7ada9 100644
--- a/protoc_plugin/Makefile
+++ b/protoc_plugin/Makefile
@@ -56,7 +56,9 @@
package1 \
package2 \
package3 \
+ proto2_repeated \
proto3_optional \
+ proto3_repeated \
service \
service2 \
service3 \
diff --git a/protoc_plugin/lib/src/extension_generator.dart b/protoc_plugin/lib/src/extension_generator.dart
index 295100f..4404e07 100644
--- a/protoc_plugin/lib/src/extension_generator.dart
+++ b/protoc_plugin/lib/src/extension_generator.dart
@@ -122,7 +122,7 @@
}
} else {
invocation = '$protobufImportPrefix.Extension<$dartType>';
- named['defaultOrMaker'] = _field.generateDefaultFunction(fileGen!);
+ named['defaultOrMaker'] = _field.generateDefaultFunction();
if (type.isMessage || type.isGroup) {
named['subBuilder'] = '$dartType.create';
} else if (type.isEnum) {
diff --git a/protoc_plugin/lib/src/file_generator.dart b/protoc_plugin/lib/src/file_generator.dart
index ebe141e..b2600b6 100644
--- a/protoc_plugin/lib/src/file_generator.dart
+++ b/protoc_plugin/lib/src/file_generator.dart
@@ -30,6 +30,11 @@
': $value';
}
+enum ProtoSyntax {
+ proto2,
+ proto3,
+}
+
/// Generates the Dart output files for one .proto input file.
///
/// Outputs include .pb.dart, pbenum.dart, and .pbjson.dart.
@@ -143,8 +148,13 @@
/// True if cross-references have been resolved.
bool _linked = false;
+ final ProtoSyntax syntax;
+
FileGenerator(this.descriptor, this.options)
- : protoFileUri = Uri.file(descriptor.name) {
+ : protoFileUri = Uri.file(descriptor.name),
+ syntax = descriptor.syntax == 'proto3'
+ ? ProtoSyntax.proto3
+ : ProtoSyntax.proto2 {
if (protoFileUri.isAbsolute) {
// protoc should never generate an import with an absolute path.
throw 'FAILURE: Import with absolute path is not supported';
diff --git a/protoc_plugin/lib/src/message_generator.dart b/protoc_plugin/lib/src/message_generator.dart
index 9b8c560..a2efc02 100644
--- a/protoc_plugin/lib/src/message_generator.dart
+++ b/protoc_plugin/lib/src/message_generator.dart
@@ -342,7 +342,7 @@
}
for (var field in _fieldList) {
- out.println(field.generateBuilderInfoCall(fileGen, package));
+ out.println(field.generateBuilderInfoCall(package));
}
if (_descriptor.extensionRange.isNotEmpty) {
@@ -372,7 +372,7 @@
' ${field.baseType.getRepeatedDartTypeIterable(fileGen)}? ${field.memberNames!.fieldName},');
} else {
out.println(
- ' ${field.getDartType(fileGen)}? ${field.memberNames!.fieldName},');
+ ' ${field.getDartType()}? ${field.memberNames!.fieldName},');
}
}
out.print('}');
@@ -510,7 +510,7 @@
void generateFieldAccessorsMutators(
ProtobufField field, IndentingWriter out, List<int> memberFieldPath) {
- var fieldTypeString = field.getDartType(fileGen);
+ var fieldTypeString = field.getDartType();
var defaultExpr = field.getDefaultExpr();
var names = field.memberNames;
diff --git a/protoc_plugin/lib/src/protobuf_field.dart b/protoc_plugin/lib/src/protobuf_field.dart
index 5fa3b1c..b8f5b3f 100644
--- a/protoc_plugin/lib/src/protobuf_field.dart
+++ b/protoc_plugin/lib/src/protobuf_field.dart
@@ -24,6 +24,7 @@
final String fullName;
final BaseType baseType;
+ final ProtobufContainer parent;
ProtobufField.message(
FieldNames names, ProtobufContainer parent, GenerationContext ctx)
@@ -33,8 +34,8 @@
ProtobufContainer parent, GenerationContext ctx)
: this._(descriptor, null, parent, ctx);
- ProtobufField._(this.descriptor, FieldNames? dartNames,
- ProtobufContainer parent, GenerationContext ctx)
+ ProtobufField._(this.descriptor, FieldNames? dartNames, this.parent,
+ GenerationContext ctx)
: memberNames = dartNames,
fullName = '${parent.fullName}.${descriptor.name}',
baseType = BaseType(descriptor, ctx);
@@ -61,8 +62,33 @@
bool get isRepeated =>
descriptor.label == FieldDescriptorProto_Label.LABEL_REPEATED;
- /// True if the field is to be encoded with [packed=true] encoding.
- bool get isPacked => isRepeated && descriptor.options.packed;
+ /// True if a numeric field is repeated and it should be encoded with packed
+ /// encoding. In proto3 repeated fields are encoded as packed by default.
+ /// proto2 requires `[packed=true]` option.
+ bool get isPacked {
+ if (!isRepeated) {
+ return false; // only repeated fields can be packed
+ }
+
+ if (!baseType.isPackable) {
+ return false;
+ }
+
+ switch (parent.fileGen!.syntax) {
+ case ProtoSyntax.proto3:
+ if (!descriptor.hasOptions()) {
+ return true; // packed by default in proto3
+ } else {
+ return !descriptor.options.hasPacked() || descriptor.options.packed;
+ }
+ case ProtoSyntax.proto2:
+ if (!descriptor.hasOptions()) {
+ return false; // not packed by default in proto3
+ } else {
+ return descriptor.options.packed;
+ }
+ }
+ }
/// Whether the field has the `overrideGetter` annotation set to true.
bool get overridesGetter => _hasBooleanOption(Dart_options.overrideGetter);
@@ -111,16 +137,15 @@
/// Returns the expression to use for the Dart type.
///
/// This will be a List for repeated types.
- /// [fileGen] represents the .proto file where we are generating code.
- String getDartType(FileGenerator fileGen) {
+ String getDartType() {
if (isMapField) {
final d = baseType.generator as MessageGenerator;
- var keyType = d._fieldList[0].baseType.getDartType(fileGen);
- var valueType = d._fieldList[1].baseType.getDartType(fileGen);
+ var keyType = d._fieldList[0].baseType.getDartType(parent.fileGen!);
+ var valueType = d._fieldList[1].baseType.getDartType(parent.fileGen!);
return '$coreImportPrefix.Map<$keyType, $valueType>';
}
- if (isRepeated) return baseType.getRepeatedDartType(fileGen);
- return baseType.getDartType(fileGen);
+ if (isRepeated) return baseType.getRepeatedDartType(parent.fileGen!);
+ return baseType.getDartType(parent.fileGen!);
}
/// Returns the tag number of the underlying proto field.
@@ -162,15 +187,14 @@
/// Returns Dart code adding this field to a BuilderInfo object.
/// The call will start with ".." and a method name.
- /// [fileGen] represents the .proto file where the code will be evaluated.
- String generateBuilderInfoCall(FileGenerator fileGen, String package) {
+ String generateBuilderInfoCall(String package) {
assert(descriptor.hasJsonName());
var quotedName = configurationDependent(
'protobuf.omit_field_names',
quoted(descriptor.jsonName),
);
- var type = baseType.getDartType(fileGen);
+ var type = baseType.getDartType(parent.fileGen!);
String invocation;
@@ -183,8 +207,8 @@
final generator = baseType.generator as MessageGenerator;
var key = generator._fieldList[0];
var value = generator._fieldList[1];
- var keyType = key.baseType.getDartType(fileGen);
- var valueType = value.baseType.getDartType(fileGen);
+ var keyType = key.baseType.getDartType(parent.fileGen!);
+ var valueType = value.baseType.getDartType(parent.fileGen!);
invocation = 'm<$keyType, $valueType>';
@@ -197,7 +221,7 @@
if (value.baseType.isEnum) {
named['valueOf'] = '$valueType.valueOf';
named['enumValues'] = '$valueType.values';
- named['defaultEnumValue'] = value.generateDefaultFunction(fileGen);
+ named['defaultEnumValue'] = value.generateDefaultFunction();
}
if (package != '') {
named['packageName'] =
@@ -219,12 +243,12 @@
} else if (baseType.isEnum) {
named['valueOf'] = '$type.valueOf';
named['enumValues'] = '$type.values';
- named['defaultEnumValue'] = generateDefaultFunction(fileGen);
+ named['defaultEnumValue'] = generateDefaultFunction();
}
}
} else {
// Singular field.
- var makeDefault = generateDefaultFunction(fileGen);
+ var makeDefault = generateDefaultFunction();
if (baseType.isEnum) {
args.add(typeConstant);
@@ -302,10 +326,7 @@
}
/// Returns a function expression that returns the field's default value.
- ///
- /// [fileGen] represents the .proto file where the expression will be
- /// evaluated.
- String? generateDefaultFunction(FileGenerator? fileGen) {
+ String? generateDefaultFunction() {
assert(!isRepeated);
switch (descriptor.type) {
case FieldDescriptorProto_Type.TYPE_BOOL:
@@ -359,9 +380,9 @@
return '() => <$coreImportPrefix.int>[$byteList]';
case FieldDescriptorProto_Type.TYPE_GROUP:
case FieldDescriptorProto_Type.TYPE_MESSAGE:
- return '${baseType.getDartType(fileGen!)}.getDefault';
+ return '${baseType.getDartType(parent.fileGen!)}.getDefault';
case FieldDescriptorProto_Type.TYPE_ENUM:
- var className = baseType.getDartType(fileGen!);
+ var className = baseType.getDartType(parent.fileGen!);
final gen = baseType.generator as EnumGenerator;
if (descriptor.hasDefaultValue() &&
descriptor.defaultValue.isNotEmpty) {
diff --git a/protoc_plugin/test/protos/proto2_repeated.proto b/protoc_plugin/test/protos/proto2_repeated.proto
new file mode 100644
index 0000000..ee3b156
--- /dev/null
+++ b/protoc_plugin/test/protos/proto2_repeated.proto
@@ -0,0 +1,11 @@
+// Copyright (c) 2017, 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.
+
+syntax = "proto2";
+
+message Proto2Repeated {
+ repeated int32 ints_default = 1;
+ repeated int32 ints_packed = 2 [packed=true];
+ repeated int32 ints_not_packed = 3 [packed=false];
+}
diff --git a/protoc_plugin/test/protos/proto3_repeated.proto b/protoc_plugin/test/protos/proto3_repeated.proto
new file mode 100644
index 0000000..723d154
--- /dev/null
+++ b/protoc_plugin/test/protos/proto3_repeated.proto
@@ -0,0 +1,11 @@
+// Copyright (c) 2017, 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.
+
+syntax = "proto3";
+
+message Proto3Repeated {
+ repeated int32 ints_default = 1;
+ repeated int32 ints_packed = 2 [packed=true];
+ repeated int32 ints_not_packed = 3 [packed=false];
+}
diff --git a/protoc_plugin/test/repeated_encoding_test.dart b/protoc_plugin/test/repeated_encoding_test.dart
new file mode 100644
index 0000000..17a990a
--- /dev/null
+++ b/protoc_plugin/test/repeated_encoding_test.dart
@@ -0,0 +1,50 @@
+#!/usr/bin/env dart
+// Copyright (c) 2015, 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:test/test.dart';
+
+import '../out/protos/proto2_repeated.pb.dart';
+import '../out/protos/proto3_repeated.pb.dart';
+
+void main() {
+ test('check proto2 and proto3 repeated field encodings', () {
+ final proto2 = Proto2Repeated(
+ intsDefault: [1, 2], intsPacked: [1, 2], intsNotPacked: [1, 2]);
+ final proto2Encoded = proto2.writeToBuffer();
+ expect(
+ proto2Encoded.toList(),
+ equals([
+ 8, // field = 1, type = varint
+ 1, // value = 1
+ 8, // field = 1, type = varint
+ 2, // value = 2
+ 18, // field = 2, type = length delimited
+ 2, // length = 2
+ 1, 2, // values = [1, 2]
+ 24, // field = 3, type = varint
+ 1, // value = 1
+ 24, // field = 3, type = varint
+ 2, // value = 2
+ ]));
+
+ final proto3 = Proto3Repeated(
+ intsDefault: [1, 2], intsPacked: [1, 2], intsNotPacked: [1, 2]);
+ final proto3Encoded = proto3.writeToBuffer();
+ expect(
+ proto3Encoded.toList(),
+ equals([
+ 10, // field = 1, type = length delimited
+ 2, // length = 2
+ 1, 2, // values = [1, 2]
+ 18, // field = 2, type = length delimited
+ 2, // length = 2
+ 1, 2, // values = [1, 2]
+ 24, // field = 3, type = varint
+ 1, // value = 1
+ 24, // field = 3, type = varint
+ 2, // value = 2
+ ]));
+ });
+}
