Disambiguate names of service methods in grpc output

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.

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<String> forbiddenTopLevelNames = <String>[
+const forbiddenTopLevelNames = <String>[
   'List',
   'Function',
   'Map',
   ..._dartReservedWords,
 ];
 
-final List<String> reservedMemberNames = <String>[
+const reservedMemberNames = <String>[
   ..._dartReservedWords,
   ...GeneratedMessage_reservedNames,
   ..._generatedMessageNames
 ];
 
-final List<String> forbiddenExtensionNames = <String>[
+const forbiddenExtensionNames = <String>[
   ..._dartReservedWords,
   ...GeneratedMessage_reservedNames,
   ..._generatedMessageNames
 ];
 
+const serviceReservedMemberNames = <String>[
+  ..._dartReservedWords,
+  ..._serviceNames,
+];
+
+const _serviceNames = <String>[
+  // 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<String> _dartReservedWords = [
+const _dartReservedWords = <String>[
   'assert',
   'bool',
   'break',

protoc_plugin/lib/src/client_generator.dart

@@ -27,28 +27,26 @@ 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);
       }
     });
     out.println();
   }
 
   // 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);');
     });
   }
 }

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 = <String>{...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<String> 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;

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 = <String>{...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<MethodDescriptorProto> get _methodDescriptors => _descriptor.method;
+  List<_ServiceMethod> methods = <_ServiceMethod>[];
 
-  String _methodName(String name) => lowerCaseFirstLetter(name);
+  String _methodName(String name, Set<String> 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,
+  );
+}

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

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'

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<SimpleResponse> abc(ServerContext ctx, SimpleReq request) async =>
+      SimpleResponse(test: 'Abc called');
+
+  @override
+  Future<SimpleResponse> abc_Pre_(ServerContext ctx, SimpleReq request) async =>
+      SimpleResponse(test: 'abc_Pre called');
+
+  @override
+  Future<SimpleResponse> handleCall_(
+          ServerContext ctx, SimpleReq request) async =>
+      SimpleResponse(test: 'handleCall called');
+
+  @override
+  Future<SimpleResponse> if_(ServerContext ctx, SimpleReq request) async =>
+      SimpleResponse(test: 'If called');
+
+  @override
+  Future<SimpleResponse> new_(ServerContext ctx, SimpleReq request) async =>
+      SimpleResponse(test: 'New called');
+
+  @override
+  Future<SimpleResponse> x32SillyName_(
+          ServerContext ctx, SimpleReq request) async =>
+      SimpleResponse(test: '_32SillyName called');
+}
+
 void main() {
   final throwsInvalidProtocolBufferException =
       throwsA(TypeMatcher<InvalidProtocolBufferException>());
@@ -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'));
+  });
 }

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,11 +137,24 @@ 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);
   $async.Stream<$0.Output> serverStreaming(
       $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);
 }

protoc_plugin/test/protos/proto3_optional.proto

@@ -17,3 +17,7 @@ message Foo {
 message Submessage {
  int32 a = 1;
 }
+
+message M1 {
+   string a = 1;
+}