[ggj][engx][comments] feat: parse/print paragraphs and itemized lists in @param Javadocs

.githooks/pre-commit

@@ -177,6 +177,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/engine/ast/JavaDocComment.java

@@ -22,6 +22,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 @AutoValue
 public abstract class JavaDocComment implements Comment {
@@ -44,6 +45,8 @@ public void accept(AstNodeVisitor visitor) {
 
   @AutoValue.Builder
   public abstract static class Builder {
+    static final String PARAM_INDENT = "       ";
+
     // The lack of a getter for these local variables in the external class is WAI.
     String throwsType = null;
     String throwsDescription = null;
@@ -67,7 +70,7 @@ public Builder setDeprecated(String deprecatedText) {
     }
 
     public Builder addParam(String name, String description) {
-      paramsList.add(String.format("@param %s %s", name, description));
+      paramsList.add(String.format("@param %s %s", name, processParamComment(description)));
       return this;
     }
 
@@ -135,5 +138,42 @@ public JavaDocComment build() {
       setComment(String.join("\n", componentsList));
       return autoBuild();
     }
+
+    // TODO(miraleung): Refactor param paragraph parsing to be more robust.
+    private static String processParamComment(String rawComment) {
+      StringBuilder processedCommentBuilder = new StringBuilder();
+      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) {
+            processedCommentBuilder.append(String.format("%s", listItems.get(0)));
+          } else {
+            processedCommentBuilder.append(
+                String.format("%s<p> %s", PARAM_INDENT, listItems.get(0)));
+          }
+        }
+        if (listItems.size() > 1 || startsWithItemizedList) {
+          processedCommentBuilder.append(
+              String.format(
+                  "%s<ul>\n%s\n%s</ul>",
+                  PARAM_INDENT,
+                  listItems.subList(startsWithItemizedList ? 0 : 1, listItems.size()).stream()
+                      .map(li -> String.format("%s  <li>%s", PARAM_INDENT, li))
+                      .reduce("", String::concat),
+                  PARAM_INDENT));
+        }
+      }
+      return processedCommentBuilder.toString();
+    }
   }
 }

src/main/java/com/google/api/generator/engine/escaper/MetacharEscaper.java

@@ -29,7 +29,6 @@ public class MetacharEscaper extends Escaper {
           .addEscape('\b', "\\b")
           .addEscape('\r', "\\r")
           .addEscape('\f', "\\f")
-          .addEscape('\n', "\\n")
           .addEscape('\\', "\\\\")
           .build();
 

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

@@ -169,9 +169,7 @@ static List<CommentStatement> createRpcMethodHeaderComment(
       for (MethodArgument argument : methodArguments) {
         // TODO(miraleung): Remove the newline replacement when we support CommonMark.
         String description =
-            argument.field().hasDescription()
-                ? argument.field().description().replace("\n", "")
-                : EMPTY_STRING;
+            argument.field().hasDescription() ? argument.field().description() : EMPTY_STRING;
         methodJavadocBuilder.addParam(argument.name(), description);
       }
     }

src/test/java/com/google/api/generator/engine/ast/JavaDocCommentTest.java

@@ -37,14 +37,18 @@ public void createJavaDocComment_specialCharacter() {
             .addComment("Service comment may include special characters: \\ \t\b\r&\"\f\n`'@*/")
             .addParagraph("title: GetBigBook: <War and Peace>")
             .addSampleCode(
-                "ApiFuture<Shelf> future = libraryClient.createShelfCallable().futureCall(request);")
+                "ApiFuture<Shelf> future ="
+                    + " libraryClient.createShelfCallable().futureCall(request);")
             .setThrows("Exception", "This is an exception.")
             .build();
     String expected =
-        "Service comment may include special characters: \\\\ \\t\\b\\r&amp;\"\\f\\n`'{@literal @}&#42;/\n"
+        "Service comment may include special characters: \\\\ \\t\\b\\r"
+            + "&amp;\"\\f\n"
+            + "`'{@literal @}&#42;/\n"
             + "<p> title: GetBigBook: <War and Peace>\n"
             + "<pre><code>\n"
-            + "ApiFuture&lt;Shelf&gt; future = libraryClient.createShelfCallable().futureCall(request);\n"
+            + "ApiFuture&lt;Shelf&gt; future ="
+            + " libraryClient.createShelfCallable().futureCall(request);\n"
             + "</code></pre>\n"
             + "@throws Exception This is an exception.";
     assertEquals(javaDocComment.comment(), expected);

src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java

@@ -595,8 +595,8 @@ public void writeLineComment_specialChar() {
     String expected =
         "// usage: gradle run -PmainClass=com.google.example.examples.library.v1.Hopper"
             + " [--args='[--shelf\n"
-            + "// \"Novel\\\\\"`\\b\\t\\n"
-            + "\\r"
+            + "// \"Novel\\\\\"`\\b\\t\n"
+            + "// \\r"
             + "\"]']\n";
     lineComment.accept(writerVisitor);
     assertEquals(writerVisitor.write(), expected);

test/integration/goldens/asset/AssetServiceClient.java

@@ -473,31 +473,47 @@ public final UnaryCallable<DeleteFeedRequest, Empty> deleteFeedCallable() {
    * @param scope Required. A scope can be a project, a folder, or an organization. The search is
    *     limited to the resources within the `scope`. The caller must be granted the
    *     [`cloudasset.assets.searchAllResources`](http://cloud.google.com/asset-inventory/docs/access-control#required_permissions)
-   *     permission on the desired scope. The allowed values are: * projects/{PROJECT_ID} (e.g.,
-   *     "projects/foo-bar") * projects/{PROJECT_NUMBER} (e.g., "projects/12345678") *
-   *     folders/{FOLDER_NUMBER} (e.g., "folders/1234567") * organizations/{ORGANIZATION_NUMBER}
-   *     (e.g., "organizations/123456")
+   *     permission on the desired scope.
+   *     <p>The allowed values are:
+   *     <ul>
+   *       <li>projects/{PROJECT_ID} (e.g., "projects/foo-bar")
+   *       <li>projects/{PROJECT_NUMBER} (e.g., "projects/12345678")
+   *       <li>folders/{FOLDER_NUMBER} (e.g., "folders/1234567")
+   *       <li>organizations/{ORGANIZATION_NUMBER} (e.g., "organizations/123456")
+   *     </ul>
+   *
    * @param query Optional. The query statement. See [how to construct a
    *     query](http://cloud.google.com/asset-inventory/docs/searching-resources#how_to_construct_a_query)
    *     for more information. If not specified or empty, it will search all the resources within
    *     the specified `scope`. Note that the query string is compared against each Cloud IAM policy
    *     binding, including its members, roles, and Cloud IAM conditions. The returned Cloud IAM
    *     policies will only contain the bindings that match your query. To learn more about the IAM
    *     policy structure, see [IAM policy
-   *     doc](https://cloud.google.com/iam/docs/policies#structure). Examples: * `name:Important` to
-   *     find Cloud resources whose name contains "Important" as a word. * `displayName:Impor*` to
-   *     find Cloud resources whose display name contains "Impor" as a prefix. * `description:*por*`
-   *     to find Cloud resources whose description contains "por" as a substring. *
-   *     `location:us-west*` to find Cloud resources whose location is prefixed with "us-west". *
-   *     `labels:prod` to find Cloud resources whose labels contain "prod" as a key or value. *
-   *     `labels.env:prod` to find Cloud resources that have a label "env" and its value is "prod".
-   *     * `labels.env:*` to find Cloud resources that have a label "env". * `Important` to find
-   *     Cloud resources that contain "Important" as a word in any of the searchable fields. *
-   *     `Impor*` to find Cloud resources that contain "Impor" as a prefix in any of the searchable
-   *     fields. * `*por*` to find Cloud resources that contain "por" as a substring in any of the
-   *     searchable fields. * `Important location:(us-west1 OR global)` to find Cloud resources that
-   *     contain "Important" as a word in any of the searchable fields and are also located in the
-   *     "us-west1" region or the "global" location.
+   *     doc](https://cloud.google.com/iam/docs/policies#structure).
+   *     <p>Examples:
+   *     <ul>
+   *       <li>`name:Important` to find Cloud resources whose name contains "Important" as a word.
+   *       <li>`displayName:Impor*` to find Cloud resources whose display name contains "Impor" as a
+   *           prefix.
+   *       <li>`description:*por*` to find Cloud resources whose description contains "por" as a
+   *           substring.
+   *       <li>`location:us-west*` to find Cloud resources whose location is prefixed with
+   *           "us-west".
+   *       <li>`labels:prod` to find Cloud resources whose labels contain "prod" as a key or value.
+   *       <li>`labels.env:prod` to find Cloud resources that have a label "env" and its value is
+   *           "prod".
+   *       <li>`labels.env:*` to find Cloud resources that have a label "env".
+   *       <li>`Important` to find Cloud resources that contain "Important" as a word in any of the
+   *           searchable fields.
+   *       <li>`Impor*` to find Cloud resources that contain "Impor" as a prefix in any of the
+   *           searchable fields.
+   *       <li>`*por*` to find Cloud resources that contain "por" as a substring in any of the
+   *           searchable fields.
+   *       <li>`Important location:(us-west1 OR global)` to find Cloud resources that contain
+   *           "Important" as a word in any of the searchable fields and are also located in the
+   *           "us-west1" region or the "global" location.
+   *     </ul>
+   *
    * @param asset_types Optional. A list of asset types that this request searches for. If empty, it
    *     will search all the [searchable asset
    *     types](https://cloud.google.com/asset-inventory/docs/supported-asset-types#searchable_asset_types).
@@ -567,26 +583,38 @@ public final SearchAllResourcesPagedResponse searchAllResources(
    * @param scope Required. A scope can be a project, a folder, or an organization. The search is
    *     limited to the IAM policies within the `scope`. The caller must be granted the
    *     [`cloudasset.assets.searchAllIamPolicies`](http://cloud.google.com/asset-inventory/docs/access-control#required_permissions)
-   *     permission on the desired scope. The allowed values are: * projects/{PROJECT_ID} (e.g.,
-   *     "projects/foo-bar") * projects/{PROJECT_NUMBER} (e.g., "projects/12345678") *
-   *     folders/{FOLDER_NUMBER} (e.g., "folders/1234567") * organizations/{ORGANIZATION_NUMBER}
-   *     (e.g., "organizations/123456")
+   *     permission on the desired scope.
+   *     <p>The allowed values are:
+   *     <ul>
+   *       <li>projects/{PROJECT_ID} (e.g., "projects/foo-bar")
+   *       <li>projects/{PROJECT_NUMBER} (e.g., "projects/12345678")
+   *       <li>folders/{FOLDER_NUMBER} (e.g., "folders/1234567")
+   *       <li>organizations/{ORGANIZATION_NUMBER} (e.g., "organizations/123456")
+   *     </ul>
+   *
    * @param query Optional. The query statement. See [how to construct a
    *     query](https://cloud.google.com/asset-inventory/docs/searching-iam-policies#how_to_construct_a_query)
    *     for more information. If not specified or empty, it will search all the IAM policies within
-   *     the specified `scope`. Examples: * `policy:[email protected]` to find IAM policy bindings that
-   *     specify user "[email protected]". * `policy:roles/compute.admin` to find IAM policy bindings
-   *     that specify the Compute Admin role. * `policy.role.permissions:storage.buckets.update` to
-   *     find IAM policy bindings that specify a role containing "storage.buckets.update"
-   *     permission. Note that if callers don't have `iam.roles.get` access to a role's included
-   *     permissions, policy bindings that specify this role will be dropped from the search
-   *     results. * `resource:organizations/123456` to find IAM policy bindings that are set on
-   *     "organizations/123456". * `Important` to find IAM policy bindings that contain "Important"
-   *     as a word in any of the searchable fields (except for the included permissions). * `*por*`
-   *     to find IAM policy bindings that contain "por" as a substring in any of the searchable
-   *     fields (except for the included permissions). * `resource:(instance1 OR instance2)
-   *     policy:amy` to find IAM policy bindings that are set on resources "instance1" or
-   *     "instance2" and also specify user "amy".
+   *     the specified `scope`.
+   *     <p>Examples:
+   *     <ul>
+   *       <li>`policy:[email protected]` to find IAM policy bindings that specify user "[email protected]".
+   *       <li>`policy:roles/compute.admin` to find IAM policy bindings that specify the Compute
+   *           Admin role.
+   *       <li>`policy.role.permissions:storage.buckets.update` to find IAM policy bindings that
+   *           specify a role containing "storage.buckets.update" permission. Note that if callers
+   *           don't have `iam.roles.get` access to a role's included permissions, policy bindings
+   *           that specify this role will be dropped from the search results.
+   *       <li>`resource:organizations/123456` to find IAM policy bindings that are set on
+   *           "organizations/123456".
+   *       <li>`Important` to find IAM policy bindings that contain "Important" as a word in any of
+   *           the searchable fields (except for the included permissions).
+   *       <li>`*por*` to find IAM policy bindings that contain "por" as a substring in any of the
+   *           searchable fields (except for the included permissions).
+   *       <li>`resource:(instance1 OR instance2) policy:amy` to find IAM policy bindings that are
+   *           set on resources "instance1" or "instance2" and also specify user "amy".
+   *     </ul>
+   *
    * @throws com.google.api.gax.rpc.ApiException if the remote call fails
    */
   public final SearchAllIamPoliciesPagedResponse searchAllIamPolicies(String scope, String query) {