Skip to content

Commit

Permalink
Support optional proto3 fields (#444)
Browse files Browse the repository at this point in the history
Also update plugin.proto and descriptor.proto to the latest version
  • Loading branch information
iinozemtsev committed Nov 19, 2020
1 parent f06208e commit 1319847
Show file tree
Hide file tree
Showing 20 changed files with 684 additions and 192 deletions.
22 changes: 11 additions & 11 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# Created with package:mono_repo v2.3.0
# Created with package:mono_repo v3.0.0
language: dart

# Custom configuration
before_install:
- "wget -O protoc.zip https://github.com/protocolbuffers/protobuf/releases/download/v3.6.1/protoc-3.6.1-linux-x86_64.zip"
- "wget -O protoc.zip https://github.com/protocolbuffers/protobuf/releases/download/v3.14.0/protoc-3.14.0-linux-x86_64.zip"
- "unzip -d protoc protoc.zip"
- "export PATH=$PWD/protoc/bin:$PATH"
- echo $PATH
Expand All @@ -20,55 +20,55 @@ jobs:
dart: dev
os: linux
env: PKGS="api_benchmark query_benchmark"
script: ./tool/travis.sh command_0 dartfmt dartanalyzer_0
script: tool/travis.sh command_0 dartfmt dartanalyzer_0
- stage: format_analyzer_tests
name: "SDK: 2.7.0; PKG: protobuf; TASKS: `dartanalyzer --fatal-warnings .`"
dart: "2.7.0"
os: linux
env: PKGS="protobuf"
script: ./tool/travis.sh dartanalyzer_0
script: tool/travis.sh dartanalyzer_0
- stage: format_analyzer_tests
name: "SDK: dev; PKG: protobuf; TASKS: [`dartanalyzer --fatal-infos --fatal-warnings lib test`, `dartfmt -n --set-exit-if-changed .`]"
dart: dev
os: linux
env: PKGS="protobuf"
script: ./tool/travis.sh dartanalyzer_1 dartfmt
script: tool/travis.sh dartanalyzer_1 dartfmt
- stage: format_analyzer_tests
name: "SDK: 2.7.0; PKG: protobuf; TASKS: `pub run test`"
dart: "2.7.0"
os: linux
env: PKGS="protobuf"
script: ./tool/travis.sh test
script: tool/travis.sh test
- stage: format_analyzer_tests
name: "SDK: dev; PKG: protobuf; TASKS: `pub run test`"
dart: dev
os: linux
env: PKGS="protobuf"
script: ./tool/travis.sh test
script: tool/travis.sh test
- stage: format_analyzer_tests
name: "SDK: 2.7.0; PKG: protoc_plugin; TASKS: [`make protos`, `dartanalyzer --fatal-warnings .`]"
dart: "2.7.0"
os: linux
env: PKGS="protoc_plugin"
script: ./tool/travis.sh command_1 dartanalyzer_0
script: tool/travis.sh command_1 dartanalyzer_0
- stage: format_analyzer_tests
name: "SDK: dev; PKG: protoc_plugin; TASKS: [`make protos`, `dartfmt -n --set-exit-if-changed .`, `dartanalyzer --fatal-infos --fatal-warnings .`]"
dart: dev
os: linux
env: PKGS="protoc_plugin"
script: ./tool/travis.sh command_1 dartfmt dartanalyzer_2
script: tool/travis.sh command_1 dartfmt dartanalyzer_2
- stage: format_analyzer_tests
name: "SDK: 2.7.0; PKG: protoc_plugin; TASKS: [`make protos`, `pub run test`]"
dart: "2.7.0"
os: linux
env: PKGS="protoc_plugin"
script: ./tool/travis.sh command_1 test
script: tool/travis.sh command_1 test
- stage: format_analyzer_tests
name: "SDK: dev; PKG: protoc_plugin; TASKS: [`make protos`, `pub run test`]"
dart: dev
os: linux
env: PKGS="protoc_plugin"
script: ./tool/travis.sh command_1 test
script: tool/travis.sh command_1 test

stages:
- format_analyzer_tests
Expand Down
2 changes: 1 addition & 1 deletion mono_repo.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
travis:
before_install:
# Install our custom protoc, as the version we get with apt-get is too old.
- wget -O protoc.zip https://github.com/protocolbuffers/protobuf/releases/download/v3.6.1/protoc-3.6.1-linux-x86_64.zip
- wget -O protoc.zip https://github.com/protocolbuffers/protobuf/releases/download/v3.14.0/protoc-3.14.0-linux-x86_64.zip
- unzip -d protoc protoc.zip
- export PATH=$PWD/protoc/bin:$PATH
- echo $PATH
Expand Down
4 changes: 4 additions & 0 deletions protoc_plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 19.2.1

* Support optional proto3 fields.

## 19.2.0+1

* Fix syntax error introduced by gRPC client interceptor changes.
Expand Down
4 changes: 3 additions & 1 deletion protoc_plugin/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ TEST_PROTO_LIST = \
package1 \
package2 \
package3 \
proto3_optional \
service \
service2 \
service3 \
Expand All @@ -73,6 +74,7 @@ PREGENERATED_SRCS=protos/descriptor.proto protos/plugin.proto protos/dart_option
$(TEST_PROTO_LIBS): $(PLUGIN_PATH) $(TEST_PROTO_SRCS)
[ -d $(TEST_PROTO_DIR) ] || mkdir -p $(TEST_PROTO_DIR)
protoc\
--experimental_allow_proto3_optional\
--dart_out=$(TEST_PROTO_DIR)\
-Iprotos\
-I$(TEST_PROTO_SRC_DIR)\
Expand All @@ -86,7 +88,7 @@ update-pregenerated: $(PLUGIN_PATH) $(PREGENERATED_SRCS)
protoc --dart_out=lib/src -Iprotos --plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH)) $(PREGENERATED_SRCS)
rm lib/src/descriptor.pb{json,server}.dart
rm lib/src/dart_options.pb{enum,json,server}.dart
rm lib/src/plugin.pb{enum,json,server}.dart
rm lib/src/plugin.pb{json,server}.dart
dartfmt -w lib/src

protos: $(PLUGIN_PATH) $(TEST_PROTO_LIBS)
Expand Down
3 changes: 3 additions & 0 deletions protoc_plugin/lib/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ class CodeGenerator extends ProtobufContainer {
response.file.addAll(gen.generateFiles(config));
}
}
response.supportedFeatures =
Int64(CodeGeneratorResponse_Feature.FEATURE_PROTO3_OPTIONAL.value);

_streamOut.add(response.writeToBuffer());
});
}
Expand Down
37 changes: 21 additions & 16 deletions protoc_plugin/lib/message_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ class MessageGenerator extends ProtobufContainer {
final List<EnumGenerator> _enumGenerators = <EnumGenerator>[];
final List<MessageGenerator> _messageGenerators = <MessageGenerator>[];
final List<ExtensionGenerator> _extensionGenerators = <ExtensionGenerator>[];
// Stores the list of fields belonging to each oneof declaration identified
// by the index in the containing types's oneof_decl list.

/// Stores the list of fields belonging to each oneof declaration identified
/// by the index in the containing types's oneof_decl list.
/// Only contains the 'real' oneofs.
final List<List<ProtobufField>> _oneofFields;
List<OneofNames> _oneofNames;

Expand Down Expand Up @@ -92,7 +94,7 @@ class MessageGenerator extends ProtobufContainer {
? descriptor.name
: '${parent.fullName}.${descriptor.name}',
_oneofFields =
List.generate(descriptor.oneofDecl.length, (int index) => []) {
List.generate(countRealOneofs(descriptor), (int index) => []) {
mixin = _getMixin(declaredMixins, defaultMixin);
for (var i = 0; i < _descriptor.enumType.length; i++) {
var e = _descriptor.enumType[i];
Expand Down Expand Up @@ -195,7 +197,8 @@ class MessageGenerator extends ProtobufContainer {
_fieldList = <ProtobufField>[];
for (var names in members.fieldNames) {
var field = ProtobufField.message(names, this, ctx);
if (field.descriptor.hasOneofIndex()) {
if (field.descriptor.hasOneofIndex() &&
!field.descriptor.proto3Optional) {
_oneofFields[field.descriptor.oneofIndex].add(field);
}
_fieldList.add(field);
Expand Down Expand Up @@ -526,18 +529,20 @@ class MessageGenerator extends ProtobufContainer {
start: 'set '.length)
]);
}
_emitDeprecatedIf(field.isDeprecated, out);
_emitOverrideIf(field.overridesHasMethod, out);
_emitIndexAnnotation(field.number, out);
out.printlnAnnotated(
'$_coreImportPrefix.bool ${names.hasMethodName}() =>'
' \$_has(${field.index});',
[
NamedLocation(
name: names.hasMethodName,
fieldPathSegment: memberFieldPath,
start: '$_coreImportPrefix.bool '.length)
]);
if (field.hasPresence) {
_emitDeprecatedIf(field.isDeprecated, out);
_emitOverrideIf(field.overridesHasMethod, out);
_emitIndexAnnotation(field.number, out);
out.printlnAnnotated(
'$_coreImportPrefix.bool ${names.hasMethodName}() =>'
' \$_has(${field.index});',
[
NamedLocation(
name: names.hasMethodName,
fieldPathSegment: memberFieldPath,
start: '$_coreImportPrefix.bool '.length)
]);
}
_emitDeprecatedIf(field.isDeprecated, out);
_emitOverrideIf(field.overridesClearMethod, out);
_emitIndexAnnotation(field.number, out);
Expand Down
18 changes: 17 additions & 1 deletion protoc_plugin/lib/names.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// 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 'dart:math' as math;

import 'package:protobuf/meta.dart';
import 'package:protoc_plugin/src/dart_options.pb.dart';
import 'package:protoc_plugin/src/descriptor.pb.dart';
Expand Down Expand Up @@ -311,7 +313,8 @@ MemberNames messageMemberNames(DescriptorProto descriptor,
return [_defaultWhichMethodName(name), _defaultClearMethodName(name)];
}

for (var i = 0; i < descriptor.oneofDecl.length; i++) {
final realOneofCount = countRealOneofs(descriptor);
for (var i = 0; i < realOneofCount; i++) {
var oneof = descriptor.oneofDecl[i];

var oneofName = disambiguateName(
Expand Down Expand Up @@ -567,3 +570,16 @@ const _protobufEnumNames = <String>[

// List of names used in Dart enums, which can't be used as enum member names.
const _oneofEnumMemberNames = <String>['default', 'index', 'values'];

// Count the number of 'real' oneofs - that is oneofs not created for an
// optional proto3 field.
int countRealOneofs(DescriptorProto descriptor) {
var highestIndexSeen = -1;
for (final field in descriptor.field) {
if (field.hasOneofIndex() && !field.proto3Optional) {
highestIndexSeen = math.max(highestIndexSeen, field.oneofIndex);
}
}
// The number of entries is one higher than the highest seen index.
return highestIndexSeen + 1;
}
18 changes: 18 additions & 0 deletions protoc_plugin/lib/protobuf_field.dart
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,24 @@ class ProtobufField {
return generator._descriptor.options.hasMapEntry();
}

// `true` if this field should have a `hazzer` generated.
bool get hasPresence {
if (isRepeated) return false;
return true;
// TODO(sigurdm): to provide the correct semantics for non-optional proto3
// fields would need something like the following:
// return baseType.isMessage ||
// descriptor.proto3Optional ||
// parent.fileGen.descriptor.syntax == "proto2";
//
// This change would break any accidental uses of the proto3 hazzers, and
// would require some clean-up.
//
// We could consider keeping hazzers for proto3-oneof fields. There they
// seem useful and not breaking proto3 semantics, and dart protobuf uses it
// for example in package:protobuf/src/protobuf/mixins/well_known.dart.
}

/// Returns the expression to use for the Dart type.
///
/// This will be a List for repeated types.
Expand Down
1 change: 1 addition & 0 deletions protoc_plugin/lib/protoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ library protoc;
import 'dart:io';

import 'package:dart_style/dart_style.dart';
import 'package:fixnum/fixnum.dart';
import 'package:protobuf/protobuf.dart';
import 'package:path/path.dart' as path;

Expand Down
Loading

0 comments on commit 1319847

Please sign in to comment.