diff --git a/protoc_plugin/CHANGELOG.md b/protoc_plugin/CHANGELOG.md index 239fb377..38f8fa7e 100644 --- a/protoc_plugin/CHANGELOG.md +++ b/protoc_plugin/CHANGELOG.md @@ -7,11 +7,15 @@ This change requires protobuf-4.0.0. * Generated files now export `GeneratedMessageGenericExtensions` from the protobuf library. ([#503], [#907]) +* Generate doc comments for enum types and values, rpc services and methods. + ([#900], [#909]) [#738]: https://github.com/google/protobuf.dart/issues/738 [#903]: https://github.com/google/protobuf.dart/pull/903 [#503]: https://github.com/google/protobuf.dart/issues/503 [#907]: https://github.com/google/protobuf.dart/pull/907 +[#900]: https://github.com/google/protobuf.dart/issues/900 +[#909]: https://github.com/google/protobuf.dart/pull/909 ## 21.1.2 diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile index 69bca65d..77dca0ba 100644 --- a/protoc_plugin/Makefile +++ b/protoc_plugin/Makefile @@ -24,6 +24,7 @@ TEST_PROTO_LIST = \ google/protobuf/wrappers \ custom_option \ dart_name \ + doc_comments \ default_value_escape \ entity \ enum_extension \ diff --git a/protoc_plugin/lib/src/client_generator.dart b/protoc_plugin/lib/src/client_generator.dart index f927a1f9..2f94b549 100644 --- a/protoc_plugin/lib/src/client_generator.dart +++ b/protoc_plugin/lib/src/client_generator.dart @@ -10,7 +10,29 @@ class ClientApiGenerator { final String className; final Set usedMethodNames = {...reservedMemberNames}; - ClientApiGenerator(this.service, Set usedNames) + /// Tag of `FileDescriptorProto.service`. + static const _fileDescriptorServiceTag = 6; + + /// Tag of `ServiceDescriptorProto.method`. + static const _serviceDescriptorMethodTag = 2; + + /// Index of the service in `FileDescriptorProto.service` repeated field. + final int _repeatedFieldIndex; + + List get _serviceDescriptorPath => [ + ...service.fileGen.fieldPath, + _fileDescriptorServiceTag, + _repeatedFieldIndex + ]; + + List _methodDescriptorPath(int methodRepeatedFieldIndex) => [ + ..._serviceDescriptorPath, + _serviceDescriptorMethodTag, + methodRepeatedFieldIndex + ]; + + ClientApiGenerator( + this.service, Set usedNames, this._repeatedFieldIndex) : className = disambiguateName( avoidInitialUnderscore(service._descriptor.name), usedNames, @@ -20,20 +42,25 @@ class ClientApiGenerator { String get _clientType => '$protobufImportPrefix.RpcClient'; void generate(IndentingWriter out) { + final commentBlock = service.fileGen.commentBlock(_serviceDescriptorPath); + if (commentBlock != null) { + out.println(commentBlock); + } out.addBlock('class ${className}Api {', '}', () { out.println('$_clientType _client;'); out.println('${className}Api(this._client);'); out.println(); - for (final m in service._descriptor.method) { - generateMethod(out, m); + for (var i = 0; i < service._descriptor.method.length; i++) { + generateMethod(out, service._descriptor.method[i], i); } }); out.println(); } // Subclasses can override this. - void generateMethod(IndentingWriter out, MethodDescriptorProto m) { + void generateMethod(IndentingWriter out, MethodDescriptorProto m, + int methodRepeatedFieldIndex) { final methodName = disambiguateName( avoidInitialUnderscore(service._methodName(m.name)), usedMethodNames, @@ -41,6 +68,11 @@ class ClientApiGenerator { final inputType = service._getDartClassName(m.inputType, forMainFile: true); final outputType = service._getDartClassName(m.outputType, forMainFile: true); + final commentBlock = service.fileGen + .commentBlock(_methodDescriptorPath(methodRepeatedFieldIndex)); + if (commentBlock != null) { + out.println(commentBlock); + } out.addBlock( '$asyncImportPrefix.Future<$outputType> $methodName(' '$protobufImportPrefix.ClientContext? ctx, $inputType request) =>', diff --git a/protoc_plugin/lib/src/enum_generator.dart b/protoc_plugin/lib/src/enum_generator.dart index 03c80b0a..145b8e80 100644 --- a/protoc_plugin/lib/src/enum_generator.dart +++ b/protoc_plugin/lib/src/enum_generator.dart @@ -12,10 +12,10 @@ class EnumAlias { class EnumGenerator extends ProtobufContainer { @override - final ProtobufContainer? parent; + final ProtobufContainer parent; @override - final String? classname; + final String classname; @override final String fullName; @@ -32,16 +32,15 @@ class EnumGenerator extends ProtobufContainer { List? _fieldPath; final List _fieldPathSegment; - /// See [[ProtobufContainer] @override - List? get fieldPath => - _fieldPath ??= List.from(parent!.fieldPath!)..addAll(_fieldPathSegment); + List get fieldPath => + _fieldPath ??= List.from(parent.fieldPath!)..addAll(_fieldPathSegment); EnumGenerator._(EnumDescriptorProto descriptor, this.parent, Set usedClassNames, int repeatedFieldIndex, int fieldIdTag) : _fieldPathSegment = [fieldIdTag, repeatedFieldIndex], classname = messageOrEnumClassName(descriptor.name, usedClassNames, - parent: parent!.classname ?? ''), + parent: parent.classname ?? ''), fullName = parent.fullName == '' ? descriptor.name : '${parent.fullName}.${descriptor.name}', @@ -80,10 +79,10 @@ class EnumGenerator extends ProtobufContainer { _nestedFieldTag); @override - String get package => parent!.package; + String get package => parent.package; @override - FileGenerator? get fileGen => parent!.fileGen; + FileGenerator? get fileGen => parent.fileGen; /// Make this enum available as a field type. void register(GenerationContext ctx) { @@ -103,17 +102,15 @@ class EnumGenerator extends ProtobufContainer { static const int _enumValueTag = 2; void generate(IndentingWriter out) { - final comment = fileGen?.commentBlock(fieldPath!); - if (comment != null) { - out.println(comment); + final commentBlock = fileGen?.commentBlock(fieldPath); + if (commentBlock != null) { + out.println(commentBlock); } out.addAnnotatedBlock( 'class $classname extends $protobufImportPrefix.ProtobufEnum {', '}\n', [ NamedLocation( - name: classname!, - fieldPathSegment: fieldPath!, - start: 'class '.length) + name: classname, fieldPathSegment: fieldPath, start: 'class '.length) ], () { // ----------------------------------------------------------------- // Define enum types. @@ -124,14 +121,21 @@ class EnumGenerator extends ProtobufContainer { out.addSuffix( omitEnumNames.constFieldName, omitEnumNames.constDefinition); final conditionalValName = omitEnumNames.createTernary(val.name); + final fieldPathSegment = List.from(fieldPath) + ..addAll([_enumValueTag, _originalCanonicalIndices[i]]); + + final commentBlock = fileGen?.commentBlock(fieldPathSegment); + if (commentBlock != null) { + out.println(commentBlock); + } + out.printlnAnnotated( 'static const $classname $name = ' '$classname._(${val.number}, $conditionalValName);', [ NamedLocation( name: name, - fieldPathSegment: List.from(fieldPath!) - ..addAll([_enumValueTag, _originalCanonicalIndices[i]]), + fieldPathSegment: fieldPathSegment, start: 'static const $classname '.length) ]); } @@ -146,7 +150,7 @@ class EnumGenerator extends ProtobufContainer { [ NamedLocation( name: name, - fieldPathSegment: List.from(fieldPath!) + fieldPathSegment: List.from(fieldPath) ..addAll([_enumValueTag, _originalAliasIndices[i]]), start: 'static const $classname '.length) ]); diff --git a/protoc_plugin/lib/src/file_generator.dart b/protoc_plugin/lib/src/file_generator.dart index b14684e8..e74ae365 100644 --- a/protoc_plugin/lib/src/file_generator.dart +++ b/protoc_plugin/lib/src/file_generator.dart @@ -173,7 +173,8 @@ class FileGenerator extends ProtobufContainer { extensionGenerators.add(ExtensionGenerator.topLevel( descriptor.extension[i], this, usedExtensionNames, i)); } - for (final service in descriptor.service) { + for (var i = 0; i < descriptor.service.length; i++) { + final service = descriptor.service[i]; if (options.useGrpc) { grpcGenerators.add(GrpcServiceGenerator(service, this)); } else { @@ -181,7 +182,7 @@ class FileGenerator extends ProtobufContainer { ServiceGenerator(service, this, usedTopLevelServiceNames); serviceGenerators.add(serviceGen); clientApiGenerators - .add(ClientApiGenerator(serviceGen, usedTopLevelNames)); + .add(ClientApiGenerator(serviceGen, usedTopLevelNames, i)); } } } diff --git a/protoc_plugin/lib/src/message_generator.dart b/protoc_plugin/lib/src/message_generator.dart index 95963d55..bd04d797 100644 --- a/protoc_plugin/lib/src/message_generator.dart +++ b/protoc_plugin/lib/src/message_generator.dart @@ -67,7 +67,6 @@ class MessageGenerator extends ProtobufContainer { final List _fieldPathSegment; - /// See [[ProtobufContainer] @override late final List fieldPath = List.from(parent!.fieldPath!) ..addAll(_fieldPathSegment); @@ -118,8 +117,13 @@ class MessageGenerator extends ProtobufContainer { } } + /// Tag of `FileDescriptorProto.message_type`. static const _topLevelMessageTag = 4; + + /// Tag of `DescriptorProto.nested_type`. static const _nestedMessageTag = 3; + + /// Tag of `DescriptorProto.field`. static const _messageFieldTag = 2; MessageGenerator.topLevel( @@ -319,13 +323,12 @@ class MessageGenerator extends ProtobufContainer { extendedClass = 'GeneratedMessage'; } - var commentBlock = fileGen.commentBlock(fieldPath) ?? ''; - if (commentBlock.isNotEmpty) { - commentBlock = '$commentBlock\n'; + final commentBlock = fileGen.commentBlock(fieldPath); + if (commentBlock != null) { + out.println(commentBlock); } - out.addAnnotatedBlock( - '${commentBlock}class $classname extends $protobufImportPrefix.$extendedClass$mixinClause {', + 'class $classname extends $protobufImportPrefix.$extendedClass$mixinClause {', '}', [ NamedLocation( name: classname, fieldPathSegment: fieldPath, start: 'class '.length) @@ -539,7 +542,7 @@ class MessageGenerator extends ProtobufContainer { final commentBlock = fileGen.commentBlock(memberFieldPath); if (commentBlock != null) { - out.println(commentBlock.trim()); + out.println(commentBlock); } _emitDeprecatedIf(field.isDeprecated, out); diff --git a/protoc_plugin/lib/src/shared.dart b/protoc_plugin/lib/src/shared.dart index b01fcdb4..8fe66bb5 100644 --- a/protoc_plugin/lib/src/shared.dart +++ b/protoc_plugin/lib/src/shared.dart @@ -14,6 +14,14 @@ const grpcImportPrefix = r'$grpc'; const mixinImportPrefix = r'$mixin'; extension FileDescriptorProtoExt on FileGenerator { + /// Convert leading comments of a definition at [path] to Dart doc comment + /// syntax. + /// + /// This never returns an empty string: if the comment is empty it returns + /// `null`. + /// + /// The output can contain multiple lines. None of the lines will have + /// trailing whitespace. String? commentBlock(List path) { final bits = descriptor.sourceCodeInfo.location .where((element) => element.path.toString() == path.toString()) @@ -33,6 +41,10 @@ extension FileDescriptorProtoExt on FileGenerator { } } +/// Convert a comment to Dart doc comment syntax. +/// +/// This is the internal method for [FileDescriptorProtoExt.commentBlock], +/// public to be able to test. String? toDartComment(String value) { if (value.isEmpty) return null; diff --git a/protoc_plugin/test/doc_comments_test.dart b/protoc_plugin/test/doc_comments_test.dart new file mode 100644 index 00000000..20468a33 --- /dev/null +++ b/protoc_plugin/test/doc_comments_test.dart @@ -0,0 +1,22 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:io'; + +import 'package:test/test.dart'; + +import 'golden_file.dart'; + +void main() { + test('Doc comment generation for messages', () { + final actual = File('out/protos/doc_comments.pb.dart').readAsStringSync(); + expectMatchesGoldenFile(actual, 'test/goldens/doc_comments'); + }); + + test('Doc comment generation for enums', () { + final actual = File('out/protos/constructor_args/doc_comments.pbenum.dart') + .readAsStringSync(); + expectMatchesGoldenFile(actual, 'test/goldens/doc_comments.pbenum'); + }); +} diff --git a/protoc_plugin/test/goldens/doc_comments b/protoc_plugin/test/goldens/doc_comments new file mode 100644 index 00000000..406ef045 --- /dev/null +++ b/protoc_plugin/test/goldens/doc_comments @@ -0,0 +1,139 @@ +// +// Generated code. Do not modify. +// source: doc_comments.proto +// +// @dart = 2.12 + +// ignore_for_file: annotate_overrides, camel_case_types, comment_references +// ignore_for_file: constant_identifier_names, library_prefixes +// ignore_for_file: non_constant_identifier_names, prefer_final_fields +// ignore_for_file: unnecessary_import, unnecessary_this, unused_import + +import 'dart:async' as $async; +import 'dart:core' as $core; + +import 'package:protobuf/protobuf.dart' as $pb; + +export 'doc_comments.pbenum.dart'; + +/// This is a message. +class HelloRequest extends $pb.GeneratedMessage { + factory HelloRequest() => create(); + HelloRequest._() : super(); + factory HelloRequest.fromBuffer($core.List<$core.int> i, + [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => + create()..mergeFromBuffer(i, r); + factory HelloRequest.fromJson($core.String i, + [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => + create()..mergeFromJson(i, r); + + static final $pb.BuilderInfo _i = $pb.BuilderInfo( + _omitMessageNames ? '' : 'HelloRequest', + package: const $pb.PackageName(_omitMessageNames ? '' : 'service'), + createEmptyInstance: create) + ..aOS(1, _omitFieldNames ? '' : 'name') + ..hasRequiredFields = false; + + @$core.Deprecated('Using this can add significant overhead to your binary. ' + 'Use [GeneratedMessageGenericExtensions.deepCopy] instead. ' + 'Will be removed in next major version') + HelloRequest clone() => HelloRequest()..mergeFromMessage(this); + @$core.Deprecated('Using this can add significant overhead to your binary. ' + 'Use [GeneratedMessageGenericExtensions.rebuild] instead. ' + 'Will be removed in next major version') + HelloRequest copyWith(void Function(HelloRequest) updates) => + super.copyWith((message) => updates(message as HelloRequest)) + as HelloRequest; + + $pb.BuilderInfo get info_ => _i; + + @$core.pragma('dart2js:noInline') + static HelloRequest create() => HelloRequest._(); + HelloRequest createEmptyInstance() => create(); + static $pb.PbList createRepeated() => + $pb.PbList(); + @$core.pragma('dart2js:noInline') + static HelloRequest getDefault() => _defaultInstance ??= + $pb.GeneratedMessage.$_defaultFor(create); + static HelloRequest? _defaultInstance; + + /// This is a message field. + @$pb.TagNumber(1) + $core.String get name => $_getSZ(0); + @$pb.TagNumber(1) + set name($core.String v) { + $_setString(0, v); + } + + @$pb.TagNumber(1) + $core.bool hasName() => $_has(0); + @$pb.TagNumber(1) + void clearName() => clearField(1); +} + +class HelloReply extends $pb.GeneratedMessage { + factory HelloReply() => create(); + HelloReply._() : super(); + factory HelloReply.fromBuffer($core.List<$core.int> i, + [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => + create()..mergeFromBuffer(i, r); + factory HelloReply.fromJson($core.String i, + [$pb.ExtensionRegistry r = $pb.ExtensionRegistry.EMPTY]) => + create()..mergeFromJson(i, r); + + static final $pb.BuilderInfo _i = $pb.BuilderInfo( + _omitMessageNames ? '' : 'HelloReply', + package: const $pb.PackageName(_omitMessageNames ? '' : 'service'), + createEmptyInstance: create) + ..aOS(1, _omitFieldNames ? '' : 'message') + ..hasRequiredFields = false; + + @$core.Deprecated('Using this can add significant overhead to your binary. ' + 'Use [GeneratedMessageGenericExtensions.deepCopy] instead. ' + 'Will be removed in next major version') + HelloReply clone() => HelloReply()..mergeFromMessage(this); + @$core.Deprecated('Using this can add significant overhead to your binary. ' + 'Use [GeneratedMessageGenericExtensions.rebuild] instead. ' + 'Will be removed in next major version') + HelloReply copyWith(void Function(HelloReply) updates) => + super.copyWith((message) => updates(message as HelloReply)) as HelloReply; + + $pb.BuilderInfo get info_ => _i; + + @$core.pragma('dart2js:noInline') + static HelloReply create() => HelloReply._(); + HelloReply createEmptyInstance() => create(); + static $pb.PbList createRepeated() => $pb.PbList(); + @$core.pragma('dart2js:noInline') + static HelloReply getDefault() => _defaultInstance ??= + $pb.GeneratedMessage.$_defaultFor(create); + static HelloReply? _defaultInstance; + + @$pb.TagNumber(1) + $core.String get message => $_getSZ(0); + @$pb.TagNumber(1) + set message($core.String v) { + $_setString(0, v); + } + + @$pb.TagNumber(1) + $core.bool hasMessage() => $_has(0); + @$pb.TagNumber(1) + void clearMessage() => clearField(1); +} + +/// This is a service. +class GreeterApi { + $pb.RpcClient _client; + GreeterApi(this._client); + + /// This is a service method. + $async.Future sayHello( + $pb.ClientContext? ctx, HelloRequest request) => + _client.invoke( + ctx, 'Greeter', 'SayHello', request, HelloReply()); +} + +const _omitFieldNames = $core.bool.fromEnvironment('protobuf.omit_field_names'); +const _omitMessageNames = + $core.bool.fromEnvironment('protobuf.omit_message_names'); diff --git a/protoc_plugin/test/goldens/doc_comments.pbenum b/protoc_plugin/test/goldens/doc_comments.pbenum new file mode 100644 index 00000000..3a400053 --- /dev/null +++ b/protoc_plugin/test/goldens/doc_comments.pbenum @@ -0,0 +1,36 @@ +// +// Generated code. Do not modify. +// source: doc_comments.proto +// +// @dart = 2.12 + +// ignore_for_file: annotate_overrides, camel_case_types, comment_references +// ignore_for_file: constant_identifier_names, library_prefixes +// ignore_for_file: non_constant_identifier_names, prefer_final_fields +// ignore_for_file: unnecessary_import, unnecessary_this, unused_import + +import 'dart:core' as $core; + +import 'package:protobuf/protobuf.dart' as $pb; + +/// This is an enum. +class A extends $pb.ProtobufEnum { + /// This is an enum variant. + static const A A1 = A._(0, _omitEnumNames ? '' : 'A1'); + + /// This is another enum variant. + static const A A2 = A._(1, _omitEnumNames ? '' : 'A2'); + + static const $core.List values = [ + A1, + A2, + ]; + + static final $core.Map<$core.int, A> _byValue = + $pb.ProtobufEnum.initByValue(values); + static A? valueOf($core.int value) => _byValue[value]; + + const A._($core.int v, $core.String n) : super(v, n); +} + +const _omitEnumNames = $core.bool.fromEnvironment('protobuf.omit_enum_names'); diff --git a/protoc_plugin/test/protos/doc_comments.proto b/protoc_plugin/test/protos/doc_comments.proto new file mode 100644 index 00000000..e4021c5f --- /dev/null +++ b/protoc_plugin/test/protos/doc_comments.proto @@ -0,0 +1,31 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +syntax = "proto3"; + +package service; + +// This is a service. +service Greeter { + // This is a service method. + rpc SayHello (HelloRequest) returns (HelloReply) {} +} + +// This is a message. +message HelloRequest { + // This is a message field. + string name = 1; +} + +message HelloReply { + string message = 1; +} + +// This is an enum. +enum A { + // This is an enum variant. + A1 = 0; + // This is another enum variant. + A2 = 1; +}