Allow application of external mixins to generated dart protos.
These are defined using the new dart_option "mixins".
For example usage, see test/protos/mixins.proto.
This change requires issue 2090033002 to be submitted first, which introduces protobuf version 0.5.2.
R=skybrian@google.com
Review URL: https://chromiumcodereview.appspot.com//2086253002 .
diff --git a/Makefile b/Makefile
index e1d95c4..3f9c344 100644
--- a/Makefile
+++ b/Makefile
@@ -21,6 +21,7 @@
enum_extension \
map_api \
map_api2 \
+ mixins \
multiple_files_test \
nested_extension \
non_nested_extension \
diff --git a/lib/file_generator.dart b/lib/file_generator.dart
index 65cba47..a921786 100644
--- a/lib/file_generator.dart
+++ b/lib/file_generator.dart
@@ -4,23 +4,89 @@
part of protoc;
+final _dartIdentifier = new RegExp(r'^\w+$');
+
/// Generates the Dart output files for one .proto input file.
///
/// Outputs include .pb.dart, pbenum.dart, and .pbjson.dart.
class FileGenerator extends ProtobufContainer {
- /// Returns the the mixin to use by default in this file,
- /// or null for no mixin by default.
- static PbMixin _getDefaultMixin(FileDescriptorProto desc) {
- if (!desc.hasOptions()) return null;
- if (!desc.options.hasExtension(Dart_options.defaultMixin)) {
- return null;
+ /// Reads and the declared mixins in the file, keyed by name.
+ ///
+ /// Performs some basic validation on declared mixins, e.g. whether names
+ /// are valid dart identifiers and whether there are cycles in the `parent`
+ /// hierarchy.
+ /// Does not check for existence of import files or classes.
+ static Map<String, PbMixin> _getDeclaredMixins(FileDescriptorProto desc) {
+ String mixinError(String error) =>
+ 'Option "mixins" in file ${desc.name}: $error';
+
+ if (!desc.hasOptions() ||
+ !desc.options.hasExtension(Dart_options.importedMixins)) {
+ return <String, PbMixin>{};
}
- var name = desc.options.getExtension(Dart_options.defaultMixin);
- PbMixin mixin = findMixin(name);
- if (mixin == null) {
- throw ("unknown mixin class: ${name}");
+ var dartMixins = <String, DartMixin>{};
+ ImportedMixins importedMixins =
+ desc.options.getExtension(Dart_options.importedMixins);
+ for (DartMixin mixin in importedMixins.mixins) {
+ if (dartMixins.containsKey(mixin.name)) {
+ throw mixinError('Duplicate mixin name: "${mixin.name}"');
+ }
+ if (!mixin.name.startsWith(_dartIdentifier)) {
+ throw mixinError(
+ '"${mixin.name}" is not a valid dart class identifier');
+ }
+ if (mixin.hasParent() && !mixin.parent.startsWith(_dartIdentifier)) {
+ throw mixinError('Mixin parent "${mixin.parent}" of "${mixin.name}" is '
+ 'not a valid dart class identifier');
+ }
+ dartMixins[mixin.name] = mixin;
}
- return mixin;
+
+ // Detect cycles and unknown parents.
+ for (var mixin in dartMixins.values) {
+ if (!mixin.hasParent()) continue;
+ var currentMixin = mixin;
+ var parentChain = <String>[];
+ while (currentMixin.hasParent()) {
+ var parentName = currentMixin.parent;
+
+ bool declaredMixin = dartMixins.containsKey(parentName);
+ bool internalMixin = !declaredMixin && findMixin(parentName) != null;
+
+ if (internalMixin) break; // No further validation of parent chain.
+
+ if (!declaredMixin) {
+ throw mixinError('Unknown mixin parent "${mixin.parent}" of '
+ '"${currentMixin.name}"');
+ }
+
+ if (parentChain.contains(parentName)) {
+ var cycle = parentChain.join('->') + '->$parentName';
+ throw mixinError('Cycle in parent chain: $cycle');
+ }
+ parentChain.add(parentName);
+ currentMixin = dartMixins[parentName];
+ }
+ }
+
+ // Turn DartMixins into PbMixins.
+ final pbMixins = <String, PbMixin>{};
+ PbMixin resolveMixin(String name) {
+ if (pbMixins.containsKey(name)) return pbMixins[name];
+ if (dartMixins.containsKey(name)) {
+ var dartMixin = dartMixins[name];
+ var pbMixin = new PbMixin(dartMixin.name,
+ importFrom: dartMixin.importFrom,
+ parent: resolveMixin(dartMixin.parent));
+ pbMixins[name] = pbMixin;
+ return pbMixin;
+ }
+ return findMixin(name);
+ }
+ for (var mixin in dartMixins.values) {
+ resolveMixin(mixin.name);
+ }
+ return pbMixins;
}
final FileDescriptorProto _fileDescriptor;
@@ -45,15 +111,23 @@
throw "FAILURE: Import with absolute path is not supported";
}
- var defaultMixin = _getDefaultMixin(_fileDescriptor);
+ var declaredMixins = _getDeclaredMixins(descriptor);
+ var defaultMixinName =
+ descriptor.options?.getExtension(Dart_options.defaultMixin) ?? '';
+ var defaultMixin =
+ declaredMixins[defaultMixinName] ?? findMixin(defaultMixinName);
+ if (defaultMixin == null && defaultMixinName.isNotEmpty) {
+ throw ('Option default_mixin on file ${descriptor.name}: Unknown mixin '
+ '$defaultMixinName');
+ }
// Load and register all enum and message types.
for (EnumDescriptorProto enumType in _fileDescriptor.enumType) {
enumGenerators.add(new EnumGenerator(enumType, this));
}
for (DescriptorProto messageType in _fileDescriptor.messageType) {
- messageGenerators
- .add(new MessageGenerator(messageType, this, defaultMixin));
+ messageGenerators.add(new MessageGenerator(
+ messageType, this, declaredMixins, defaultMixin));
}
for (FieldDescriptorProto extension in _fileDescriptor.extension) {
extensionGenerators.add(new ExtensionGenerator(extension, this));
diff --git a/lib/message_generator.dart b/lib/message_generator.dart
index 4dd64bb..40c6028 100644
--- a/lib/message_generator.dart
+++ b/lib/message_generator.dart
@@ -5,18 +5,23 @@
part of protoc;
class MessageGenerator extends ProtobufContainer {
- // Returns the mixin for this message, or null if none.
- static PbMixin _getMixin(DescriptorProto desc, PbMixin defaultValue) {
- if (!desc.hasOptions()) return defaultValue;
- if (!desc.options.hasExtension(Dart_options.mixin)) return defaultValue;
+ /// Returns the mixin for this message, or null if none.
+ ///
+ /// First searches [declaredMixins], then internal mixins declared by
+ /// [findMixin].
+ static PbMixin _getMixin(DescriptorProto desc,
+ Map<String, PbMixin> declaredMixins, PbMixin defaultMixin) {
+ PbMixin getMixinByName(String name) {
+ if (name.isEmpty) return null; // don't use a mixin (override any default)
+ return declaredMixins[name] ?? findMixin(name);
+ }
+
+ if (!desc.hasOptions() || !desc.options.hasExtension(Dart_options.mixin)) {
+ return defaultMixin;
+ }
String name = desc.options.getExtension(Dart_options.mixin);
- if (name.isEmpty) return null; // don't use a mixin (override any default)
- var mixin = findMixin(name);
- if (mixin == null) {
- throw ("unknown mixin class: ${name}");
- }
- return mixin;
+ return getMixinByName(name);
}
final String classname;
@@ -33,7 +38,7 @@
List<ProtobufField> _fieldList;
MessageGenerator(DescriptorProto descriptor, ProtobufContainer parent,
- PbMixin defaultMixin)
+ Map<String, PbMixin> declaredMixins, PbMixin defaultMixin)
: _descriptor = descriptor,
_parent = parent,
classname = (parent.classname == '')
@@ -44,13 +49,14 @@
: (parent.fqname == '.'
? '.${descriptor.name}'
: '${parent.fqname}.${descriptor.name}'),
- mixin = _getMixin(descriptor, defaultMixin) {
+ mixin = _getMixin(descriptor, declaredMixins, defaultMixin) {
for (EnumDescriptorProto e in _descriptor.enumType) {
_enumGenerators.add(new EnumGenerator(e, this));
}
for (DescriptorProto n in _descriptor.nestedType) {
- _messageGenerators.add(new MessageGenerator(n, this, defaultMixin));
+ _messageGenerators
+ .add(new MessageGenerator(n, this, declaredMixins, defaultMixin));
}
for (FieldDescriptorProto x in _descriptor.extension) {
diff --git a/lib/protoc.dart b/lib/protoc.dart
index f8fde96..94cb333 100644
--- a/lib/protoc.dart
+++ b/lib/protoc.dart
@@ -3,17 +3,17 @@
import 'dart:async';
import 'dart:io';
-import 'package:protobuf/protobuf.dart';
import 'package:protobuf/mixins_meta.dart';
+import 'package:protobuf/protobuf.dart';
import 'package:path/path.dart' as path;
+import 'src/dart_options.pb.dart';
import 'src/descriptor.pb.dart';
import 'src/plugin.pb.dart';
-import 'src/dart_options.pb.dart';
import 'const_generator.dart' show writeJsonConst;
-import 'names.dart';
import 'indenting_writer.dart' show IndentingWriter;
+import 'names.dart';
part 'base_type.dart';
part 'client_generator.dart';
diff --git a/lib/src/dart_options.pb.dart b/lib/src/dart_options.pb.dart
index ea4f483..92961dd 100644
--- a/lib/src/dart_options.pb.dart
+++ b/lib/src/dart_options.pb.dart
@@ -5,13 +5,110 @@
import 'package:protobuf/protobuf.dart';
+class DartOptions extends GeneratedMessage {
+ static final BuilderInfo _i = new BuilderInfo('DartOptions')
+ ..hasRequiredFields = false
+ ;
+
+ DartOptions() : super();
+ DartOptions.fromBuffer(List<int> i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r);
+ DartOptions.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r);
+ DartOptions clone() => new DartOptions()..mergeFromMessage(this);
+ BuilderInfo get info_ => _i;
+ static DartOptions create() => new DartOptions();
+ static PbList<DartOptions> createRepeated() => new PbList<DartOptions>();
+ static DartOptions getDefault() {
+ if (_defaultInstance == null) _defaultInstance = new _ReadonlyDartOptions();
+ return _defaultInstance;
+ }
+ static DartOptions _defaultInstance;
+ static void $checkItem(DartOptions v) {
+ if (v is !DartOptions) checkItemFailed(v, 'DartOptions');
+ }
+}
+
+class _ReadonlyDartOptions extends DartOptions with ReadonlyMessageMixin {}
+
+class DartMixin extends GeneratedMessage {
+ static final BuilderInfo _i = new BuilderInfo('DartMixin')
+ ..a/*<String>*/(1, 'name', PbFieldType.OS)
+ ..a/*<String>*/(2, 'importFrom', PbFieldType.OS)
+ ..a/*<String>*/(3, 'parent', PbFieldType.OS)
+ ..hasRequiredFields = false
+ ;
+
+ DartMixin() : super();
+ DartMixin.fromBuffer(List<int> i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r);
+ DartMixin.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r);
+ DartMixin clone() => new DartMixin()..mergeFromMessage(this);
+ BuilderInfo get info_ => _i;
+ static DartMixin create() => new DartMixin();
+ static PbList<DartMixin> createRepeated() => new PbList<DartMixin>();
+ static DartMixin getDefault() {
+ if (_defaultInstance == null) _defaultInstance = new _ReadonlyDartMixin();
+ return _defaultInstance;
+ }
+ static DartMixin _defaultInstance;
+ static void $checkItem(DartMixin v) {
+ if (v is !DartMixin) checkItemFailed(v, 'DartMixin');
+ }
+
+ String get name => $_get(0, 1, '');
+ void set name(String v) { $_setString(0, 1, v); }
+ bool hasName() => $_has(0, 1);
+ void clearName() => clearField(1);
+
+ String get importFrom => $_get(1, 2, '');
+ void set importFrom(String v) { $_setString(1, 2, v); }
+ bool hasImportFrom() => $_has(1, 2);
+ void clearImportFrom() => clearField(2);
+
+ String get parent => $_get(2, 3, '');
+ void set parent(String v) { $_setString(2, 3, v); }
+ bool hasParent() => $_has(2, 3);
+ void clearParent() => clearField(3);
+}
+
+class _ReadonlyDartMixin extends DartMixin with ReadonlyMessageMixin {}
+
+class ImportedMixins extends GeneratedMessage {
+ static final BuilderInfo _i = new BuilderInfo('ImportedMixins')
+ ..pp/*<DartMixin>*/(1, 'mixins', PbFieldType.PM, DartMixin.$checkItem, DartMixin.create)
+ ..hasRequiredFields = false
+ ;
+
+ ImportedMixins() : super();
+ ImportedMixins.fromBuffer(List<int> i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r);
+ ImportedMixins.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r);
+ ImportedMixins clone() => new ImportedMixins()..mergeFromMessage(this);
+ BuilderInfo get info_ => _i;
+ static ImportedMixins create() => new ImportedMixins();
+ static PbList<ImportedMixins> createRepeated() => new PbList<ImportedMixins>();
+ static ImportedMixins getDefault() {
+ if (_defaultInstance == null) _defaultInstance = new _ReadonlyImportedMixins();
+ return _defaultInstance;
+ }
+ static ImportedMixins _defaultInstance;
+ static void $checkItem(ImportedMixins v) {
+ if (v is !ImportedMixins) checkItemFailed(v, 'ImportedMixins');
+ }
+
+ List<DartMixin> get mixins => $_get(0, 1, null);
+}
+
+class _ReadonlyImportedMixins extends ImportedMixins with ReadonlyMessageMixin {}
+
class Dart_options {
+ static final Extension importedMixins = new Extension<ImportedMixins>('FileOptions', 'importedMixins', 28125061, PbFieldType.OM, ImportedMixins.getDefault, ImportedMixins.create);
static final Extension defaultMixin = new Extension<String>('FileOptions', 'defaultMixin', 96128839, PbFieldType.OS);
static final Extension mixin = new Extension<String>('MessageOptions', 'mixin', 96128839, PbFieldType.OS);
+ static final Extension override = new Extension<bool>('FieldOptions', 'override', 28205290, PbFieldType.OB);
static final Extension dartName = new Extension<String>('FieldOptions', 'dartName', 28700919, PbFieldType.OS);
static void registerAllExtensions(ExtensionRegistry registry) {
+ registry.add(importedMixins);
registry.add(defaultMixin);
registry.add(mixin);
+ registry.add(override);
registry.add(dartName);
}
}
diff --git a/lib/testing/mixins.dart b/lib/testing/mixins.dart
new file mode 100644
index 0000000..694840c
--- /dev/null
+++ b/lib/testing/mixins.dart
@@ -0,0 +1,15 @@
+abstract class Mixin1 {
+ String get overriddenString => "mixin1";
+
+ String get interfaceString;
+ set interfaceString(String string);
+ bool hasInterfaceString();
+}
+
+abstract class Mixin2 {
+ String get overriddenString => "mixin2";
+
+ bool hasOverriddenHasMethod() => false;
+}
+
+abstract class Mixin3 {}
diff --git a/pubspec.yaml b/pubspec.yaml
index 7bab074..4acb45c 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -8,7 +8,7 @@
dependencies:
fixnum: ^0.10.5
path: ^1.0.0
- protobuf: ^0.5.1
+ protobuf: ^0.5.2
dev_dependencies:
browser: any
test: ^0.12.0
diff --git a/test/all_tests.dart b/test/all_tests.dart
index c0e1e9d..3e6cbf9 100755
--- a/test/all_tests.dart
+++ b/test/all_tests.dart
@@ -18,6 +18,7 @@
import 'map_test.dart' as map;
import 'message_generator_test.dart' as message_generator;
import 'message_test.dart' as message;
+import 'mixin_test.dart' as mixin;
import 'protoc_options_test.dart' as protoc_options;
import 'repeated_field_test.dart' as repeated_field;
import 'service_test.dart' as service;
@@ -40,6 +41,7 @@
map.main();
message_generator.main();
message.main();
+ mixin.main();
protoc_options.main();
repeated_field.main();
service.main();
diff --git a/test/message_generator_test.dart b/test/message_generator_test.dart
index 9cb0a33..e28c401 100755
--- a/test/message_generator_test.dart
+++ b/test/message_generator_test.dart
@@ -7,9 +7,10 @@
import 'package:protoc_plugin/indenting_writer.dart';
import 'package:protoc_plugin/protoc.dart';
+import 'package:test/test.dart';
+
import 'package:protoc_plugin/src/descriptor.pb.dart';
import 'package:protoc_plugin/src/plugin.pb.dart';
-import 'package:test/test.dart';
void main() {
test('testMessageGenerator', () {
@@ -127,7 +128,7 @@
new CodeGeneratorRequest(), new CodeGeneratorResponse());
FileGenerator fg = new FileGenerator(fd);
- MessageGenerator mg = new MessageGenerator(md, fg, null);
+ MessageGenerator mg = new MessageGenerator(md, fg, {}, null);
var ctx = new GenerationContext(options);
mg.register(ctx);
diff --git a/test/mixin_test.dart b/test/mixin_test.dart
new file mode 100644
index 0000000..2e8c789
--- /dev/null
+++ b/test/mixin_test.dart
@@ -0,0 +1,67 @@
+import 'package:test/test.dart';
+
+import '../out/protos/mixins.pb.dart' as pb;
+import 'package:protoc_plugin/testing/mixins.dart';
+
+void main() {
+ group('Proto with Mixin1', () {
+ pb.Mixin1PB proto;
+
+ setUp(() {
+ proto = new pb.Mixin1PB();
+ });
+
+ test('is a Mixin1', () {
+ expect(proto is Mixin1, isTrue);
+ expect(proto is Mixin2, isFalse);
+ });
+
+ test('implements interface defined by mixins', () {
+ proto.interfaceString = 'test';
+ expect(proto.hasInterfaceString(), isTrue);
+ expect(proto.interfaceString, equals('test'));
+ });
+ });
+
+ group('Proto with Mixin2', () {
+ pb.Mixin2PB proto;
+
+ setUp(() {
+ proto = new pb.Mixin2PB();
+ });
+
+ test('overrides has method', () {
+ expect(proto.hasOverriddenHasMethod(), isFalse);
+ proto.overriddenHasMethod = 'test';
+
+ expect(proto.hasOverriddenHasMethod(), isTrue);
+ });
+ });
+
+ group('Proto without mixins', () {
+ pb.NoMixinPB proto;
+
+ setUp(() {
+ proto = new pb.NoMixinPB();
+ });
+
+ test('is neither Mixin1 nor Mixin2', () {
+ expect(proto is Mixin1, isFalse);
+ expect(proto is Mixin2, isFalse);
+ });
+ });
+
+ group('Proto with Mixin3', () {
+ pb.Mixin3PB proto;
+
+ setUp(() {
+ proto = new pb.Mixin3PB();
+ });
+
+ test('is both Mixin1 (from parent) and Mixin3', () {
+ expect(proto is Mixin1, isTrue);
+ expect(proto is Mixin2, isFalse);
+ expect(proto is Mixin3, isTrue);
+ });
+ });
+}
diff --git a/test/protos/dart_options.proto b/test/protos/dart_options.proto
index 7a16d81..7de2170 100644
--- a/test/protos/dart_options.proto
+++ b/test/protos/dart_options.proto
@@ -10,8 +10,38 @@
import "descriptor_2_5_opensource.proto";
+// A mixin to be applied in the 'with' clause of a generated proto in dart.
+message DartMixin {
+ // Class name of the mixin class.
+ optional string name = 1;
+
+ // File from which the mixin class is imported.
+ // The generated Dart code will use this in an import statement.
+ optional string import_from = 2;
+
+ // Used to apply multiple mixins to a proto.
+ // The mixin listed in parent will always be applied before this one,
+ // making the generated proto implement both mixins.
+ optional string parent = 3;
+}
+
+// Defines mixins imported from dart files to be used on messages in this file.
+message ImportedMixins {
+
+ // The DartMixins to be imported.
+ repeated DartMixin mixins = 1;
+}
+
extend google.protobuf.FileOptions {
+ // Defines the named mixins to be used on messages in this file.
+ // These mixins are in addition to internally defined mixins (e.g PbMapMixin)
+ // and may override them.
+ // Warning: there is no checking at compile time for name conflicts between
+ // identifiers defined in the mixin and proto-generated identifiers.
+ // Running dartanalyzer on the generated file might find some of them.
+ optional ImportedMixins imported_mixins = 28125061;
+
// Applies the named mixin to all messages in this file.
// (May be overridden by the "mixin" option on a message.)
// For now, "PbMapMixin" is the only available mixin.
diff --git a/test/protos/mixins.proto b/test/protos/mixins.proto
new file mode 100644
index 0000000..fccd674
--- /dev/null
+++ b/test/protos/mixins.proto
@@ -0,0 +1,41 @@
+syntax = "proto2";
+
+import "dart_options.proto";
+
+option (dart_options.imported_mixins) = {
+ mixins: [{
+ name: "Mixin1"
+ import_from: "package:protoc_plugin/testing/mixins.dart"
+ },
+ {
+ name: "Mixin2"
+ import_from: "package:protoc_plugin/testing/mixins.dart"
+ },
+ {
+ name: "Mixin3"
+ import_from: "package:protoc_plugin/testing/mixins.dart"
+ parent: "Mixin1"
+ }];
+};
+
+option (dart_options.default_mixin) = "Mixin1";
+
+message NoMixinPB {
+ option (dart_options.mixin) = "";
+}
+
+message Mixin1PB {
+ optional string interface_string = 1;
+}
+
+message Mixin2PB {
+ option (dart_options.mixin) = "Mixin2";
+
+ optional string overridden_has_method = 3;
+}
+
+message Mixin3PB {
+ option (dart_options.mixin) = "Mixin3";
+
+ optional string interface_string = 1;
+}
