diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index 2dcbcf413..2adc01eb2 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -1,3 +1,10 @@ +## 20.0.1 + +* Handle name disambiguation for service methods to avoid clashes with + existing names. + + Now you can have a method called `New`. In dart the method name will be `new_`. + ## 20.0.0 * Stable release generating null-safe code. diff --git a/protoc_plugin/lib/names.dart b/protoc_plugin/lib/names.dart index a2094437c..e5e33fea5 100644 --- a/protoc_plugin/lib/names.dart +++ b/protoc_plugin/lib/names.dart @@ -491,28 +491,39 @@ bool _isDartFieldName(String name) => name.startsWith(_dartFieldNameExpr); final _dartFieldNameExpr = RegExp(r'^[a-z]\w+$'); /// Names that would collide as top-level identifiers. -final List forbiddenTopLevelNames = [ +const forbiddenTopLevelNames = [ 'List', 'Function', 'Map', ..._dartReservedWords, ]; -final List reservedMemberNames = [ +const reservedMemberNames = [ ..._dartReservedWords, ...GeneratedMessage_reservedNames, ..._generatedMessageNames ]; -final List forbiddenExtensionNames = [ +const forbiddenExtensionNames = [ ..._dartReservedWords, ...GeneratedMessage_reservedNames, ..._generatedMessageNames ]; +const serviceReservedMemberNames = [ + ..._dartReservedWords, + ..._serviceNames, +]; + +const _serviceNames = [ + // From GeneratedService + r'createRequest', + r'handleCall', +]; + // List of Dart language reserved words in names which cannot be used in a // subclass of GeneratedMessage. -const List _dartReservedWords = [ +const _dartReservedWords = [ 'assert', 'bool', 'break', diff --git a/protoc_plugin/lib/src/client_generator.dart b/protoc_plugin/lib/src/client_generator.dart index 500c0c944..7a058bcc4 100644 --- a/protoc_plugin/lib/src/client_generator.dart +++ b/protoc_plugin/lib/src/client_generator.dart @@ -27,7 +27,7 @@ class ClientApiGenerator { out.println('${className}Api(this._client);'); out.println(); - for (var m in service._descriptor.method) { + for (var m in service.methods) { generateMethod(out, m); } }); @@ -35,20 +35,18 @@ class ClientApiGenerator { } // Subclasses can override this. - void generateMethod(IndentingWriter out, MethodDescriptorProto m) { - var methodName = disambiguateName( - avoidInitialUnderscore(service._methodName(m.name)), - usedMethodNames, - defaultSuffixes()); - var inputType = service._getDartClassName(m.inputType, forMainFile: true); - var outputType = service._getDartClassName(m.outputType, forMainFile: true); + void generateMethod(IndentingWriter out, _ServiceMethod m) { + var inputType = + service._getDartClassName(m.descriptor.inputType, forMainFile: true); + var outputType = + service._getDartClassName(m.descriptor.outputType, forMainFile: true); out.addBlock( - '$asyncImportPrefix.Future<$outputType> $methodName(' + '$asyncImportPrefix.Future<$outputType> ${m.dartName}(' '$protobufImportPrefix.ClientContext? ctx, $inputType request) {', '}', () { out.println('var emptyResponse = $outputType();'); out.println('return _client.invoke<$outputType>(ctx, \'$className\', ' - '\'${m.name}\', request, emptyResponse);'); + '\'${m.descriptor.name}\', request, emptyResponse);'); }); } } diff --git a/protoc_plugin/lib/src/grpc_generator.dart b/protoc_plugin/lib/src/grpc_generator.dart index 73fe67eb2..7a3591245 100644 --- a/protoc_plugin/lib/src/grpc_generator.dart +++ b/protoc_plugin/lib/src/grpc_generator.dart @@ -58,8 +58,9 @@ class GrpcServiceGenerator { /// in [_undefinedDeps]. /// Precondition: messages have been registered and resolved. void resolve(GenerationContext ctx) { + final usedNames = {...serviceReservedMemberNames}; for (var method in _descriptor.method) { - _methods.add(_GrpcMethod(this, ctx, method)); + _methods.add(_GrpcMethod(this, ctx, method, usedNames)); } } @@ -181,9 +182,13 @@ class _GrpcMethod { this._serverReturnType); factory _GrpcMethod(GrpcServiceGenerator service, GenerationContext ctx, - MethodDescriptorProto method) { + MethodDescriptorProto method, Set usedNames) { final grpcName = method.name; - final dartName = lowerCaseFirstLetter(grpcName); + final dartName = disambiguateName( + avoidInitialUnderscore(lowerCaseFirstLetter(grpcName)), + usedNames, + defaultSuffixes(), + generateVariants: (name) => [name, '${name}_Pre']); final clientStreaming = method.clientStreaming; final serverStreaming = method.serverStreaming; diff --git a/protoc_plugin/lib/src/service_generator.dart b/protoc_plugin/lib/src/service_generator.dart index 3fcd1a943..d73c62fc0 100644 --- a/protoc_plugin/lib/src/service_generator.dart +++ b/protoc_plugin/lib/src/service_generator.dart @@ -51,7 +51,10 @@ class ServiceGenerator { /// If a type name can't be resolved, puts it in [_undefinedDeps]. /// Precondition: messages have been registered and resolved. void resolve(GenerationContext ctx) { - for (var m in _methodDescriptors) { + final usedNames = {...serviceReservedMemberNames}; + + for (var m in _descriptor.method) { + methods.add(_ServiceMethod(_methodName(m.name, usedNames), m)); _addDependency(ctx, m.inputType, 'input type of ${m.name}'); _addDependency(ctx, m.outputType, 'output type of ${m.name}'); } @@ -132,23 +135,26 @@ class ServiceGenerator { return mg.fileImportPrefix + '.' + mg.classname; } - List get _methodDescriptors => _descriptor.method; + List<_ServiceMethod> methods = <_ServiceMethod>[]; - String _methodName(String name) => lowerCaseFirstLetter(name); + String _methodName(String name, Set usedNames) { + return disambiguateName(avoidInitialUnderscore(lowerCaseFirstLetter(name)), + usedNames, defaultSuffixes(), + generateVariants: (name) => [name, '${name}_Pre']); + } String get _parentClass => _generatedService; - void _generateStub(IndentingWriter out, MethodDescriptorProto m) { - var methodName = _methodName(m.name); - var inputClass = _getDartClassName(m.inputType); - var outputClass = _getDartClassName(m.outputType); + void _generateStub(IndentingWriter out, _ServiceMethod m) { + var inputClass = _getDartClassName(m.descriptor.inputType); + var outputClass = _getDartClassName(m.descriptor.outputType); - out.println('$_future<$outputClass> $methodName(' + out.println('$_future<$outputClass> ${m.dartName}(' '$_serverContext ctx, $inputClass request);'); } void _generateStubs(IndentingWriter out) { - for (var m in _methodDescriptors) { + for (var m in methods) { _generateStub(out, m); } out.println(); @@ -159,9 +165,9 @@ class ServiceGenerator { '$_generatedMessage createRequest($coreImportPrefix.String method) {', '}', () { out.addBlock('switch (method) {', '}', () { - for (var m in _methodDescriptors) { - var inputClass = _getDartClassName(m.inputType); - out.println("case '${m.name}': return $inputClass();"); + for (var m in methods) { + var inputClass = _getDartClassName(m.descriptor.inputType); + out.println("case '${m.descriptor.name}': return $inputClass();"); } out.println('default: ' "throw $coreImportPrefix.ArgumentError('Unknown method: \$method');"); @@ -176,10 +182,9 @@ class ServiceGenerator { '$coreImportPrefix.String method, $_generatedMessage request) {', '}', () { out.addBlock('switch (method) {', '}', () { - for (var m in _methodDescriptors) { - var methodName = _methodName(m.name); - var inputClass = _getDartClassName(m.inputType); - out.println("case '${m.name}': return this.$methodName" + for (var m in methods) { + var inputClass = _getDartClassName(m.descriptor.inputType); + out.println("case '${m.descriptor.name}': return this.${m.dartName}" '(ctx, request as $inputClass);'); } out.println('default: ' @@ -267,3 +272,13 @@ class ServiceGenerator { static final String _generatedService = '$protobufImportPrefix.GeneratedService'; } + +class _ServiceMethod { + final String dartName; + final MethodDescriptorProto descriptor; + + _ServiceMethod( + this.dartName, + this.descriptor, + ); +} diff --git a/protoc_plugin/pubspec.yaml b/protoc_plugin/pubspec.yaml index 2febd4286..fafd93070 100644 --- a/protoc_plugin/pubspec.yaml +++ b/protoc_plugin/pubspec.yaml @@ -17,6 +17,7 @@ dev_dependencies: lints: ^1.0.0 matcher: ^0.12.10 collection: ^1.15.0 + grpc: ^3.0.0 executables: protoc-gen-dart: protoc_plugin diff --git a/protoc_plugin/test/file_generator_test.dart b/protoc_plugin/test/file_generator_test.dart index 84c733a26..f5ad5aadb 100644 --- a/protoc_plugin/test/file_generator_test.dart +++ b/protoc_plugin/test/file_generator_test.dart @@ -300,9 +300,23 @@ void main() { ..clientStreaming = true ..serverStreaming = true; + // Some methods with names that overlap reserved words or existing methods + // in the base classes Client/Service of package:grpc. + final names = [ + MethodDescriptorProto() + ..name = 'New' + ..inputType = '.Input' + ..outputType = '.Output', + MethodDescriptorProto() + ..name = r'_32SillyName' + ..inputType = '.Input' + ..outputType = '.Output', + ]; + var sd = ServiceDescriptorProto() ..name = 'Test' - ..method.addAll([unary, clientStreaming, serverStreaming, bidirectional]); + ..method.addAll( + [unary, clientStreaming, serverStreaming, bidirectional, ...names]); var fd = FileDescriptorProto() ..name = 'test' diff --git a/protoc_plugin/test/generated_message_test.dart b/protoc_plugin/test/generated_message_test.dart index ce7f0ec71..ad83bfead 100755 --- a/protoc_plugin/test/generated_message_test.dart +++ b/protoc_plugin/test/generated_message_test.dart @@ -17,12 +17,41 @@ import '../out/protos/package1.pb.dart' as p1; import '../out/protos/package2.pb.dart' as p2; import '../out/protos/package3.pb.dart' as p3; import '../out/protos/reserved_names.pb.dart'; +import '../out/protos/reserved_names.pbserver.dart'; import '../out/protos/reserved_names_extension.pb.dart'; import '../out/protos/reserved_names_message.pb.dart'; import '../out/protos/toplevel.pb.dart'; import '../out/protos/toplevel_import.pb.dart' as t; import 'test_util.dart'; +class ReservedNamesService extends ReservedNamesServiceBase { + @override + Future abc(ServerContext ctx, SimpleReq request) async => + SimpleResponse(test: 'Abc called'); + + @override + Future abc_Pre_(ServerContext ctx, SimpleReq request) async => + SimpleResponse(test: 'abc_Pre called'); + + @override + Future handleCall_( + ServerContext ctx, SimpleReq request) async => + SimpleResponse(test: 'handleCall called'); + + @override + Future if_(ServerContext ctx, SimpleReq request) async => + SimpleResponse(test: 'If called'); + + @override + Future new_(ServerContext ctx, SimpleReq request) async => + SimpleResponse(test: 'New called'); + + @override + Future x32SillyName_( + ServerContext ctx, SimpleReq request) async => + SimpleResponse(test: '_32SillyName called'); +} + void main() { final throwsInvalidProtocolBufferException = throwsA(TypeMatcher()); @@ -967,4 +996,21 @@ void main() { assertAllFieldsSet(value); }); + + test('Service name disambiguation', () async { + final service = ReservedNamesService(); + expect(await service.handleCall(ServerContext(), 'New', SimpleReq()), + SimpleResponse(test: 'New called')); + expect(await service.handleCall(ServerContext(), 'If', SimpleReq()), + SimpleResponse(test: 'If called')); + expect(await service.handleCall(ServerContext(), 'Abc', SimpleReq()), + SimpleResponse(test: 'Abc called')); + expect(await service.handleCall(ServerContext(), 'abc_Pre', SimpleReq()), + SimpleResponse(test: 'abc_Pre called')); + expect( + await service.handleCall(ServerContext(), '_32SillyName', SimpleReq()), + SimpleResponse(test: '_32SillyName called')); + expect(await service.handleCall(ServerContext(), 'handleCall', SimpleReq()), + SimpleResponse(test: 'handleCall called')); + }); } diff --git a/protoc_plugin/test/goldens/grpc_service.pbgrpc b/protoc_plugin/test/goldens/grpc_service.pbgrpc index a221d1f07..bb9bd75ef 100644 --- a/protoc_plugin/test/goldens/grpc_service.pbgrpc +++ b/protoc_plugin/test/goldens/grpc_service.pbgrpc @@ -30,6 +30,14 @@ class TestClient extends $grpc.Client { '/Test/Bidirectional', ($0.Input value) => value.writeToBuffer(), ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); + static final _$new_ = $grpc.ClientMethod<$0.Input, $0.Output>( + '/Test/New', + ($0.Input value) => value.writeToBuffer(), + ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); + static final _$x32SillyName_ = $grpc.ClientMethod<$0.Input, $0.Output>( + '/Test/_32SillyName', + ($0.Input value) => value.writeToBuffer(), + ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); TestClient($grpc.ClientChannel channel, {$grpc.CallOptions? options, @@ -59,6 +67,16 @@ class TestClient extends $grpc.Client { {$grpc.CallOptions? options}) { return $createStreamingCall(_$bidirectional, request, options: options); } + + $grpc.ResponseFuture<$0.Output> new_($0.Input request, + {$grpc.CallOptions? options}) { + return $createUnaryCall(_$new_, request, options: options); + } + + $grpc.ResponseFuture<$0.Output> x32SillyName_($0.Input request, + {$grpc.CallOptions? options}) { + return $createUnaryCall(_$x32SillyName_, request, options: options); + } } abstract class TestServiceBase extends $grpc.Service { @@ -93,6 +111,20 @@ abstract class TestServiceBase extends $grpc.Service { true, ($core.List<$core.int> value) => $0.Input.fromBuffer(value), ($0.Output value) => value.writeToBuffer())); + $addMethod($grpc.ServiceMethod<$0.Input, $0.Output>( + 'New', + new__Pre, + false, + false, + ($core.List<$core.int> value) => $0.Input.fromBuffer(value), + ($0.Output value) => value.writeToBuffer())); + $addMethod($grpc.ServiceMethod<$0.Input, $0.Output>( + '_32SillyName', + x32SillyName__Pre, + false, + false, + ($core.List<$core.int> value) => $0.Input.fromBuffer(value), + ($0.Output value) => value.writeToBuffer())); } $async.Future<$0.Output> unary_Pre( @@ -105,6 +137,16 @@ abstract class TestServiceBase extends $grpc.Service { yield* serverStreaming(call, await request); } + $async.Future<$0.Output> new__Pre( + $grpc.ServiceCall call, $async.Future<$0.Input> request) async { + return new_(call, await request); + } + + $async.Future<$0.Output> x32SillyName__Pre( + $grpc.ServiceCall call, $async.Future<$0.Input> request) async { + return x32SillyName_(call, await request); + } + $async.Future<$0.Output> unary($grpc.ServiceCall call, $0.Input request); $async.Future<$0.Output> clientStreaming( $grpc.ServiceCall call, $async.Stream<$0.Input> request); @@ -112,4 +154,7 @@ abstract class TestServiceBase extends $grpc.Service { $grpc.ServiceCall call, $0.Input request); $async.Stream<$0.Output> bidirectional( $grpc.ServiceCall call, $async.Stream<$0.Input> request); + $async.Future<$0.Output> new_($grpc.ServiceCall call, $0.Input request); + $async.Future<$0.Output> x32SillyName_( + $grpc.ServiceCall call, $0.Input request); } diff --git a/protoc_plugin/test/protos/proto3_optional.proto b/protoc_plugin/test/protos/proto3_optional.proto index 7dc79315c..628a1b573 100644 --- a/protoc_plugin/test/protos/proto3_optional.proto +++ b/protoc_plugin/test/protos/proto3_optional.proto @@ -17,3 +17,7 @@ message Foo { message Submessage { int32 a = 1; } + +message M1 { + string a = 1; +} diff --git a/protoc_plugin/test/protos/reserved_names.proto b/protoc_plugin/test/protos/reserved_names.proto index dff00aa51..235c45f0e 100644 --- a/protoc_plugin/test/protos/reserved_names.proto +++ b/protoc_plugin/test/protos/reserved_names.proto @@ -256,6 +256,24 @@ message MessageWithReservedEnum { optional ReservedEnum enum = 1; } +message SimpleReq { + optional string test = 1; +} + +message SimpleResponse { + optional string test = 1; +} + +service ReservedNamesService { + rpc New(SimpleReq) returns (SimpleResponse); + rpc If(SimpleReq) returns (SimpleResponse); + rpc Abc(SimpleReq) returns (SimpleResponse); + rpc abc_Pre(SimpleReq) returns (SimpleResponse); + rpc _32SillyName(SimpleReq) returns (SimpleResponse); + rpc handleCall(SimpleReq) returns (SimpleResponse); +} + + enum ReservedEnum { assert = 0; break = 1;