Merge remote-tracking branch 'origin/master' into osa1/service_and_me…

…thod_deprecation

benchmarks/bin/deep_copy.dart

@@ -2,7 +2,6 @@
 // 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 'package:protobuf/protobuf.dart';
 import 'package:protobuf_benchmarks/benchmark_base.dart';
 import 'package:protobuf_benchmarks/generated/google_message1_proto2.pb.dart'
     as p2;

protoc_plugin/CHANGELOG.md

@@ -5,13 +5,20 @@
   fields is now `PbMap` (instead of `Map`). ([#903])
 
   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])
 * `deprecated` options in messages, grpc services and methods, and enum types
   and values are now handled to generate Dart `@deprecated` annotations.
   ([#900], [#908])
 
 [#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
 [#908]: https://github.com/google/protobuf.dart/pull/908
 
 ## 21.1.2

protoc_plugin/Makefile

@@ -25,6 +25,7 @@ TEST_PROTO_LIST = \
 	custom_option \
 	dart_name \
 	deprecations \
+	doc_comments \
         default_value_escape \
 	entity \
 	enum_extension \

protoc_plugin/lib/indenting_writer.dart

@@ -173,14 +173,16 @@ class ImportWriter {
   final Set<String> _dartImports = SplayTreeSet<String>();
   final Set<String> _packageImports = SplayTreeSet<String>();
   final Set<String> _fileImports = SplayTreeSet<String>();
-  final Set<String> _exports = SplayTreeSet<String>();
+  final Set<String> _packageExports = SplayTreeSet<String>();
+  final Set<String> _fileExports = SplayTreeSet<String>();
 
   /// Whether any imports were written.
   bool get hasImports =>
       _dartImports.isNotEmpty ||
       _packageImports.isNotEmpty ||
       _fileImports.isNotEmpty ||
-      _exports.isNotEmpty;
+      _packageExports.isNotEmpty ||
+      _fileExports.isNotEmpty;
 
   /// Add an import with an optional import prefix.
   void addImport(String url, {String? prefix}) {
@@ -196,8 +198,15 @@ class ImportWriter {
   }
 
   /// And an export.
-  void addExport(String url) {
-    _exports.add("export '$url';");
+  void addExport(String url, {List<String> members = const []}) {
+    final directive = members.isNotEmpty
+        ? "export '$url' show ${members.join(', ')};"
+        : "export '$url';";
+    if (url.startsWith('package:')) {
+      _packageExports.add(directive);
+    } else {
+      _fileExports.add(directive);
+    }
   }
 
   /// Return the generated text for the set of imports.
@@ -215,9 +224,13 @@ class ImportWriter {
       if (buf.isNotEmpty) buf.writeln();
       _fileImports.forEach(buf.writeln);
     }
-    if (_exports.isNotEmpty) {
+    if (_packageExports.isNotEmpty) {
+      if (buf.isNotEmpty) buf.writeln();
+      _packageExports.forEach(buf.writeln);
+    }
+    if (_fileExports.isNotEmpty) {
       if (buf.isNotEmpty) buf.writeln();
-      _exports.forEach(buf.writeln);
+      _fileExports.forEach(buf.writeln);
     }
 
     return buf.toString();

protoc_plugin/lib/src/client_generator.dart

@@ -10,7 +10,29 @@ class ClientApiGenerator {
   final String className;
   final Set<String> usedMethodNames = {...reservedMemberNames};
 
-  ClientApiGenerator(this.service, Set<String> 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<int> get _serviceDescriptorPath => [
+        ...service.fileGen.fieldPath,
+        _fileDescriptorServiceTag,
+        _repeatedFieldIndex
+      ];
+
+  List<int> _methodDescriptorPath(int methodRepeatedFieldIndex) => [
+        ..._serviceDescriptorPath,
+        _serviceDescriptorMethodTag,
+        methodRepeatedFieldIndex
+      ];
+
+  ClientApiGenerator(
+      this.service, Set<String> usedNames, this._repeatedFieldIndex)
       : className = disambiguateName(
             avoidInitialUnderscore(service._descriptor.name),
             usedNames,
@@ -20,6 +42,10 @@ class ClientApiGenerator {
   String get _clientType => '$protobufImportPrefix.RpcClient';
 
   void generate(IndentingWriter out) {
+    final commentBlock = service.fileGen.commentBlock(_serviceDescriptorPath);
+    if (commentBlock != null) {
+      out.println(commentBlock);
+    }
     if (service._descriptor.options.deprecated) {
       out.println(
           '@$coreImportPrefix.Deprecated(\'This service is deprecated\')');
@@ -29,22 +55,28 @@ class ClientApiGenerator {
       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,
         defaultSuffixes());
     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);
+    }
     if (m.options.deprecated) {
       out.println(
           '@$coreImportPrefix.Deprecated(\'This method is deprecated\')');

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;
@@ -33,14 +33,14 @@ class EnumGenerator extends ProtobufContainer {
   final List<int> _fieldPathSegment;
 
   @override
-  List<int>? get fieldPath =>
-      _fieldPath ??= List.from(parent!.fieldPath!)..addAll(_fieldPathSegment);
+  List<int> get fieldPath =>
+      _fieldPath ??= List.from(parent.fieldPath!)..addAll(_fieldPathSegment);
 
   EnumGenerator._(EnumDescriptorProto descriptor, this.parent,
       Set<String> 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}',
@@ -79,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) {
@@ -102,9 +102,9 @@ 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);
     }
     if (_descriptor.options.deprecated) {
       out.println('@$coreImportPrefix.Deprecated(\'This enum is deprecated\')');
@@ -113,9 +113,7 @@ class EnumGenerator extends ProtobufContainer {
         'class $classname extends $protobufImportPrefix.ProtobufEnum {',
         '}\n', [
       NamedLocation(
-          name: classname!,
-          fieldPathSegment: fieldPath!,
-          start: 'class '.length)
+          name: classname, fieldPathSegment: fieldPath, start: 'class '.length)
     ], () {
       // -----------------------------------------------------------------
       // Define enum types.
@@ -126,18 +124,26 @@ class EnumGenerator extends ProtobufContainer {
         out.addSuffix(
             omitEnumNames.constFieldName, omitEnumNames.constDefinition);
         final conditionalValName = omitEnumNames.createTernary(val.name);
+        final fieldPathSegment = List<int>.from(fieldPath)
+          ..addAll([_enumValueTag, _originalCanonicalIndices[i]]);
+
+        final commentBlock = fileGen?.commentBlock(fieldPathSegment);
+        if (commentBlock != null) {
+          out.println(commentBlock);
+        }
+
         if (val.options.deprecated) {
           out.println(
               '@$coreImportPrefix.Deprecated(\'This enum value is deprecated\')');
         }
+
         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)
             ]);
       }
@@ -152,7 +158,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)
               ]);

protoc_plugin/lib/src/file_generator.dart

@@ -173,15 +173,16 @@ 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 {
         final serviceGen =
             ServiceGenerator(service, this, usedTopLevelServiceNames);
         serviceGenerators.add(serviceGen);
         clientApiGenerators
-            .add(ClientApiGenerator(serviceGen, usedTopLevelNames));
+            .add(ClientApiGenerator(serviceGen, usedTopLevelNames, i));
       }
     }
   }
@@ -341,6 +342,9 @@ class FileGenerator extends ProtobufContainer {
       _addImport(importWriter, config, target, '.pbenum.dart');
     }
 
+    importWriter.addExport(_protobufImportUrl,
+        members: ['GeneratedMessageGenericExtensions']);
+
     for (final publicDependency in descriptor.publicDependency) {
       _addExport(importWriter, config,
           Uri.file(descriptor.dependency[publicDependency]), '.pb.dart');

protoc_plugin/lib/src/message_generator.dart

@@ -67,7 +67,6 @@ class MessageGenerator extends ProtobufContainer {
 
   final List<int> _fieldPathSegment;
 
-  /// See [[ProtobufContainer]
   @override
   late final List<int> 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,17 +323,16 @@ 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);
     }
-
     if (_descriptor.options.deprecated) {
       out.println(
           '@$coreImportPrefix.Deprecated(\'This message is deprecated\')');
     }
     out.addAnnotatedBlock(
-        '${commentBlock}class $classname extends $protobufImportPrefix.$extendedClass$mixinClause {',
+        'class $classname extends $protobufImportPrefix.$extendedClass$mixinClause {',
         '}', [
       NamedLocation(
           name: classname, fieldPathSegment: fieldPath, start: 'class '.length)
@@ -543,7 +546,7 @@ class MessageGenerator extends ProtobufContainer {
 
     final commentBlock = fileGen.commentBlock(memberFieldPath);
     if (commentBlock != null) {
-      out.println(commentBlock.trim());
+      out.println(commentBlock);
     }
 
     _emitDeprecatedIf(field.isDeprecated, out);

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<int> 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;