[ggj][codegen][comments] fix: preserve newlines and parse itemized li…

…sts in protobuf comments (#443)

* fix: support non-name fields with res-refs in resname def parsing

* fix: add workaround for missing default_host and oauth_scopes annotation

* fix: clarify LRO parsing error messages

* feat: support deeply-nested types in AST and proto message parsing

* fix: prevent resname tokens from matching subcomponents

* fix: use TypeParser for proto message parsing

* fix: use generic types in field instantiation in ServiceClientTest

* fix: prevent descension into map types in nested message parsing

* fix: merge master

* fix: use both map generics in ServiceClientTest codegen

* fix: dir structure of generated files

* test: add asset API gradle pkg rules

* fix: remove unused dep

* test: add logging integration target and goldens, consolidate rules

* fix: fix asset_java_gapic build

* fix: fix test src packaging, update integration goldens

* fix: pass all tokens to instansiate in resname 1-pattern case

* fix: update goldens

* fix: update goldens

* build: add all integration tests to pre-commit hook

* build: run pre-commit when goldens have changed

* build: add integration golden tests to CircleCI

* fix: preserve newlines and parse itemized lists in protobuf comments

* fix: remove extraneous escaper

* fix: update goldens

* fix: update integration goldens for ServiceClient comment names (#444)

.githooks/pre-commit

@@ -132,6 +132,21 @@ then
   fi
 fi
 
+# Check integration tests.
+if [ $NUM_JAVA_FILES_CHANGED -gt 0 ] \
+  || [ $NUM_INTEGRATION_GOLDEN_FILES_CHANGED -gt 0 ] \
+  || [ $NUM_INTEGRATION_BAZEL_FILES_CHANGED -gt 0 ]
+then
+  echo_status "Checking integration tests..."
+  bazel --batch test --disk_cache="$BAZEL_CACHE_DIR" //test/integration/...
+  TEST_STATUS=$?
+  if [ $TEST_STATUS != 0 ]
+  then
+    echo_error "Tests failed." "Please fix them and try again."
+    exit 1
+  fi
+fi
+
 # Check and fix Bazel format.
 if [ $NUM_BAZEL_FILES_CHANGED -gt 0 ]
 then

src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java

@@ -25,6 +25,9 @@
 import com.google.api.generator.gapic.model.Service;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.annotation.Generated;
 
 public class ClientLibraryPackageInfoComposer {
@@ -73,16 +76,34 @@ private static CommentStatement createPackageInfoJavadoc(GapicContext context) {
           javaDocCommentBuilder.addParagraph(
               String.format("%s %s %s", DIVIDER, javaClientName, DIVIDER));
 
-      // TODO(miraleung): Paragraphs
+      // TODO(miraleung): Replace this with a comment converter when we upport CommonMark.
       if (service.hasDescription()) {
-        String[] descriptionParagraphs = service.description().split("\\r?\\n");
+        String[] descriptionParagraphs = service.description().split("\\n\\n");
         for (int i = 0; i < descriptionParagraphs.length; i++) {
-          if (i == 0) {
+          boolean startsWithItemizedList = descriptionParagraphs[i].startsWith(" * ");
+          // Split by listed items, then join newlines.
+          List<String> listItems =
+              Stream.of(descriptionParagraphs[i].split("\\n \\*"))
+                  .map(s -> s.replace("\n", ""))
+                  .collect(Collectors.toList());
+          if (startsWithItemizedList) {
+            // Remove the first asterisk.
+            listItems.set(0, listItems.get(0).substring(2));
+          }
+
+          if (!startsWithItemizedList) {
+            if (i == 0) {
+              javaDocCommentBuilder =
+                  javaDocCommentBuilder.addParagraph(
+                      String.format(SERVICE_DESCRIPTION_HEADER_PATTERN, listItems.get(0)));
+            } else {
+              javaDocCommentBuilder = javaDocCommentBuilder.addParagraph(listItems.get(0));
+            }
+          }
+          if (listItems.size() > 1 || startsWithItemizedList) {
             javaDocCommentBuilder =
-                javaDocCommentBuilder.addParagraph(
-                    String.format(SERVICE_DESCRIPTION_HEADER_PATTERN, descriptionParagraphs[i]));
-          } else {
-            javaDocCommentBuilder = javaDocCommentBuilder.addParagraph(descriptionParagraphs[i]);
+                javaDocCommentBuilder.addUnorderedList(
+                    listItems.subList(startsWithItemizedList ? 0 : 1, listItems.size()));
           }
         }
       }

src/main/java/com/google/api/generator/gapic/composer/ServiceClientCommentComposer.java

@@ -21,9 +21,12 @@
 import com.google.api.generator.gapic.model.MethodArgument;
 import com.google.api.generator.gapic.model.Service;
 import com.google.api.generator.gapic.utils.JavaStyle;
+import com.google.common.base.Strings;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 class ServiceClientCommentComposer {
   // Tokens.
@@ -102,8 +105,11 @@ class ServiceClientCommentComposer {
   static List<CommentStatement> createClassHeaderComments(Service service) {
     JavaDocComment.Builder classHeaderJavadocBuilder = JavaDocComment.builder();
     if (service.hasDescription()) {
-      classHeaderJavadocBuilder.addComment(
-          String.format(SERVICE_DESCRIPTION_SUMMARY_PATTERN, service.description()));
+      classHeaderJavadocBuilder =
+          processProtobufComment(
+              service.description(),
+              classHeaderJavadocBuilder,
+              SERVICE_DESCRIPTION_SUMMARY_PATTERN);
     }
 
     // Service introduction.
@@ -146,7 +152,8 @@ static List<CommentStatement> createRpcMethodHeaderComment(
     JavaDocComment.Builder methodJavadocBuilder = JavaDocComment.builder();
 
     if (method.hasDescription()) {
-      methodJavadocBuilder.addComment(method.description());
+      methodJavadocBuilder =
+          processProtobufComment(method.description(), methodJavadocBuilder, null);
     }
 
     methodJavadocBuilder.addParagraph(METHOD_DESCRIPTION_SAMPLE_CODE_SUMMARY_STRING);
@@ -157,8 +164,11 @@ static List<CommentStatement> createRpcMethodHeaderComment(
           "request", "The request object containing all of the parameters for the API call.");
     } else {
       for (MethodArgument argument : methodArguments) {
+        // TODO(miraleung): Remove the newline replacement when we support CommonMark.
         String description =
-            argument.field().hasDescription() ? argument.field().description() : EMPTY_STRING;
+            argument.field().hasDescription()
+                ? argument.field().description().replace("\n", "")
+                : EMPTY_STRING;
         methodJavadocBuilder.addParam(argument.name(), description);
       }
     }
@@ -190,7 +200,8 @@ static List<CommentStatement> createRpcCallableMethodHeaderComment(Method method
     JavaDocComment.Builder methodJavadocBuilder = JavaDocComment.builder();
 
     if (method.hasDescription()) {
-      methodJavadocBuilder.addComment(method.description());
+      methodJavadocBuilder =
+          processProtobufComment(method.description(), methodJavadocBuilder, null);
     }
 
     methodJavadocBuilder.addParagraph(METHOD_DESCRIPTION_SAMPLE_CODE_SUMMARY_STRING);
@@ -204,4 +215,42 @@ static List<CommentStatement> createRpcCallableMethodHeaderComment(Method method
   private static CommentStatement toSimpleComment(String comment) {
     return CommentStatement.withComment(JavaDocComment.withComment(comment));
   }
+
+  // TODO(miraleung): Replace this with a comment converter when we upport CommonMark.
+  private static JavaDocComment.Builder processProtobufComment(
+      String rawComment, JavaDocComment.Builder originalCommentBuilder, String firstPattern) {
+    JavaDocComment.Builder commentBuilder = originalCommentBuilder;
+    String[] descriptionParagraphs = rawComment.split("\\n\\n");
+    for (int i = 0; i < descriptionParagraphs.length; i++) {
+      boolean startsWithItemizedList = descriptionParagraphs[i].startsWith(" * ");
+      // Split by listed items, then join newlines.
+      List<String> listItems =
+          Stream.of(descriptionParagraphs[i].split("\\n \\*"))
+              .map(s -> s.replace("\n", ""))
+              .collect(Collectors.toList());
+      if (startsWithItemizedList) {
+        // Remove the first asterisk.
+        listItems.set(0, listItems.get(0).substring(2));
+      }
+      if (!startsWithItemizedList) {
+        if (i == 0) {
+          if (!Strings.isNullOrEmpty(firstPattern)) {
+            commentBuilder =
+                commentBuilder.addParagraph(String.format(firstPattern, listItems.get(0)));
+          } else {
+            commentBuilder = commentBuilder.addParagraph(listItems.get(0));
+          }
+        } else {
+          commentBuilder = commentBuilder.addParagraph(listItems.get(0));
+        }
+      }
+      if (listItems.size() > 1 || startsWithItemizedList) {
+        commentBuilder =
+            commentBuilder.addUnorderedList(
+                listItems.subList(startsWithItemizedList ? 0 : 1, listItems.size()));
+      }
+    }
+
+    return commentBuilder;
+  }
 }

src/main/java/com/google/api/generator/gapic/model/SourceCodeInfoLocation.java

@@ -14,8 +14,6 @@
 
 package com.google.api.generator.gapic.model;
 
-import com.google.common.escape.Escaper;
-import com.google.common.escape.Escapers;
 import com.google.protobuf.DescriptorProtos.SourceCodeInfo.Location;
 import javax.annotation.Nonnull;
 
@@ -24,9 +22,6 @@
  * additional documentation on descriptor.proto.
  */
 public class SourceCodeInfoLocation {
-  // Not a singleton because of nested-class instantiation mechanics.
-  private final NewlineEscaper ESCAPER = new NewlineEscaper();
-
   @Nonnull private final Location location;
 
   private SourceCodeInfoLocation(Location location) {
@@ -50,15 +45,6 @@ public String getLeadingDetachedComments(int index) {
   }
 
   private String processProtobufComment(String s) {
-    return ESCAPER.escape(s).trim();
-  }
-
-  private class NewlineEscaper extends Escaper {
-    private final Escaper charEscaper = Escapers.builder().addEscape('\n', "").build();
-
-    @Override
-    public String escape(String sourceString) {
-      return charEscaper.escape(sourceString);
-    }
+    return s.trim();
   }
 }

src/test/java/com/google/api/generator/gapic/protoparser/SourceCodeInfoParserTest.java

@@ -48,7 +48,7 @@ public void setUp() throws Exception {
   public void getServiceInfo() {
     SourceCodeInfoLocation location = parser.getLocation(protoFile.findServiceByName("FooService"));
     assertEquals(
-        "This is a service description. It takes up multiple lines, like so.",
+        "This is a service description.\n It takes up multiple lines, like so.",
         location.getLeadingComments());
 
     location = parser.getLocation(protoFile.findServiceByName("BarService"));
@@ -60,7 +60,7 @@ public void getMethodInfo() {
     ServiceDescriptor service = protoFile.findServiceByName("FooService");
     SourceCodeInfoLocation location = parser.getLocation(service.findMethodByName("FooMethod"));
     assertEquals(
-        "FooMethod does something. This comment also takes up multiple lines.",
+        "FooMethod does something.\n This comment also takes up multiple lines.",
         location.getLeadingComments());
 
     service = protoFile.findServiceByName("BarService");
@@ -73,13 +73,13 @@ public void getOuterMessageInfo() {
     Descriptor message = protoFile.findMessageTypeByName("FooMessage");
     SourceCodeInfoLocation location = parser.getLocation(message);
     assertEquals(
-        "This is a message descxription. Lorum ipsum dolor sit amet consectetur adipiscing elit.",
+        "This is a message descxription.\n Lorum ipsum dolor sit amet consectetur adipiscing elit.",
         location.getLeadingComments());
 
     // Fields.
     location = parser.getLocation(message.findFieldByName("field_one"));
     assertEquals(
-        "This is a field description for field_one. And here is the second line of that"
+        "This is a field description for field_one.\n And here is the second line of that"
             + " description.",
         location.getLeadingComments());
     assertEquals("A field trailing comment.", location.getTrailingComments());