diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index 6860df55..8df454fd 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -5,6 +5,7 @@ * Ignore `unused_import` diagnostics for `*.pbjson.dart` files. * Revert the change to not generate empty `*.pbenum.dart` files; these can be exported from other enum files. +* Improve the readablity of generated gRPC client files. [#1010]: https://github.com/google/protobuf.dart/issues/1010 diff --git a/protoc_plugin/analysis_options.yaml b/protoc_plugin/analysis_options.yaml index 19b181d0..4d48209f 100644 --- a/protoc_plugin/analysis_options.yaml +++ b/protoc_plugin/analysis_options.yaml @@ -2,7 +2,7 @@ include: ../analysis_options.yaml analyzer: errors: - # The generated code in lib/src/gen trigger this lint. + # The generated code in lib/src/gen triggers this lint. unintended_html_in_doc_comment: ignore exclude: - test/goldens/** diff --git a/protoc_plugin/lib/src/grpc_generator.dart b/protoc_plugin/lib/src/grpc_generator.dart index 93366679..a36d4242 100644 --- a/protoc_plugin/lib/src/grpc_generator.dart +++ b/protoc_plugin/lib/src/grpc_generator.dart @@ -145,15 +145,25 @@ class GrpcServiceGenerator { out.println(); } - for (final method in _methods) { - method.generateClientMethodDescriptor(out); - } - out.println(); + // generate the constructor out.println( '$_clientClassname(super.channel, {super.options, super.interceptors});'); + + // generate the service call methods for (var i = 0; i < _methods.length; i++) { _methods[i].generateClientStub(out, this, i); } + + // generate the method descriptors + out.println(); + if (_methods.isNotEmpty) { + out.println(' // method descriptors'); + out.println(); + + for (final method in _methods) { + method.generateClientMethodDescriptor(out); + } + } }); } @@ -171,10 +181,12 @@ class GrpcServiceGenerator { }); out.println(); for (final method in _methods) { - method.generateServiceMethodPreamble(out); - } - for (final method in _methods) { + if (!method._clientStreaming) { + method.generateServiceMethodPreamble(out); + out.println(); + } method.generateServiceMethodStub(out); + out.println(); } }); } @@ -259,8 +271,7 @@ class _GrpcMethod { 'static final _\$$_dartName = $_clientMethod<$_requestType, $_responseType>('); out.println(' \'/$_serviceName/$_grpcName\','); out.println(' ($_requestType value) => value.writeToBuffer(),'); - out.println( - ' ($coreImportPrefix.List<$coreImportPrefix.int> value) => $_responseType.fromBuffer(value));'); + out.println(' $_responseType.fromBuffer);'); } List _methodDescriptorPath(GrpcServiceGenerator generator, int index) { @@ -284,7 +295,7 @@ class _GrpcMethod { '@$coreImportPrefix.Deprecated(\'This method is deprecated\')'); } out.addBlock( - '$_clientReturnType $_dartName($_argumentType request, {${GrpcServiceGenerator._callOptions}? options}) {', + '$_clientReturnType $_dartName($_argumentType request, {${GrpcServiceGenerator._callOptions}? options,}) {', '}', () { if (_clientStreaming && _serverStreaming) { out.println( @@ -314,8 +325,6 @@ class _GrpcMethod { } void generateServiceMethodPreamble(IndentingWriter out) { - if (_clientStreaming) return; - out.addBlock( '$_serverReturnType ${_dartName}_Pre($_serviceCall \$call, $_future<$_requestType> \$request) async${_serverStreaming ? '*' : ''} {', '}', () { @@ -325,7 +334,6 @@ class _GrpcMethod { out.println('return $_dartName(\$call, await \$request);'); } }); - out.println(); } void generateServiceMethodStub(IndentingWriter out) { diff --git a/protoc_plugin/test/file_generator_test.dart b/protoc_plugin/test/file_generator_test.dart index 632f2144..34d9e9ab 100644 --- a/protoc_plugin/test/file_generator_test.dart +++ b/protoc_plugin/test/file_generator_test.dart @@ -350,7 +350,10 @@ void main() { final writer = IndentingWriter(filename: ''); fg.writeMainHeader(writer); - expectGolden(fg.generateGrpcFile(), 'grpc_service.pbgrpc.dart'); + // We use a '.~dart' file extension here, insead of '.dart', so that + // 'pub publish' won't try and validate that all the imports for this file + // are listed in the pubspec. + expectGolden(fg.generateGrpcFile(), 'grpc_service.pbgrpc.~dart'); }); test('FileGenerator generates imports for .pb.dart files', () { diff --git a/protoc_plugin/test/goldens/grpc_service.pbgrpc.dart b/protoc_plugin/test/goldens/grpc_service.pbgrpc.~dart similarity index 70% rename from protoc_plugin/test/goldens/grpc_service.pbgrpc.dart rename to protoc_plugin/test/goldens/grpc_service.pbgrpc.~dart index 750eb1d1..a0968c47 100644 --- a/protoc_plugin/test/goldens/grpc_service.pbgrpc.dart +++ b/protoc_plugin/test/goldens/grpc_service.pbgrpc.~dart @@ -31,66 +31,58 @@ class TestClient extends $grpc.Client { 'https://www.googleapis.com/auth/datastore', ]; + TestClient(super.channel, {super.options, super.interceptors}); + + $grpc.ResponseFuture<$0.Output> unary($0.Input request, {$grpc.CallOptions? options,}) { + return $createUnaryCall(_$unary, request, options: options); + } + + $grpc.ResponseFuture<$0.Output> clientStreaming($async.Stream<$0.Input> request, {$grpc.CallOptions? options,}) { + return $createStreamingCall(_$clientStreaming, request, options: options).single; + } + + $grpc.ResponseStream<$0.Output> serverStreaming($0.Input request, {$grpc.CallOptions? options,}) { + return $createStreamingCall(_$serverStreaming, $async.Stream.fromIterable([request]), options: options); + } + + $grpc.ResponseStream<$0.Output> bidirectional($async.Stream<$0.Input> request, {$grpc.CallOptions? options,}) { + return $createStreamingCall(_$bidirectional, request, options: options); + } + + $grpc.ResponseFuture<$0.Output> call($0.Input request, {$grpc.CallOptions? options,}) { + return $createUnaryCall(_$call, request, options: options); + } + + $grpc.ResponseFuture<$0.Output> request($0.Input request, {$grpc.CallOptions? options,}) { + return $createUnaryCall(_$request, request, options: options); + } + + // method descriptors + static final _$unary = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/Unary', ($0.Input value) => value.writeToBuffer(), - ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); + $0.Output.fromBuffer); static final _$clientStreaming = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/ClientStreaming', ($0.Input value) => value.writeToBuffer(), - ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); + $0.Output.fromBuffer); static final _$serverStreaming = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/ServerStreaming', ($0.Input value) => value.writeToBuffer(), - ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); + $0.Output.fromBuffer); static final _$bidirectional = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/Bidirectional', ($0.Input value) => value.writeToBuffer(), - ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); + $0.Output.fromBuffer); static final _$call = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/Call', ($0.Input value) => value.writeToBuffer(), - ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); + $0.Output.fromBuffer); static final _$request = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/Request', ($0.Input value) => value.writeToBuffer(), - ($core.List<$core.int> value) => $0.Output.fromBuffer(value)); - - TestClient(super.channel, {super.options, super.interceptors}); - - $grpc.ResponseFuture<$0.Output> unary($0.Input request, - {$grpc.CallOptions? options}) { - return $createUnaryCall(_$unary, request, options: options); - } - - $grpc.ResponseFuture<$0.Output> clientStreaming( - $async.Stream<$0.Input> request, - {$grpc.CallOptions? options}) { - return $createStreamingCall(_$clientStreaming, request, options: options) - .single; - } - - $grpc.ResponseStream<$0.Output> serverStreaming($0.Input request, - {$grpc.CallOptions? options}) { - return $createStreamingCall( - _$serverStreaming, $async.Stream.fromIterable([request]), - options: options); - } - - $grpc.ResponseStream<$0.Output> bidirectional($async.Stream<$0.Input> request, - {$grpc.CallOptions? options}) { - return $createStreamingCall(_$bidirectional, request, options: options); - } - - $grpc.ResponseFuture<$0.Output> call($0.Input request, - {$grpc.CallOptions? options}) { - return $createUnaryCall(_$call, request, options: options); - } - - $grpc.ResponseFuture<$0.Output> request($0.Input request, - {$grpc.CallOptions? options}) { - return $createUnaryCall(_$request, request, options: options); - } + $0.Output.fromBuffer); } @$pb.GrpcServiceName('Test') @@ -142,33 +134,32 @@ abstract class TestServiceBase extends $grpc.Service { ($0.Output value) => value.writeToBuffer())); } - $async.Future<$0.Output> unary_Pre( - $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { + $async.Future<$0.Output> unary_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return unary($call, await $request); } - $async.Stream<$0.Output> serverStreaming_Pre( - $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async* { + $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_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async* { yield* serverStreaming($call, await $request); } - $async.Future<$0.Output> call_Pre( - $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { + $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> call_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return call($call, await $request); } - $async.Future<$0.Output> request_Pre( - $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { + $async.Future<$0.Output> call($grpc.ServiceCall call, $0.Input request); + + $async.Future<$0.Output> request_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return request($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> call($grpc.ServiceCall call, $0.Input request); $async.Future<$0.Output> request($grpc.ServiceCall call, $0.Input request); + }