[ggj][codegen] fix!: wrap protobuf location and process comments (#336)

* feat: add protobuf comment parser util

* fix: add basic proto build rules

* feat: add header comments to ServiceClient

* fix: build protoc at test time

* fix!: wrap protobuf location and process comments

* fix: merge

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

@@ -0,0 +1,64 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+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;
+
+/**
+ * A light wrapper around SourceCodeInfo.Location to provide cleaner protobuf comments. Please see
+ * 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) {
+    this.location = location;
+  }
+
+  public static SourceCodeInfoLocation create(@Nonnull Location location) {
+    return new SourceCodeInfoLocation(location);
+  }
+
+  public String getLeadingComments() {
+    return processProtobufComment(location.getLeadingComments());
+  }
+
+  public String getTrailingComments() {
+    return processProtobufComment(location.getTrailingComments());
+  }
+
+  public String getLeadingDetachedComments(int index) {
+    return processProtobufComment(location.getLeadingDetachedComments(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);
+    }
+  }
+}

src/main/java/com/google/api/generator/gapic/protoparser/SourceCodeInfoParser.java

@@ -14,6 +14,7 @@
 
 package com.google.api.generator.gapic.protoparser;
 
+import com.google.api.generator.gapic.model.SourceCodeInfoLocation;
 import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
@@ -60,72 +61,72 @@ public class SourceCodeInfoParser {
 
   /** Gets the location of a message, if available. */
   @Nullable
-  public Location getLocation(Descriptor message) {
+  public SourceCodeInfoLocation getLocation(Descriptor message) {
     FileDescriptor file = message.getFile();
     if (!file.toProto().hasSourceCodeInfo()) {
       return null;
     }
-    return getLocation(file, buildPath(message));
+    return SourceCodeInfoLocation.create(getLocation(file, buildPath(message)));
   }
 
   /** Gets the location of a field, if available. */
   @Nullable
-  public Location getLocation(FieldDescriptor field) {
+  public SourceCodeInfoLocation getLocation(FieldDescriptor field) {
     FileDescriptor file = field.getFile();
     if (!file.toProto().hasSourceCodeInfo()) {
       return null;
     }
-    return getLocation(file, buildPath(field));
+    return SourceCodeInfoLocation.create(getLocation(file, buildPath(field)));
   }
 
   /** Gets the location of a service, if available. */
   @Nullable
-  public Location getLocation(ServiceDescriptor service) {
+  public SourceCodeInfoLocation getLocation(ServiceDescriptor service) {
     FileDescriptor file = service.getFile();
     if (!file.toProto().hasSourceCodeInfo()) {
       return null;
     }
-    return getLocation(file, buildPath(service));
+    return SourceCodeInfoLocation.create(getLocation(file, buildPath(service)));
   }
 
   /** Gets the location of a method, if available. */
   @Nullable
-  public Location getLocation(MethodDescriptor method) {
+  public SourceCodeInfoLocation getLocation(MethodDescriptor method) {
     FileDescriptor file = method.getFile();
     if (!file.toProto().hasSourceCodeInfo()) {
       return null;
     }
-    return getLocation(file, buildPath(method));
+    return SourceCodeInfoLocation.create(getLocation(file, buildPath(method)));
   }
 
   /** Gets the location of an enum type, if available. */
   @Nullable
-  public Location getLocation(EnumDescriptor enumType) {
+  public SourceCodeInfoLocation getLocation(EnumDescriptor enumType) {
     FileDescriptor file = enumType.getFile();
     if (!file.toProto().hasSourceCodeInfo()) {
       return null;
     }
-    return getLocation(file, buildPath(enumType));
+    return SourceCodeInfoLocation.create(getLocation(file, buildPath(enumType)));
   }
 
   /** Gets the location of an enum value, if available. */
   @Nullable
-  public Location getLocation(EnumValueDescriptor enumValue) {
+  public SourceCodeInfoLocation getLocation(EnumValueDescriptor enumValue) {
     FileDescriptor file = enumValue.getFile();
     if (!file.toProto().hasSourceCodeInfo()) {
       return null;
     }
-    return getLocation(file, buildPath(enumValue));
+    return SourceCodeInfoLocation.create(getLocation(file, buildPath(enumValue)));
   }
 
   /** Gets the location of a oneof, if available. */
   @Nullable
-  public Location getLocation(OneofDescriptor oneof) {
+  public SourceCodeInfoLocation getLocation(OneofDescriptor oneof) {
     FileDescriptor file = oneof.getFile();
     if (!file.toProto().hasSourceCodeInfo()) {
       return null;
     }
-    return getLocation(file, buildPath(oneof));
+    return SourceCodeInfoLocation.create(getLocation(file, buildPath(oneof)));
   }
 
   // -----------------------------------------------------------------------------

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

@@ -17,9 +17,9 @@
 import static com.google.common.truth.Truth.assertThat;
 import static junit.framework.Assert.assertEquals;
 
+import com.google.api.generator.gapic.model.SourceCodeInfoLocation;
 import com.google.protobuf.DescriptorProtos.FileDescriptorProto;
 import com.google.protobuf.DescriptorProtos.FileDescriptorSet;
-import com.google.protobuf.DescriptorProtos.SourceCodeInfo.Location;
 import com.google.protobuf.Descriptors.Descriptor;
 import com.google.protobuf.Descriptors.EnumDescriptor;
 import com.google.protobuf.Descriptors.FileDescriptor;
@@ -46,48 +46,47 @@ public void setUp() throws Exception {
 
   @Test
   public void getServiceInfo() {
-    Location location = parser.getLocation(protoFile.findServiceByName("FooService"));
+    SourceCodeInfoLocation location = parser.getLocation(protoFile.findServiceByName("FooService"));
     assertEquals(
-        " This is a service description.\n It takes up multiple lines, like so.\n",
+        "This is a service description. It takes up multiple lines, like so.",
         location.getLeadingComments());
 
     location = parser.getLocation(protoFile.findServiceByName("BarService"));
-    assertEquals(" This is another service description.\n", location.getLeadingComments());
+    assertEquals("This is another service description.", location.getLeadingComments());
   }
 
   @Test
   public void getMethodInfo() {
     ServiceDescriptor service = protoFile.findServiceByName("FooService");
-    Location location = parser.getLocation(service.findMethodByName("FooMethod"));
+    SourceCodeInfoLocation location = parser.getLocation(service.findMethodByName("FooMethod"));
     assertEquals(
-        " FooMethod does something.\n This comment also takes up multiple lines.\n",
+        "FooMethod does something. This comment also takes up multiple lines.",
         location.getLeadingComments());
 
     service = protoFile.findServiceByName("BarService");
     location = parser.getLocation(service.findMethodByName("BarMethod"));
-    assertEquals(" BarMethod does another thing.\n", location.getLeadingComments());
+    assertEquals("BarMethod does another thing.", location.getLeadingComments());
   }
 
   @Test
   public void getOuterMessageInfo() {
     Descriptor message = protoFile.findMessageTypeByName("FooMessage");
-    Location location = parser.getLocation(message);
+    SourceCodeInfoLocation location = parser.getLocation(message);
     assertEquals(
-        " This is a message descxription.\n"
-            + " Lorum ipsum dolor sit amet consectetur adipiscing elit.\n",
+        "This is a message descxription. 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.\n"
-            + " And here is the second line of that description.\n",
+        "This is a field description for field_one. And here is the second line of that"
+            + " description.",
         location.getLeadingComments());
-    assertEquals(" A field trailing comment.\n", location.getTrailingComments());
+    assertEquals("A field trailing comment.", location.getTrailingComments());
 
     location = parser.getLocation(message.findFieldByName("field_two"));
-    assertEquals(" This is another field description.\n", location.getLeadingComments());
-    assertEquals(" Another field trailing comment.\n", location.getTrailingComments());
+    assertEquals("This is another field description.", location.getLeadingComments());
+    assertEquals("Another field trailing comment.", location.getTrailingComments());
   }
 
   @Test
@@ -96,52 +95,52 @@ public void getInnerMessageInfo() {
     assertThat(message).isNotNull();
     message = message.findNestedTypeByName("BarMessage");
 
-    Location location = parser.getLocation(message);
+    SourceCodeInfoLocation location = parser.getLocation(message);
     assertEquals(
-        " This is an inner message description for BarMessage.\n", location.getLeadingComments());
+        "This is an inner message description for BarMessage.", location.getLeadingComments());
 
     // Fields.
     location = parser.getLocation(message.findFieldByName("field_three"));
-    assertEquals(" A third leading comment for field_three.\n", location.getLeadingComments());
+    assertEquals("A third leading comment for field_three.", location.getLeadingComments());
 
     location = parser.getLocation(message.findFieldByName("field_two"));
-    assertEquals("\n This is a block comment for field_two.\n", location.getLeadingComments());
+    assertEquals("This is a block comment for field_two.", location.getLeadingComments());
   }
 
   @Test
   public void getOuterEnumInfo() {
     EnumDescriptor protoEnum = protoFile.findEnumTypeByName("OuterEnum");
-    Location location = parser.getLocation(protoEnum);
-    assertEquals(" This is an outer enum.\n", location.getLeadingComments());
+    SourceCodeInfoLocation location = parser.getLocation(protoEnum);
+    assertEquals("This is an outer enum.", location.getLeadingComments());
 
     // Enum fields.
     location = parser.getLocation(protoEnum.findValueByName("VALUE_UNSPECIFIED"));
-    assertEquals(" Another unspecified value.\n", location.getLeadingComments());
+    assertEquals("Another unspecified value.", location.getLeadingComments());
   }
 
   @Test
   public void getInnerEnumInfo() {
     Descriptor message = protoFile.findMessageTypeByName("FooMessage");
     EnumDescriptor protoEnum = message.findEnumTypeByName("FoodEnum");
-    Location location = parser.getLocation(protoEnum);
-    assertEquals(" An inner enum.\n", location.getLeadingComments());
+    SourceCodeInfoLocation location = parser.getLocation(protoEnum);
+    assertEquals("An inner enum.", location.getLeadingComments());
 
     // Enum fields.
     location = parser.getLocation(protoEnum.findValueByName("RICE"));
-    assertEquals(" 😋 🍚.\n", location.getLeadingComments());
+    assertEquals("😋 🍚.", location.getLeadingComments());
     location = parser.getLocation(protoEnum.findValueByName("CHOCOLATE"));
-    assertEquals(" 🤤 🍫.\n", location.getLeadingComments());
+    assertEquals("🤤 🍫.", location.getLeadingComments());
   }
 
   @Test
   public void getOnoeofInfo() {
     Descriptor message = protoFile.findMessageTypeByName("FooMessage");
     OneofDescriptor protoOneof = message.getOneofs().get(0);
-    Location location = parser.getLocation(protoOneof);
-    assertEquals(" An inner oneof.\n", location.getLeadingComments());
+    SourceCodeInfoLocation location = parser.getLocation(protoOneof);
+    assertEquals("An inner oneof.", location.getLeadingComments());
 
     location = parser.getLocation(protoOneof.getField(0));
-    assertEquals(" An InnerOneof comment for its field.\n", location.getLeadingComments());
+    assertEquals("An InnerOneof comment for its field.", location.getLeadingComments());
   }
 
   /**