From b61803fdb42754f0eddb63060d9df770ce53017f Mon Sep 17 00:00:00 2001 From: Nic Hite Date: Tue, 4 Jun 2019 14:04:42 -0700 Subject: [PATCH] Always prefix .pb.dart imports in .pbserver, .pb.json, .pbgrpc files. (#250) * Always prefix .pb.dart imports in .pbserver, .pb.json, .pbgrpc files. --- protoc_plugin/CHANGELOG.md | 4 ++++ protoc_plugin/lib/client_generator.dart | 4 ++-- protoc_plugin/lib/file_generator.dart | 6 +++++- protoc_plugin/lib/grpc_generator.dart | 2 +- protoc_plugin/lib/service_generator.dart | 11 +++++++---- protoc_plugin/pubspec.yaml | 2 +- protoc_plugin/test/goldens/grpc_service.pbgrpc | 2 +- protoc_plugin/test/goldens/imports.pb | 16 ++++++++-------- protoc_plugin/test/goldens/service.pbserver | 6 +++--- protoc_plugin/test/goldens/serviceGenerator | 8 ++++---- .../test/goldens/serviceGenerator.pb.json | 6 +++--- 11 files changed, 39 insertions(+), 28 deletions(-) diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index d434e8a448bc7..228c7bde32b54 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -1,3 +1,7 @@ +## 16.0.7 + +* Always prefix .pb.dart imports in .pbserver, .pb.json, .pbgrpc files. + ## 16.0.6 * Track the original order of proto fields and include it in metadata diff --git a/protoc_plugin/lib/client_generator.dart b/protoc_plugin/lib/client_generator.dart index d54bce965e010..e23cdd788c7d8 100644 --- a/protoc_plugin/lib/client_generator.dart +++ b/protoc_plugin/lib/client_generator.dart @@ -39,8 +39,8 @@ class ClientApiGenerator { avoidInitialUnderscore(service._methodName(m.name)), usedMethodNames, defaultSuffixes()); - var inputType = service._getDartClassName(m.inputType); - var outputType = service._getDartClassName(m.outputType); + var inputType = service._getDartClassName(m.inputType, forMainFile: true); + var outputType = service._getDartClassName(m.outputType, forMainFile: true); out.addBlock( '$_asyncImportPrefix.Future<$outputType> $methodName(' '$_protobufImportPrefix.ClientContext ctx, $inputType request) {', diff --git a/protoc_plugin/lib/file_generator.dart b/protoc_plugin/lib/file_generator.dart index 14e9d398d260b..07fce5cb46166 100644 --- a/protoc_plugin/lib/file_generator.dart +++ b/protoc_plugin/lib/file_generator.dart @@ -575,7 +575,11 @@ class FileGenerator extends ProtobufContainer { Uri resolvedImport = config.resolveImport(target.protoFileUri, protoFileUri, extension); out.print("import '$resolvedImport'"); - if (protoFileUri != target.protoFileUri) { + + // .pb.dart files should always be prefixed--the protoFileUri check + // will evaluate to true not just for the main .pb.dart file based off + // the proto file, but also for the .pbserver.dart, .pbgrpc.dart files. + if ((extension == ".pb.dart") || protoFileUri != target.protoFileUri) { out.print(' as ${target.fileImportPrefix}'); } out.println(';'); diff --git a/protoc_plugin/lib/grpc_generator.dart b/protoc_plugin/lib/grpc_generator.dart index 4313e32b5d54d..cb38b650d4401 100644 --- a/protoc_plugin/lib/grpc_generator.dart +++ b/protoc_plugin/lib/grpc_generator.dart @@ -93,7 +93,7 @@ class GrpcServiceGenerator { var mg = _deps[fqname]; if (mg == null) { var location = _undefinedDeps[fqname]; - // TODO(jakobr): Throw more actionable error. + // TODO(nichite): Throw more actionable error. throw 'FAILURE: Unknown type reference (${fqname}) for ${location}'; } if (fileGen.protoFileUri == mg.fileGen.protoFileUri) { diff --git a/protoc_plugin/lib/service_generator.dart b/protoc_plugin/lib/service_generator.dart index 95a84f61d801c..2b330536cc52d 100644 --- a/protoc_plugin/lib/service_generator.dart +++ b/protoc_plugin/lib/service_generator.dart @@ -110,16 +110,19 @@ class ServiceGenerator { } } - /// Returns the Dart class name to use for a message type. + /// Returns the Dart class name to use for a message type or throws an + /// exception if it can't be resolved. /// - /// Throws an exception if it can't be resolved. - String _getDartClassName(String fqname) { + /// When generating the main file (if [forMainFile] is true), all imports + /// should be prefixed unless the target file is the main file (the client + /// generator calls this method). Otherwise, prefix everything. + String _getDartClassName(String fqname, {forMainFile = false}) { var mg = _deps[fqname]; if (mg == null) { var location = _undefinedDeps[fqname]; throw 'FAILURE: Unknown type reference (${fqname}) for ${location}'; } - if (fileGen.protoFileUri == mg.fileGen.protoFileUri) { + if (forMainFile && fileGen.protoFileUri == mg.fileGen.protoFileUri) { // If it's the same file, we import it without using "as". return mg.classname; } diff --git a/protoc_plugin/pubspec.yaml b/protoc_plugin/pubspec.yaml index 14f1bd1210334..0622cc3e03704 100644 --- a/protoc_plugin/pubspec.yaml +++ b/protoc_plugin/pubspec.yaml @@ -1,5 +1,5 @@ name: protoc_plugin -version: 16.0.6 +version: 16.0.7 author: Dart Team description: Protoc compiler plugin to generate Dart code homepage: https://github.com/dart-lang/protobuf diff --git a/protoc_plugin/test/goldens/grpc_service.pbgrpc b/protoc_plugin/test/goldens/grpc_service.pbgrpc index 07a98150c0612..3bf477fb81978 100644 --- a/protoc_plugin/test/goldens/grpc_service.pbgrpc +++ b/protoc_plugin/test/goldens/grpc_service.pbgrpc @@ -10,7 +10,7 @@ import 'package:grpc/service_api.dart' as $grpc; import 'dart:core' as $core show int, String, List; -import 'test.pb.dart'; +import 'test.pb.dart' as $0; export 'test.pb.dart'; class TestClient extends $grpc.Client { diff --git a/protoc_plugin/test/goldens/imports.pb b/protoc_plugin/test/goldens/imports.pb index 63d01eae7ca6b..e25694a7d09ee 100644 --- a/protoc_plugin/test/goldens/imports.pb +++ b/protoc_plugin/test/goldens/imports.pb @@ -8,14 +8,14 @@ import 'dart:core' as $core show bool, Deprecated, double, int, List, Map, overr import 'package:protobuf/protobuf.dart' as $pb; -import 'package1.pb.dart' as $0; -import 'package2.pb.dart' as $1; +import 'package1.pb.dart' as $1; +import 'package2.pb.dart' as $2; class M extends $pb.GeneratedMessage { static final $pb.BuilderInfo _i = $pb.BuilderInfo('M') ..a(1, 'm', $pb.PbFieldType.OM, M.getDefault, M.create) - ..a<$0.M>(2, 'm1', $pb.PbFieldType.OM, $0.M.getDefault, $0.M.create) - ..a<$1.M>(3, 'm2', $pb.PbFieldType.OM, $1.M.getDefault, $1.M.create) + ..a<$1.M>(2, 'm1', $pb.PbFieldType.OM, $1.M.getDefault, $1.M.create) + ..a<$2.M>(3, 'm2', $pb.PbFieldType.OM, $2.M.getDefault, $2.M.create) ..hasRequiredFields = false ; @@ -36,13 +36,13 @@ class M extends $pb.GeneratedMessage { $core.bool hasM() => $_has(0); void clearM() => clearField(1); - $0.M get m1 => $_getN(1); - set m1($0.M v) { setField(2, v); } + $1.M get m1 => $_getN(1); + set m1($1.M v) { setField(2, v); } $core.bool hasM1() => $_has(1); void clearM1() => clearField(2); - $1.M get m2 => $_getN(2); - set m2($1.M v) { setField(3, v); } + $2.M get m2 => $_getN(2); + set m2($2.M v) { setField(3, v); } $core.bool hasM2() => $_has(2); void clearM2() => clearField(3); } diff --git a/protoc_plugin/test/goldens/service.pbserver b/protoc_plugin/test/goldens/service.pbserver index 3e53cdde489ae..53aa91a496289 100644 --- a/protoc_plugin/test/goldens/service.pbserver +++ b/protoc_plugin/test/goldens/service.pbserver @@ -9,17 +9,17 @@ import 'dart:async' as $async; import 'package:protobuf/protobuf.dart' as $pb; import 'dart:core' as $core show String, Map, ArgumentError, dynamic; -import 'test.pb.dart'; +import 'test.pb.dart' as $0; import 'test.pbjson.dart'; export 'test.pb.dart'; abstract class TestServiceBase extends $pb.GeneratedService { - $async.Future ping($pb.ServerContext ctx, Empty request); + $async.Future<$0.Empty> ping($pb.ServerContext ctx, $0.Empty request); $pb.GeneratedMessage createRequest($core.String method) { switch (method) { - case 'Ping': return Empty(); + case 'Ping': return $0.Empty(); default: throw $core.ArgumentError('Unknown method: $method'); } } diff --git a/protoc_plugin/test/goldens/serviceGenerator b/protoc_plugin/test/goldens/serviceGenerator index ca0c3fe328d7e..77d87287a4609 100644 --- a/protoc_plugin/test/goldens/serviceGenerator +++ b/protoc_plugin/test/goldens/serviceGenerator @@ -1,11 +1,11 @@ abstract class TestServiceBase extends $pb.GeneratedService { - $async.Future aMethod($pb.ServerContext ctx, SomeRequest request); - $async.Future<$0.AnotherReply> anotherMethod($pb.ServerContext ctx, $0.EmptyMessage request); + $async.Future<$0.SomeReply> aMethod($pb.ServerContext ctx, $0.SomeRequest request); + $async.Future<$1.AnotherReply> anotherMethod($pb.ServerContext ctx, $1.EmptyMessage request); $pb.GeneratedMessage createRequest($core.String method) { switch (method) { - case 'AMethod': return SomeRequest(); - case 'AnotherMethod': return $0.EmptyMessage(); + case 'AMethod': return $0.SomeRequest(); + case 'AnotherMethod': return $1.EmptyMessage(); default: throw $core.ArgumentError('Unknown method: $method'); } } diff --git a/protoc_plugin/test/goldens/serviceGenerator.pb.json b/protoc_plugin/test/goldens/serviceGenerator.pb.json index 5ad78ef828be2..2f38a4dd94473 100644 --- a/protoc_plugin/test/goldens/serviceGenerator.pb.json +++ b/protoc_plugin/test/goldens/serviceGenerator.pb.json @@ -4,7 +4,7 @@ /// // ignore_for_file: camel_case_types,non_constant_identifier_names,library_prefixes,unused_import,unused_shown_name -import 'foobar.pbjson.dart' as $0; +import 'foobar.pbjson.dart' as $1; const SomeRequest$json = const { '1': 'SomeRequest', @@ -25,7 +25,7 @@ const TestServiceBase$json = const { const TestServiceBase$messageJson = const { '.testpkg.SomeRequest': SomeRequest$json, '.testpkg.SomeReply': SomeReply$json, - '.foo.bar.EmptyMessage': $0.EmptyMessage$json, - '.foo.bar.AnotherReply': $0.AnotherReply$json, + '.foo.bar.EmptyMessage': $1.EmptyMessage$json, + '.foo.bar.AnotherReply': $1.AnotherReply$json, };