From 26757b694f25af5d61387657d6faa1e2f48e620a Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Fri, 13 Jun 2025 15:57:04 -0700 Subject: [PATCH 1/4] improve the readablity of generated gRPC client files --- protoc_plugin/CHANGELOG.md | 3 +- protoc_plugin/analysis_options.yaml | 2 +- protoc_plugin/lib/src/grpc_generator.dart | 34 +++--- protoc_plugin/pubspec.yaml | 2 +- .../test/goldens/grpc_service.pbgrpc.dart | 104 ++++++++++-------- 5 files changed, 84 insertions(+), 61 deletions(-) diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index 6860df55..d21dc94a 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -1,10 +1,11 @@ -## 22.4.0-wip +## 22.4.0 * Update how we calculate import prefixes ([#1010]); import prefixes are now unique per-library instead of being unique across all generated libraries. * 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/pubspec.yaml b/protoc_plugin/pubspec.yaml index 91d89a9f..2ffd3670 100644 --- a/protoc_plugin/pubspec.yaml +++ b/protoc_plugin/pubspec.yaml @@ -1,5 +1,5 @@ name: protoc_plugin -version: 22.4.0-wip +version: 22.4.0 description: A protobuf protoc compiler plugin used to generate Dart code. repository: https://github.com/google/protobuf.dart/tree/master/protoc_plugin diff --git a/protoc_plugin/test/goldens/grpc_service.pbgrpc.dart b/protoc_plugin/test/goldens/grpc_service.pbgrpc.dart index c3860f02..0e25916c 100644 --- a/protoc_plugin/test/goldens/grpc_service.pbgrpc.dart +++ b/protoc_plugin/test/goldens/grpc_service.pbgrpc.dart @@ -31,66 +31,75 @@ class TestClient extends $grpc.Client { 'https://www.googleapis.com/auth/datastore', ]; - 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)); - 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)); - 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)); - 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)); - 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)); - 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}) { + $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}) { + $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}) { + $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}) { + $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}) { + $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}) { + $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(), $0.Output.fromBuffer); + static final _$clientStreaming = $grpc.ClientMethod<$0.Input, $0.Output>( + '/Test/ClientStreaming', + ($0.Input value) => value.writeToBuffer(), + $0.Output.fromBuffer); + static final _$serverStreaming = $grpc.ClientMethod<$0.Input, $0.Output>( + '/Test/ServerStreaming', + ($0.Input value) => value.writeToBuffer(), + $0.Output.fromBuffer); + static final _$bidirectional = $grpc.ClientMethod<$0.Input, $0.Output>( + '/Test/Bidirectional', + ($0.Input value) => value.writeToBuffer(), + $0.Output.fromBuffer); + static final _$call = $grpc.ClientMethod<$0.Input, $0.Output>('/Test/Call', + ($0.Input value) => value.writeToBuffer(), $0.Output.fromBuffer); + static final _$request = $grpc.ClientMethod<$0.Input, $0.Output>( + '/Test/Request', + ($0.Input value) => value.writeToBuffer(), + $0.Output.fromBuffer); } @$pb.GrpcServiceName('Test') @@ -147,28 +156,33 @@ abstract class TestServiceBase extends $grpc.Service { return unary($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_Pre( $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async* { yield* serverStreaming($call, await $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_Pre( $grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return call($call, await $request); } + $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); } From ae5270d42fc8492d1b68b03aeb2b7ab2fac81fc2 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Fri, 13 Jun 2025 17:59:59 -0700 Subject: [PATCH 2/4] revert reving to publish --- protoc_plugin/CHANGELOG.md | 2 +- protoc_plugin/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index d21dc94a..8df454fd 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -1,4 +1,4 @@ -## 22.4.0 +## 22.4.0-wip * Update how we calculate import prefixes ([#1010]); import prefixes are now unique per-library instead of being unique across all generated libraries. diff --git a/protoc_plugin/pubspec.yaml b/protoc_plugin/pubspec.yaml index 2ffd3670..91d89a9f 100644 --- a/protoc_plugin/pubspec.yaml +++ b/protoc_plugin/pubspec.yaml @@ -1,5 +1,5 @@ name: protoc_plugin -version: 22.4.0 +version: 22.4.0-wip description: A protobuf protoc compiler plugin used to generate Dart code. repository: https://github.com/google/protobuf.dart/tree/master/protoc_plugin From 4f32008e61d496c7dc131f3a5040c1e36b713b5e Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Fri, 13 Jun 2025 20:50:37 -0700 Subject: [PATCH 3/4] don't use a dart file extension for grpc files --- protoc_plugin/test/file_generator_test.dart | 2 +- ....pbgrpc.dart => grpc_service.pbgrpc.~dart} | 73 +++++++------------ 2 files changed, 26 insertions(+), 49 deletions(-) rename protoc_plugin/test/goldens/{grpc_service.pbgrpc.dart => grpc_service.pbgrpc.~dart} (70%) diff --git a/protoc_plugin/test/file_generator_test.dart b/protoc_plugin/test/file_generator_test.dart index 632f2144..ae9805cd 100644 --- a/protoc_plugin/test/file_generator_test.dart +++ b/protoc_plugin/test/file_generator_test.dart @@ -350,7 +350,7 @@ void main() { final writer = IndentingWriter(filename: ''); fg.writeMainHeader(writer); - expectGolden(fg.generateGrpcFile(), 'grpc_service.pbgrpc.dart'); + 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 0e25916c..3c576d05 100644 --- a/protoc_plugin/test/goldens/grpc_service.pbgrpc.dart +++ b/protoc_plugin/test/goldens/grpc_service.pbgrpc.~dart @@ -33,55 +33,36 @@ class TestClient extends $grpc.Client { TestClient(super.channel, {super.options, super.interceptors}); - $grpc.ResponseFuture<$0.Output> unary( - $0.Input request, { - $grpc.CallOptions? options, - }) { + $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.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> 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, - }) { + $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, - }) { + $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, - }) { + $grpc.ResponseFuture<$0.Output> request($0.Input request, {$grpc.CallOptions? options,}) { return $createUnaryCall(_$request, request, options: options); } - // method descriptors + // method descriptors - static final _$unary = $grpc.ClientMethod<$0.Input, $0.Output>('/Test/Unary', - ($0.Input value) => value.writeToBuffer(), $0.Output.fromBuffer); + static final _$unary = $grpc.ClientMethod<$0.Input, $0.Output>( + '/Test/Unary', + ($0.Input value) => value.writeToBuffer(), + $0.Output.fromBuffer); static final _$clientStreaming = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/ClientStreaming', ($0.Input value) => value.writeToBuffer(), @@ -94,8 +75,10 @@ class TestClient extends $grpc.Client { '/Test/Bidirectional', ($0.Input value) => value.writeToBuffer(), $0.Output.fromBuffer); - static final _$call = $grpc.ClientMethod<$0.Input, $0.Output>('/Test/Call', - ($0.Input value) => value.writeToBuffer(), $0.Output.fromBuffer); + static final _$call = $grpc.ClientMethod<$0.Input, $0.Output>( + '/Test/Call', + ($0.Input value) => value.writeToBuffer(), + $0.Output.fromBuffer); static final _$request = $grpc.ClientMethod<$0.Input, $0.Output>( '/Test/Request', ($0.Input value) => value.writeToBuffer(), @@ -151,38 +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.Future<$0.Output> unary($grpc.ServiceCall call, $0.Input request); - $async.Future<$0.Output> clientStreaming( - $grpc.ServiceCall call, $async.Stream<$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* { + $async.Stream<$0.Output> serverStreaming_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async* { yield* serverStreaming($call, await $request); } - $async.Stream<$0.Output> serverStreaming( - $grpc.ServiceCall call, $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.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 { + $async.Future<$0.Output> call_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return call($call, await $request); } $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 { + $async.Future<$0.Output> request_Pre($grpc.ServiceCall $call, $async.Future<$0.Input> $request) async { return request($call, await $request); } $async.Future<$0.Output> request($grpc.ServiceCall call, $0.Input request); + } From efb89227624ea64bb27eacd20a6c9ccc9011f2bb Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Fri, 13 Jun 2025 21:50:16 -0700 Subject: [PATCH 4/4] add comment --- protoc_plugin/test/file_generator_test.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/protoc_plugin/test/file_generator_test.dart b/protoc_plugin/test/file_generator_test.dart index ae9805cd..34d9e9ab 100644 --- a/protoc_plugin/test/file_generator_test.dart +++ b/protoc_plugin/test/file_generator_test.dart @@ -350,6 +350,9 @@ void main() { final writer = IndentingWriter(filename: ''); fg.writeMainHeader(writer); + // 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'); });