Move protobuf enums to a separate .pbenum.dart file.
This is to reduce the amount of Dart code that Dartium needs to
load when only the enums from a .proto file are needed.
Reviewed internally: cl/123342330
diff --git a/lib/bazel.dart b/lib/bazel.dart
index 6bfe90a..f66cb65 100644
--- a/lib/bazel.dart
+++ b/lib/bazel.dart
@@ -105,7 +105,7 @@
}
@override
- Uri outputPathFor(Uri input) {
+ Uri outputPathFor(Uri input, String extension) {
var pkg = _findPackage(input.path);
if (pkg == null) {
throw new ArgumentError('Unable to locate package for input $input.');
@@ -113,14 +113,15 @@
// Bazel package-relative paths.
var relativeInput = input.path.substring('${pkg.input_root}/'.length);
- var relativeOutput = replacePathExtension(relativeInput);
- var outputPath = p.join(pkg.output_root, relativeOutput);
+ var base = p.withoutExtension(relativeInput);
+ var outputPath = p.join(pkg.output_root, "$base$extension");
return new Uri.file(outputPath);
}
@override
- Uri resolveImport(Uri target, Uri source) {
- var targetUri = _packageUriFor(replacePathExtension(target.path));
+ Uri resolveImport(Uri target, Uri source, String extension) {
+ var targetBase = p.withoutExtension(target.path);
+ var targetUri = _packageUriFor("$targetBase$extension");
var sourceUri = _packageUriFor(source.path);
if (targetUri == null && sourceUri != null) {
@@ -134,7 +135,7 @@
return targetUri.uri;
}
- return super.resolveImport(target, source);
+ return super.resolveImport(target, source, extension);
}
_PackageUri _packageUriFor(String target) {
diff --git a/lib/code_generator.dart b/lib/code_generator.dart
index 2072de2..92561a4 100644
--- a/lib/code_generator.dart
+++ b/lib/code_generator.dart
@@ -66,6 +66,7 @@
var name = gen._fileDescriptor.name;
if (request.fileToGenerate.contains(name)) {
response.file.add(gen.generateResponse(config));
+ response.file.add(gen.generateEnumResponse(config));
response.file.add(gen.generateJsonDartResponse(config));
}
}
diff --git a/lib/extension_generator.dart b/lib/extension_generator.dart
index 91260aa..e675750 100644
--- a/lib/extension_generator.dart
+++ b/lib/extension_generator.dart
@@ -44,15 +44,20 @@
///
/// For each .pb.dart file that the generated code needs to import,
/// add its generator.
- void addImportsTo(Set<FileGenerator> imports) {
+ void addImportsTo(
+ Set<FileGenerator> imports, Set<FileGenerator> enumImports) {
if (_field == null) throw new StateError("resolve not called");
var typeGen = _field.baseType.generator;
if (typeGen != null && typeGen.fileGen != fileGen) {
// The type of this extension is defined in a different file,
// so we need to import it.
+ if (typeGen is EnumGenerator) {
+ enumImports.add(typeGen.fileGen);
+ } else {
imports.add(typeGen.fileGen);
}
}
+ }
/// Adds dependencies of [generateConstants] to [imports].
///
diff --git a/lib/file_generator.dart b/lib/file_generator.dart
index 76c5a7f..f1f78f2 100644
--- a/lib/file_generator.dart
+++ b/lib/file_generator.dart
@@ -118,7 +118,7 @@
Uri filePath = new Uri.file(_fileDescriptor.name);
return new CodeGeneratorResponse_File()
- ..name = config.outputPathFor(filePath).path
+ ..name = config.outputPathFor(filePath, ".pb.dart").path
..content = out.toString();
}
@@ -130,9 +130,6 @@
generateHeader(out, config);
// Generate code.
- for (EnumGenerator e in enumGenerators) {
- e.generate(out);
- }
for (MessageGenerator m in messageGenerators) {
m.generate(out);
}
@@ -181,7 +178,11 @@
if (_needsFixnumImport) {
out.println("import 'package:fixnum/fixnum.dart';");
}
+
+ if (_needsProtobufImport) {
out.println("import 'package:protobuf/protobuf.dart';");
+ out.println();
+ }
var mixinImports = findMixinsToImport();
var importNames = mixinImports.keys.toList();
@@ -190,23 +191,46 @@
var symbols = mixinImports[imp];
out.println("import '${imp}' show ${symbols.join(', ')};");
}
+ if (mixinImports.isNotEmpty) out.println();
// Import the .pb.dart files we depend on.
- for (var imported in _findProtosToImport()) {
+ var imports = new Set<FileGenerator>.identity();
+ var enumImports = new Set<FileGenerator>.identity();
+ _findProtosToImport(imports, enumImports);
+
+ void writeImport(FileGenerator target, String extension) {
Uri resolvedImport =
- config.resolveImport(imported.protoFileUri, protoFileUri);
+ config.resolveImport(target.protoFileUri, protoFileUri, extension);
out.print("import '$resolvedImport'");
- if (package != imported.package && imported.package.isNotEmpty) {
- out.print(' as ${imported.packageImportPrefix}');
+ if (package != target.package && target.package.isNotEmpty) {
+ out.print(' as ${target.packageImportPrefix}');
}
out.println(';');
}
- out.println();
+
+ for (var target in imports) {
+ writeImport(target, ".pb.dart");
+ }
+ if (imports.isNotEmpty) out.println();
+
+ for (var target in enumImports) {
+ writeImport(target, ".pbenum.dart");
+ }
+ if (enumImports.isNotEmpty) out.println();
// Services also depend on the json imports.
if (serviceGenerators.isNotEmpty) {
- Uri resolvedImport = config.resolveJsonImport(protoFileUri, protoFileUri);
- out.print("import '$resolvedImport';");
+ Uri resolvedImport =
+ config.resolveImport(protoFileUri, protoFileUri, ".pbjson.dart");
+ out.println("import '$resolvedImport';");
+ out.println();
+ }
+
+ // Export enums in main file for backward compatibility.
+ if (enumCount > 0) {
+ Uri resolvedImport =
+ config.resolveImport(protoFileUri, protoFileUri, ".pbenum.dart");
+ out.println("export '$resolvedImport';");
out.println();
}
}
@@ -221,20 +245,26 @@
return false;
}
+ bool get _needsProtobufImport =>
+ messageGenerators.isNotEmpty ||
+ extensionGenerators.isNotEmpty ||
+ clientApiGenerators.isNotEmpty ||
+ serviceGenerators.isNotEmpty;
+
/// Returns the generator for each .pb.dart file we need to import.
- Set<FileGenerator> _findProtosToImport() {
- var imports = new Set<FileGenerator>.identity();
+ void _findProtosToImport(
+ Set<FileGenerator> imports, Set<FileGenerator> enumImports) {
for (var m in messageGenerators) {
- m.addImportsTo(imports);
+ m.addImportsTo(imports, enumImports);
}
for (var x in extensionGenerators) {
- x.addImportsTo(imports);
+ x.addImportsTo(imports, enumImports);
}
for (var x in serviceGenerators) {
x.addImportsTo(imports);
}
- imports.remove(this); // Don't need to import self.
- return imports;
+ // Don't need to import self. (But we may need to import the enums.)
+ imports.remove(this);
}
/// Returns a map from import names to the Dart symbols to be imported.
@@ -262,6 +292,60 @@
return imports;
}
+ CodeGeneratorResponse_File generateEnumResponse(OutputConfiguration config) {
+ if (!_linked) throw new StateError("not linked");
+
+ IndentingWriter out = new IndentingWriter();
+
+ generateEnumFile(out, config);
+
+ Uri filePath = new Uri.file(_fileDescriptor.name);
+ return new CodeGeneratorResponse_File()
+ ..name = config.outputPathFor(filePath, ".pbenum.dart").path
+ ..content = out.toString();
+ }
+
+ void generateEnumFile(IndentingWriter out,
+ [OutputConfiguration config = const DefaultOutputConfiguration()]) {
+ Uri filePath = new Uri.file(_fileDescriptor.name);
+ if (filePath.isAbsolute) {
+ // protoc should never generate a file descriptor with an absolute path.
+ throw "FAILURE: File with an absolute path is not supported";
+ }
+
+ var baseLibraryName = _generateLibraryName(filePath);
+ var libraryName = baseLibraryName + "_pbenum";
+ out.print('''
+///
+// Generated code. Do not modify.
+///
+library $libraryName;
+
+''');
+
+ if (enumCount > 0) {
+ out.println("import 'package:protobuf/protobuf.dart';");
+ out.println();
+ }
+
+ for (EnumGenerator e in enumGenerators) {
+ e.generate(out);
+ }
+
+ for (MessageGenerator m in messageGenerators) {
+ m.generateEnums(out);
+ }
+ }
+
+ /// Returns the number of enum types generated in the .pbenum.dart file.
+ int get enumCount {
+ var count = enumGenerators.length;
+ for (MessageGenerator m in messageGenerators) {
+ count += m.enumCount;
+ }
+ return count;
+ }
+
CodeGeneratorResponse_File generateJsonDartResponse(
OutputConfiguration config) {
if (!_linked) throw new StateError("not linked");
@@ -272,7 +356,7 @@
Uri filePath = new Uri.file(_fileDescriptor.name);
return new CodeGeneratorResponse_File()
- ..name = config.jsonDartOutputPathFor(filePath).path
+ ..name = config.outputPathFor(filePath, ".pbjson.dart").path
..content = out.toString();
}
@@ -297,8 +381,8 @@
// Import the .pbjson.dart files we depend on.
var importList = _findJsonProtosToImport();
for (var imported in importList) {
- Uri resolvedImport =
- config.resolveJsonImport(imported.protoFileUri, protoFileUri);
+ Uri resolvedImport = config.resolveImport(
+ imported.protoFileUri, protoFileUri, ".pbjson.dart");
out.print("import '$resolvedImport'");
if (package != imported.package && imported.package.isNotEmpty) {
out.print(' as ${imported.packageImportPrefix}');
diff --git a/lib/message_generator.dart b/lib/message_generator.dart
index 4384d88..a3d47b6 100644
--- a/lib/message_generator.dart
+++ b/lib/message_generator.dart
@@ -197,24 +197,34 @@
///
/// For each .pb.dart file that the generated code needs to import,
/// add its generator.
- void addImportsTo(Set<FileGenerator> imports) {
+ void addImportsTo(
+ Set<FileGenerator> imports, Set<FileGenerator> enumImports) {
if (_fieldList == null) throw new StateError("message not resolved");
for (var field in _fieldList) {
var typeGen = field.baseType.generator;
- if (typeGen != null && typeGen.fileGen != fileGen) {
- // The field's type is defined in a different .pb.dart file.
- // Therefore we need to import it.
+ if (typeGen is EnumGenerator) {
+ enumImports.add(typeGen.fileGen);
+ } else if (typeGen != null) {
imports.add(typeGen.fileGen);
}
}
for (var m in _messageGenerators) {
- m.addImportsTo(imports);
+ m.addImportsTo(imports, enumImports);
}
for (var x in _extensionGenerators) {
- x.addImportsTo(imports);
+ x.addImportsTo(imports, enumImports);
}
}
+ // Returns the number of enums in this message and all nested messages.
+ int get enumCount {
+ var count = _enumGenerators.length;
+ for (var m in _messageGenerators) {
+ count += m.enumCount;
+ }
+ return count;
+ }
+
/// Adds dependencies of [generateConstants] to [imports].
///
/// For each .pbjson.dart file that the generated code needs to import,
@@ -241,10 +251,6 @@
_methodNames.addAll(mixin.findReservedNames());
}
- for (EnumGenerator e in _enumGenerators) {
- e.generate(out);
- }
-
for (MessageGenerator m in _messageGenerators) {
m.generate(out);
}
@@ -408,6 +414,16 @@
}
}
+ void generateEnums(IndentingWriter out) {
+ for (EnumGenerator e in _enumGenerators) {
+ e.generate(out);
+ }
+
+ for (MessageGenerator m in _messageGenerators) {
+ m.generateEnums(out);
+ }
+ }
+
/// Writes a Dart constant containing the JSON for the ProtoDescriptor.
/// Also writes a separate constant for each nested message,
/// to avoid duplication.
diff --git a/lib/output_config.dart b/lib/output_config.dart
index 481d03d..534f24e 100644
--- a/lib/output_config.dart
+++ b/lib/output_config.dart
@@ -9,46 +9,20 @@
abstract class OutputConfiguration {
const OutputConfiguration();
- /// Returns [filePath] with it's extension replaced with '.pb.dart'.
- String replacePathExtension(String filePath) =>
- '${path.withoutExtension(filePath)}.pb.dart';
-
- /// Returns [file] with it's extension replaced with '.pb.dart'.
- Uri replaceUriExtension(Uri file) =>
- path.url.toUri(replacePathExtension(path.url.fromUri(file)));
-
- /// Resolves an import of a .pb.dart file.
+ /// Resolves an import of a generated Dart file.
///
/// Both [source] and [target] are .proto files, where [source] imports
/// [target].
///
- /// The returned URI can be used within the source's .pb.dart file to
- /// import the target's .pb.dart file.
- Uri resolveImport(Uri target, Uri source);
-
- /// Resolves an import of a .pbjson.dart file.
- ///
- /// Both [source] and [target] are .proto files, where [source] imports
- /// [target].
- ///
- /// The returned URI can be used within the source's .pbjson.dart file to
- /// import the target's .pbjson.dart file.
- Uri resolveJsonImport(Uri target, Uri source);
+ /// The returned URI can be used within one of the source's dart files to
+ /// import the target's generated file with the given extension.
+ Uri resolveImport(Uri target, Uri source, String extension);
/// Returns the path where the .pb.dart file will be placed.
///
/// The input is a .proto file and the output is a path under the output
/// folder.
- Uri outputPathFor(Uri inputPath);
-
- /// Returns the path where the .pbjson.dart file will be place.
- ///
- /// The input is a .proto file and the output is a path under the output
- /// folder.
- ///
- /// (This file making data from the .proto file available as
- /// constants in Dart. The constants are JSON-encoded protobufs.)
- Uri jsonDartOutputPathFor(Uri inputPath);
+ Uri outputPathFor(Uri inputPath, String extension);
}
/// Default [OutputConfiguration] that uses the same path as the input
@@ -58,28 +32,17 @@
const DefaultOutputConfiguration();
@override
- Uri outputPathFor(Uri input) => replaceUriExtension(input);
-
- @override
- Uri jsonDartOutputPathFor(Uri input) {
+ Uri outputPathFor(Uri input, String extension) {
var base = path.withoutExtension(path.url.fromUri(input));
- return path.url.toUri('$base.pbjson.dart');
+ return path.url.toUri('$base$extension');
}
@override
- Uri resolveImport(Uri target, Uri source) {
- var targetPath = path.url.fromUri(target);
- var sourceDir = path.url.dirname(path.url.fromUri(source));
- return path.url.toUri(
- replacePathExtension(path.url.relative(targetPath, from: sourceDir)));
- }
-
- @override
- Uri resolveJsonImport(Uri target, Uri source) {
+ Uri resolveImport(Uri target, Uri source, String extension) {
var targetPath = path.url.fromUri(target);
var sourceDir = path.url.dirname(path.url.fromUri(source));
var base =
path.withoutExtension(path.url.relative(targetPath, from: sourceDir));
- return path.url.toUri('$base.pbjson.dart');
+ return path.url.toUri('$base$extension');
}
}
diff --git a/test/bazel_test.dart b/test/bazel_test.dart
index 7f1773d..64ed08a 100644
--- a/test/bazel_test.dart
+++ b/test/bazel_test.dart
@@ -112,7 +112,7 @@
group('BazelOutputConfiguration', () {
Map<String, BazelPackage> packages;
- var config;
+ BazelOutputConfiguration config;
setUp(() {
packages = {
@@ -126,28 +126,34 @@
group('outputPathForUri', () {
test('should handle files at package root', () {
- var p = config.outputPathFor(Uri.parse('foo/bar/quux.proto'));
+ var p =
+ config.outputPathFor(Uri.parse('foo/bar/quux.proto'), '.pb.dart');
expect(p.path, 'baz/flob/quux.pb.dart');
});
test('should handle files below package root', () {
- var p = config.outputPathFor(Uri.parse('foo/bar/a/b/quux.proto'));
+ var p = config.outputPathFor(
+ Uri.parse('foo/bar/a/b/quux.proto'), '.pb.dart');
expect(p.path, 'baz/flob/a/b/quux.pb.dart');
});
test('should handle files in a nested package root', () {
- var p = config.outputPathFor(Uri.parse('foo/bar/baz/quux.proto'));
+ var p = config.outputPathFor(
+ Uri.parse('foo/bar/baz/quux.proto'), '.pb.dart');
expect(p.path, 'baz/flob/foo/quux.pb.dart');
});
test('should handle files below a nested package root', () {
- var p = config.outputPathFor(Uri.parse('foo/bar/baz/a/b/quux.proto'));
+ var p = config.outputPathFor(
+ Uri.parse('foo/bar/baz/a/b/quux.proto'), '.pb.dart');
expect(p.path, 'baz/flob/foo/a/b/quux.pb.dart');
});
test('should throw if unable to locate the package for an input', () {
expect(
- () => config.outputPathFor(Uri.parse('a/b/c/quux.proto')), throws);
+ () =>
+ config.outputPathFor(Uri.parse('a/b/c/quux.proto'), '.pb.dart'),
+ throws);
});
});
@@ -155,28 +161,28 @@
test('should emit relative import if in same package', () {
var target = Uri.parse('foo/bar/quux.proto');
var source = Uri.parse('foo/bar/baz.proto');
- var uri = config.resolveImport(target, source);
+ var uri = config.resolveImport(target, source, '.pb.dart');
expect(uri.path, 'quux.pb.dart');
});
test('should emit relative import if in subdir of same package', () {
var target = Uri.parse('foo/bar/a/b/quux.proto');
var source = Uri.parse('foo/bar/baz.proto');
- var uri = config.resolveImport(target, source);
+ var uri = config.resolveImport(target, source, '.pb.dart');
expect(uri.path, 'a/b/quux.pb.dart');
});
test('should emit relative import if in parent dir in same package', () {
var target = Uri.parse('foo/bar/quux.proto');
var source = Uri.parse('foo/bar/a/b/baz.proto');
- var uri = config.resolveImport(target, source);
+ var uri = config.resolveImport(target, source, '.pb.dart');
expect(uri.path, '../../quux.pb.dart');
});
test('should emit package: import if in different package', () {
var target = Uri.parse('wibble/wobble/quux.proto');
var source = Uri.parse('foo/bar/baz.proto');
- var uri = config.resolveImport(target, source);
+ var uri = config.resolveImport(target, source, '.pb.dart');
expect(uri.scheme, 'package');
expect(uri.path, 'wibble.wobble/quux.pb.dart');
});
@@ -184,7 +190,7 @@
test('should emit package: import if in subdir of different package', () {
var target = Uri.parse('wibble/wobble/foo/bar/quux.proto');
var source = Uri.parse('foo/bar/baz.proto');
- var uri = config.resolveImport(target, source);
+ var uri = config.resolveImport(target, source, '.pb.dart');
expect(uri.scheme, 'package');
expect(uri.path, 'wibble.wobble/foo/bar/quux.pb.dart');
});
@@ -192,7 +198,7 @@
test('should throw if target is in unknown package', () {
var target = Uri.parse('flob/flub/quux.proto');
var source = Uri.parse('foo/bar/baz.proto');
- expect(() => config.resolveImport(target, source), throws);
+ expect(() => config.resolveImport(target, source, '.pb.dart'), throws);
});
});
});
diff --git a/test/file_generator_test.dart b/test/file_generator_test.dart
index a8c98e4..a05a12f 100644
--- a/test/file_generator_test.dart
+++ b/test/file_generator_test.dart
@@ -162,14 +162,25 @@
expect(writer.toString(), expected);
});
- test('FileGenerator generates a .pb.dart file for a top-level enum', () {
+ test('FileGenerator generates files for a top-level enum', () {
// NOTE: Below > 80 cols because it is matching generated code > 80 cols.
+
String expected = r'''
///
// Generated code. Do not modify.
///
library test;
+export 'test.pbenum.dart';
+
+''';
+
+ String expectedEnum = r'''
+///
+// Generated code. Do not modify.
+///
+library test_pbenum;
+
import 'package:protobuf/protobuf.dart';
class PhoneType extends ProtobufEnum {
@@ -195,6 +206,7 @@
}
''';
+
FileDescriptorProto fd =
buildFileDescriptor(phoneNumber: false, topLevelEnum: true);
var options = parseGenerationOptions(
@@ -206,6 +218,10 @@
var writer = new IndentingWriter();
fg.generate(writer);
expect(writer.toString(), expected);
+
+ writer = new IndentingWriter();
+ fg.generateEnumFile(writer);
+ expect(writer.toString(), expectedEnum);
});
test('FileGenerator generates a .pbjson.dart file for a top-level enum', () {
@@ -308,6 +324,7 @@
import 'package:protobuf/protobuf.dart';
import 'test.pbjson.dart';
+
''';
FileDescriptorProto fd = new FileDescriptorProto()
..name = 'test'
@@ -400,6 +417,7 @@
library test;
import 'package:protobuf/protobuf.dart';
+
import 'package1.pb.dart' as p1;
import 'package2.pb.dart' as p2;
diff --git a/test/message_generator_test.dart b/test/message_generator_test.dart
index 7b7eaa4..9cb0a33 100755
--- a/test/message_generator_test.dart
+++ b/test/message_generator_test.dart
@@ -14,7 +14,7 @@
void main() {
test('testMessageGenerator', () {
// NOTE: Below > 80 cols because it is matching generated code > 80 cols.
- String expected = r'''
+ String expectedEnums = r'''
class PhoneNumber_PhoneType extends ProtobufEnum {
static const PhoneNumber_PhoneType MOBILE = const PhoneNumber_PhoneType._(0, 'MOBILE');
static const PhoneNumber_PhoneType HOME = const PhoneNumber_PhoneType._(1, 'HOME');
@@ -37,6 +37,9 @@
const PhoneNumber_PhoneType._(int v, String n) : super(v, n);
}
+''';
+
+ String expected = r'''
class PhoneNumber extends GeneratedMessage {
static final BuilderInfo _i = new BuilderInfo('PhoneNumber')
..a/*<String>*/(1, 'number', PbFieldType.QS)
@@ -133,5 +136,9 @@
var writer = new IndentingWriter();
mg.generate(writer);
expect(writer.toString(), expected);
+
+ writer = new IndentingWriter();
+ mg.generateEnums(writer);
+ expect(writer.toString(), expectedEnums);
});
}
