diff --git a/Makefile b/Makefile index 0b3b672d1..03df90790 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,7 @@ TEST_PROTO_LIST = \ toplevel_import \ toplevel TEST_PROTO_DIR=$(OUTPUT_DIR)/protos -TEST_PROTO_LIBS=$(foreach proto, $(TEST_PROTO_LIST), $(TEST_PROTO_DIR)/$(proto).pb.dart) +TEST_PROTO_LIBS=$(foreach proto, $(TEST_PROTO_LIST), $(TEST_PROTO_DIR)/$(proto).pb.dart $(TEST_PROTO_DIR)/$(proto).pbjson.dart) TEST_PROTO_SRC_DIR=test/protos TEST_PROTO_SRCS=$(foreach proto, $(TEST_PROTO_LIST), $(TEST_PROTO_SRC_DIR)/$(proto).proto) diff --git a/lib/code_generator.dart b/lib/code_generator.dart index 9e9e3af53..2072de2eb 100644 --- a/lib/code_generator.dart +++ b/lib/code_generator.dart @@ -27,10 +27,9 @@ class CodeGenerator extends ProtobufContainer { /// for details), and [outputConfiguration] can be used to override where /// generated files are created and how imports between generated files are /// constructed (see [OutputConfiguration] for details). - void generate({ - Map optionParsers, + void generate( + {Map optionParsers, OutputConfiguration config}) { - if (config == null) { config = new DefaultOutputConfiguration(); } @@ -42,37 +41,36 @@ class CodeGenerator extends ProtobufContainer { .fold(new BytesBuilder(), (builder, data) => builder..add(data)) .then((builder) => builder.takeBytes()) .then((List bytes) { - var request = - new CodeGeneratorRequest.fromBuffer(bytes, extensions); - var response = new CodeGeneratorResponse(); + var request = new CodeGeneratorRequest.fromBuffer(bytes, extensions); + var response = new CodeGeneratorResponse(); - // Parse the options in the request. Return the errors is any. - var options = parseGenerationOptions( - request, response, optionParsers); - if (options == null) { - _streamOut.add(response.writeToBuffer()); - return; - } + // Parse the options in the request. Return the errors is any. + var options = parseGenerationOptions(request, response, optionParsers); + if (options == null) { + _streamOut.add(response.writeToBuffer()); + return; + } - // Create a syntax tree for each .proto file given to us. - // (We may import it even if we don't generate the .pb.dart file.) - List generators = []; - for (FileDescriptorProto file in request.protoFile) { - generators.add(new FileGenerator(file)); - } + // Create a syntax tree for each .proto file given to us. + // (We may import it even if we don't generate the .pb.dart file.) + List generators = []; + for (FileDescriptorProto file in request.protoFile) { + generators.add(new FileGenerator(file)); + } - // Collect field types and importable files. - link(options, generators); + // Collect field types and importable files. + link(options, generators); - // Generate the .pb.dart file if requested. - for (var gen in generators) { - var name = gen._fileDescriptor.name; - if (request.fileToGenerate.contains(name)) { - response.file.add(gen.generateResponse(config)); - } - } - _streamOut.add(response.writeToBuffer()); - }); + // Generate the .pb.dart file if requested. + for (var gen in generators) { + var name = gen._fileDescriptor.name; + if (request.fileToGenerate.contains(name)) { + response.file.add(gen.generateResponse(config)); + response.file.add(gen.generateJsonDartResponse(config)); + } + } + _streamOut.add(response.writeToBuffer()); + }); } String get package => ''; diff --git a/lib/extension_generator.dart b/lib/extension_generator.dart index 441d38b41..91260aa32 100644 --- a/lib/extension_generator.dart +++ b/lib/extension_generator.dart @@ -40,7 +40,10 @@ class ExtensionGenerator { return _field.needsFixnumImport; } - /// Adds any imports needed by the Dart code defining this extension. + /// Adds dependencies of [generate] to [imports]. + /// + /// For each .pb.dart file that the generated code needs to import, + /// add its generator. void addImportsTo(Set imports) { if (_field == null) throw new StateError("resolve not called"); var typeGen = _field.baseType.generator; @@ -51,6 +54,15 @@ class ExtensionGenerator { } } + /// Adds dependencies of [generateConstants] to [imports]. + /// + /// For each .pb.dart file that the generated code needs to import, + /// add its generator. + void addConstantImportsTo(Set imports) { + if (_field == null) throw new StateError("resolve not called"); + // No dependencies - nothing to do. + } + void generate(IndentingWriter out) { if (_field == null) throw new StateError("resolve not called"); diff --git a/lib/file_generator.dart b/lib/file_generator.dart index 3ea66a47e..76c5a7ff2 100644 --- a/lib/file_generator.dart +++ b/lib/file_generator.dart @@ -22,6 +22,9 @@ class FileGenerator extends ProtobufContainer { final FileDescriptorProto _fileDescriptor; + // The relative path used to import the .proto file, as a URI. + final Uri protoFileUri; + final List enumGenerators = []; final List messageGenerators = []; final List extensionGenerators = []; @@ -31,7 +34,14 @@ class FileGenerator extends ProtobufContainer { /// True if cross-references have been resolved. bool _linked = false; - FileGenerator(this._fileDescriptor) { + FileGenerator(FileDescriptorProto descriptor) + : _fileDescriptor = descriptor, + protoFileUri = new Uri.file(descriptor.name) { + if (protoFileUri.isAbsolute) { + // protoc should never generate an import with an absolute path. + throw "FAILURE: Import with absolute path is not supported"; + } + var defaultMixin = _getDefaultMixin(_fileDescriptor); // Load and register all enum and message types. @@ -117,13 +127,7 @@ class FileGenerator extends ProtobufContainer { [OutputConfiguration config = const DefaultOutputConfiguration()]) { if (!_linked) throw new StateError("not linked"); - 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"; - } - - generateHeader(out, filePath, config); + generateHeader(out, config); // Generate code. for (EnumGenerator e in enumGenerators) { @@ -137,7 +141,7 @@ class FileGenerator extends ProtobufContainer { // name derived from the file name. if (!extensionGenerators.isEmpty) { // TODO(antonm): do not generate a class. - String className = _generateClassName(filePath); + String className = _generateClassName(protoFileUri); out.addBlock('class $className {', '}\n', () { for (ExtensionGenerator x in extensionGenerators) { x.generate(out); @@ -157,24 +161,12 @@ class FileGenerator extends ProtobufContainer { for (ServiceGenerator s in serviceGenerators) { s.generate(out); } - - // Put JSON constants at the end of the file. - - for (var e in enumGenerators) { - e.generateConstants(out); - } - for (MessageGenerator m in messageGenerators) { - m.generateConstants(out); - } - for (ServiceGenerator s in serviceGenerators) { - s.generateConstants(out); - } } /// Prints header and imports. - void generateHeader(IndentingWriter out, Uri filePath, + void generateHeader(IndentingWriter out, [OutputConfiguration config = const DefaultOutputConfiguration()]) { - String libraryName = _generateLibraryName(filePath); + String libraryName = _generateLibraryName(protoFileUri); out.println('///\n' '// Generated code. Do not modify.\n' '///\n' @@ -199,15 +191,10 @@ class FileGenerator extends ProtobufContainer { out.println("import '${imp}' show ${symbols.join(', ')};"); } + // Import the .pb.dart files we depend on. for (var imported in _findProtosToImport()) { - String filename = imported._fileDescriptor.name; - Uri importPath = new Uri.file(filename); - if (importPath.isAbsolute) { - // protoc should never generate an import with an absolute path. - throw "FAILURE: Import with absolute path is not supported"; - } - // Create a path from the current file to the imported proto. - Uri resolvedImport = config.resolveImport(importPath, filePath); + Uri resolvedImport = + config.resolveImport(imported.protoFileUri, protoFileUri); out.print("import '$resolvedImport'"); if (package != imported.package && imported.package.isNotEmpty) { out.print(' as ${imported.packageImportPrefix}'); @@ -215,6 +202,13 @@ class FileGenerator extends ProtobufContainer { out.println(';'); } out.println(); + + // Services also depend on the json imports. + if (serviceGenerators.isNotEmpty) { + Uri resolvedImport = config.resolveJsonImport(protoFileUri, protoFileUri); + out.print("import '$resolvedImport';"); + out.println(); + } } bool get _needsFixnumImport { @@ -267,4 +261,77 @@ class FileGenerator extends ProtobufContainer { return imports; } + + CodeGeneratorResponse_File generateJsonDartResponse( + OutputConfiguration config) { + if (!_linked) throw new StateError("not linked"); + + IndentingWriter out = new IndentingWriter(); + + generateJsonDart(out, config); + + Uri filePath = new Uri.file(_fileDescriptor.name); + return new CodeGeneratorResponse_File() + ..name = config.jsonDartOutputPathFor(filePath).path + ..content = out.toString(); + } + + void generateJsonDart(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 + "_pbjson"; + out.print(''' +/// +// Generated code. Do not modify. +/// +library $libraryName; + +'''); + + // Import the .pbjson.dart files we depend on. + var importList = _findJsonProtosToImport(); + for (var imported in importList) { + Uri resolvedImport = + config.resolveJsonImport(imported.protoFileUri, protoFileUri); + out.print("import '$resolvedImport'"); + if (package != imported.package && imported.package.isNotEmpty) { + out.print(' as ${imported.packageImportPrefix}'); + } + out.println(';'); + } + if (importList.isNotEmpty) out.println(); + + for (var e in enumGenerators) { + e.generateConstants(out); + } + for (MessageGenerator m in messageGenerators) { + m.generateConstants(out); + } + for (ServiceGenerator s in serviceGenerators) { + s.generateConstants(out); + } + } + + /// Returns the generator for each .pbjson.dart file the generated + /// .pbjson.dart needs to import. + Set _findJsonProtosToImport() { + var imports = new Set.identity(); + for (var m in messageGenerators) { + m.addConstantImportsTo(imports); + } + for (var x in extensionGenerators) { + x.addConstantImportsTo(imports); + } + for (var x in serviceGenerators) { + x.addConstantImportsTo(imports); + } + imports.remove(this); // Don't need to import self. + return imports; + } } diff --git a/lib/message_generator.dart b/lib/message_generator.dart index 49ceb158b..4384d8871 100644 --- a/lib/message_generator.dart +++ b/lib/message_generator.dart @@ -193,7 +193,10 @@ class MessageGenerator extends ProtobufContainer { return false; } - /// Adds generators of the .pb.dart files that this type needs to import. + /// Adds dependencies of [generate] to [imports]. + /// + /// For each .pb.dart file that the generated code needs to import, + /// add its generator. void addImportsTo(Set imports) { if (_fieldList == null) throw new StateError("message not resolved"); for (var field in _fieldList) { @@ -212,6 +215,20 @@ class MessageGenerator extends ProtobufContainer { } } + /// Adds dependencies of [generateConstants] to [imports]. + /// + /// For each .pbjson.dart file that the generated code needs to import, + /// add its generator. + void addConstantImportsTo(Set imports) { + if (_fieldList == null) throw new StateError("message not resolved"); + for (var m in _messageGenerators) { + m.addConstantImportsTo(imports); + } + for (var x in _extensionGenerators) { + x.addConstantImportsTo(imports); + } + } + void generate(IndentingWriter out) { checkResolved(); diff --git a/lib/output_config.dart b/lib/output_config.dart index 63d8525a4..481d03d5b 100644 --- a/lib/output_config.dart +++ b/lib/output_config.dart @@ -7,7 +7,6 @@ part of protoc; /// Configures where output of the protoc compiler should be placed and how to /// import one generated file from another. abstract class OutputConfiguration { - const OutputConfiguration(); /// Returns [filePath] with it's extension replaced with '.pb.dart'. @@ -18,31 +17,69 @@ abstract class OutputConfiguration { Uri replaceUriExtension(Uri file) => path.url.toUri(replacePathExtension(path.url.fromUri(file))); - /// Resolves an import URI. Both [source] and [target] are .proto files, - /// where [target] is imported from [source]. The result URI can be used to - /// import [target]'s .pb.dart output from [source]'s .pb.dart output. + /// Resolves an import of a .pb.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); - /// Returns the path, under the output folder, where the code will be - /// generated for [inputPath]. The input is expected to be a .proto file, - /// while the output is expected to be a .pb.dart file. + /// 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); + + /// 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); } /// Default [OutputConfiguration] that uses the same path as the input /// file for the output file (just replaces the extension), and that uses /// relative paths to resolve imports. class DefaultOutputConfiguration extends OutputConfiguration { - const DefaultOutputConfiguration(); + @override Uri outputPathFor(Uri input) => replaceUriExtension(input); + @override + Uri jsonDartOutputPathFor(Uri input) { + var base = path.withoutExtension(path.url.fromUri(input)); + return path.url.toUri('$base.pbjson.dart'); + } + + @override Uri resolveImport(Uri target, Uri source) { - var builder = path.url; - var targetPath = builder.fromUri(target); - var sourceDir = builder.dirname(builder.fromUri(source)); - return builder.toUri(replacePathExtension( - builder.relative(targetPath, from: sourceDir))); + 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) { + 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'); } } diff --git a/lib/service_generator.dart b/lib/service_generator.dart index 067331255..811def232 100644 --- a/lib/service_generator.dart +++ b/lib/service_generator.dart @@ -10,12 +10,18 @@ class ServiceGenerator { /// The generator of the .pb.dart file that will contain this service. final FileGenerator fileGen; - /// The message types needed by this service. + /// The message types needed directly by this service. /// /// The key is the fully qualified name. /// Populated by [resolve]. final _deps = {}; + /// The message types needed transitively by this service. + /// + /// The key is the fully qualified name. + /// Populated by [resolve]. + final _transitiveDeps = {}; + /// Maps each undefined type to a string describing its location. /// /// Populated by [resolve]. @@ -33,7 +39,7 @@ class ServiceGenerator { /// Finds all message types used by this service. /// - /// Puts the types found in [_deps]. + /// Puts the types found in [_deps] and [_transitiveDeps]. /// If a type name can't be resolved, puts it in [_undefinedDeps]. /// Precondition: messages have been registered and resolved. void resolve(GenerationContext ctx) { @@ -59,29 +65,45 @@ class ServiceGenerator { _undefinedDeps[fqname] = location; return; } - _addDepsRecursively(mg); + _addDepsRecursively(mg, 0); } - void _addDepsRecursively(MessageGenerator mg) { - if (_deps.containsKey(mg.fqname)) return; // Already added. + void _addDepsRecursively(MessageGenerator mg, int depth) { + if (_transitiveDeps.containsKey(mg.fqname)) { + // Already added, but perhaps at a different depth. + if (depth == 0) _deps[mg.fqname] = mg; + return; + } mg.checkResolved(); - _deps[mg.fqname] = mg; + if (depth == 0) _deps[mg.fqname] = mg; + _transitiveDeps[mg.fqname] = mg; for (var field in mg._fieldList) { if (field.baseType.isGroup || field.baseType.isMessage) { - _addDepsRecursively(field.baseType.generator); + _addDepsRecursively(field.baseType.generator, depth + 1); } } } - /// Adds generators of the .pb.dart files that this service needs to import. + /// Adds dependencies of [generate] to [imports]. + /// + /// For each .pb.dart file that the generated code needs to import, + /// add its generator. void addImportsTo(Set imports) { - // Only the top-level imports are actually used so far. - // (They will be added in the next CL.) for (var mg in _deps.values) { imports.add(mg.fileGen); } } + /// Adds dependencies of [generateConstants] to [imports]. + /// + /// For each .pbjson.dart file that the generated code needs to import, + /// add its generator. + void addConstantImportsTo(Set imports) { + for (var mg in _transitiveDeps.values) { + imports.add(mg.fileGen); + } + } + /// Returns the Dart class name to use for a message type. /// /// Throws an exception if it can't be resolved. @@ -186,8 +208,8 @@ class ServiceGenerator { out.println(); var typeConstants = {}; - for (var key in _deps.keys) { - typeConstants[key] = _deps[key].getJsonConstant(fileGen); + for (var key in _transitiveDeps.keys) { + typeConstants[key] = _transitiveDeps[key].getJsonConstant(fileGen); } out.addBlock("const $messageJsonConstant = const {", "};", () { for (var key in typeConstants.keys) { diff --git a/pubspec.yaml b/pubspec.yaml index 5fac574b4..f913957e8 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: protoc_plugin -version: 0.5.1 +version: 0.5.2 author: Dart Team description: Protoc compiler plugin to generate Dart code homepage: https://github.com/dart-lang/dart-protoc-plugin diff --git a/test/file_generator_test.dart b/test/file_generator_test.dart index 0ec58963e..a8c98e417 100644 --- a/test/file_generator_test.dart +++ b/test/file_generator_test.dart @@ -69,7 +69,8 @@ FileDescriptorProto buildFileDescriptor( } void main() { - test('FileGenerator output for a proto with one message', () { + test('FileGenerator outputs a .pb.dart file for a proto with one message', + () { // NOTE: Below > 80 cols because it is matching generated code > 80 cols. String expected = r''' /// @@ -120,6 +121,26 @@ class PhoneNumber extends GeneratedMessage { class _ReadonlyPhoneNumber extends PhoneNumber with ReadonlyMessageMixin {} +'''; + FileDescriptorProto fd = buildFileDescriptor(); + var options = parseGenerationOptions( + new CodeGeneratorRequest(), new CodeGeneratorResponse()); + FileGenerator fg = new FileGenerator(fd); + link(options, [fg]); + + IndentingWriter writer = new IndentingWriter(); + fg.generate(writer); + expect(writer.toString(), expected); + }); + + test('FileGenerator outputs a pbjson.dart file for a proto with one message', + () { + var expected = r''' +/// +// Generated code. Do not modify. +/// +library test_pbjson; + const PhoneNumber$json = const { '1': 'PhoneNumber', '2': const [ @@ -137,11 +158,11 @@ const PhoneNumber$json = const { link(options, [fg]); IndentingWriter writer = new IndentingWriter(); - fg.generate(writer); + fg.generateJsonDart(writer); expect(writer.toString(), expected); }); - test('FileGenerator output for a top-level enum', () { + test('FileGenerator generates a .pb.dart file for a top-level enum', () { // NOTE: Below > 80 cols because it is matching generated code > 80 cols. String expected = r''' /// @@ -173,6 +194,27 @@ class PhoneType extends ProtobufEnum { const PhoneType._(int v, String n) : super(v, n); } +'''; + FileDescriptorProto fd = + buildFileDescriptor(phoneNumber: false, topLevelEnum: true); + var options = parseGenerationOptions( + new CodeGeneratorRequest(), new CodeGeneratorResponse()); + + FileGenerator fg = new FileGenerator(fd); + link(options, [fg]); + + var writer = new IndentingWriter(); + fg.generate(writer); + expect(writer.toString(), expected); + }); + + test('FileGenerator generates a .pbjson.dart file for a top-level enum', () { + String expected = r''' +/// +// Generated code. Do not modify. +/// +library test_pbjson; + const PhoneType$json = const { '1': 'PhoneType', '2': const [ @@ -184,6 +226,7 @@ const PhoneType$json = const { }; '''; + FileDescriptorProto fd = buildFileDescriptor(phoneNumber: false, topLevelEnum: true); var options = parseGenerationOptions( @@ -193,7 +236,7 @@ const PhoneType$json = const { link(options, [fg]); var writer = new IndentingWriter(); - fg.generate(writer); + fg.generateJsonDart(writer); expect(writer.toString(), expected); }); @@ -216,7 +259,7 @@ import 'package:protobuf/protobuf.dart'; link(options, [fg]); var writer = new IndentingWriter(); - fg.generateHeader(writer, Uri.parse("test.proto")); + fg.generateHeader(writer); expect(writer.toString(), expected); }); @@ -249,7 +292,35 @@ import 'package:protobuf/protobuf.dart'; link(options, [fg]); var writer = new IndentingWriter(); - fg.generateHeader(writer, Uri.parse("test.proto")); + fg.generateHeader(writer); + expect(writer.toString(), expected); + }); + + test('FileGenerator outputs extra imports for a service', () { + String expected = r''' +/// +// Generated code. Do not modify. +/// +library test; + +import 'dart:async'; + +import 'package:protobuf/protobuf.dart'; + +import 'test.pbjson.dart'; +'''; + FileDescriptorProto fd = new FileDescriptorProto() + ..name = 'test' + ..service.add(new ServiceDescriptorProto()); + + var options = parseGenerationOptions( + new CodeGeneratorRequest(), new CodeGeneratorResponse()); + + FileGenerator fg = new FileGenerator(fd); + link(options, [fg]); + + var writer = new IndentingWriter(); + fg.generateHeader(writer); expect(writer.toString(), expected); }); @@ -304,15 +375,6 @@ class PhoneNumber extends GeneratedMessage { class _ReadonlyPhoneNumber extends PhoneNumber with ReadonlyMessageMixin {} -const PhoneNumber$json = const { - '1': 'PhoneNumber', - '2': const [ - const {'1': 'number', '3': 1, '4': 2, '5': 9}, - const {'1': 'type', '3': 2, '4': 1, '5': 5, '6': ''}, - const {'1': 'name', '3': 3, '4': 1, '5': 9, '7': r'$'}, - ], -}; - '''; FileDescriptorProto fd = buildFileDescriptor(); var request = new CodeGeneratorRequest(); @@ -383,6 +445,14 @@ class M extends GeneratedMessage { class _ReadonlyM extends M with ReadonlyMessageMixin {} +'''; + + var expectedJson = r''' +/// +// Generated code. Do not modify. +/// +library test_pbjson; + const M$json = const { '1': 'M', '2': const [ @@ -496,5 +566,9 @@ const M$json = const { var writer = new IndentingWriter(); fg.generate(writer); expect(writer.toString(), expected); + + writer = new IndentingWriter(); + fg.generateJsonDart(writer); + expect(writer.toString(), expectedJson); }); } diff --git a/test/message_test.dart b/test/message_test.dart index 949043b3f..c7e180828 100755 --- a/test/message_test.dart +++ b/test/message_test.dart @@ -11,6 +11,7 @@ import 'test_util.dart'; import '../out/protos/descriptor_2_5_opensource.pb.dart' show DescriptorProto; import '../out/protos/google/protobuf/unittest.pb.dart'; +import '../out/protos/google/protobuf/unittest.pbjson.dart'; void main() { TestRequired TEST_REQUIRED_UNINITIALIZED = new TestRequired();