[ggj][codegen][engx] fix!: refactor field into MethodArgument, add enum/msg flags

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

@@ -157,8 +157,9 @@ static List<CommentStatement> createRpcMethodHeaderComment(
           "request", "The request object containing all of the parameters for the API call.");
     } else {
       for (MethodArgument argument : methodArguments) {
-        methodJavadocBuilder.addParam(
-            argument.name(), argument.hasDescription() ? argument.description() : EMPTY_STRING);
+        String description =
+            argument.field().hasDescription() ? argument.field().description() : EMPTY_STRING;
+        methodJavadocBuilder.addParam(argument.name(), description);
       }
     }
 

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

@@ -24,6 +24,10 @@ public abstract class Field {
 
   public abstract TypeNode type();
 
+  public abstract boolean isMessage();
+
+  public abstract boolean isEnum();
+
   public abstract boolean isRepeated();
 
   public abstract boolean isMap();
@@ -43,7 +47,11 @@ public boolean hasResourceReference() {
   }
 
   public static Builder builder() {
-    return new AutoValue_Field.Builder().setIsRepeated(false).setIsMap(false);
+    return new AutoValue_Field.Builder()
+        .setIsMessage(false)
+        .setIsEnum(false)
+        .setIsRepeated(false)
+        .setIsMap(false);
   }
 
   @AutoValue.Builder
@@ -52,6 +60,10 @@ public abstract static class Builder {
 
     public abstract Builder setType(TypeNode type);
 
+    public abstract Builder setIsMessage(boolean isMessage);
+
+    public abstract Builder setIsEnum(boolean isEnum);
+
     public abstract Builder setIsRepeated(boolean isRepeated);
 
     public abstract Builder setIsMap(boolean isMap);

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

@@ -18,28 +18,25 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import java.util.List;
-import javax.annotation.Nullable;
 
 @AutoValue
 public abstract class MethodArgument implements Comparable<MethodArgument> {
+  // The method argument name.
   public abstract String name();
 
+  // The type. This can be different from the associated field (e.g. for resource references).
   public abstract TypeNode type();
 
-  // Records the path of nested types in descending order, excluding type() (which would have
+  // Additional metadata.
+  public abstract Field field();
+
+  // Records the path of nested fields in descending order, excluding type() (which would have
   // appeared as the last element).
-  public abstract ImmutableList<TypeNode> nestedTypes();
+  public abstract ImmutableList<Field> nestedFields();
 
-  // Returns true if this is a resource name helper tyep.
+  // Returns true if this is a resource name helper method argument.
   public abstract boolean isResourceNameHelper();
 
-  @Nullable
-  public abstract String description();
-
-  public boolean hasDescription() {
-    return description() != null;
-  }
-
   @Override
   public int compareTo(MethodArgument other) {
     int compareVal = type().compareTo(other.type());
@@ -51,7 +48,7 @@ public int compareTo(MethodArgument other) {
 
   public static Builder builder() {
     return new AutoValue_MethodArgument.Builder()
-        .setNestedTypes(ImmutableList.of())
+        .setNestedFields(ImmutableList.of())
         .setIsResourceNameHelper(false);
   }
 
@@ -61,11 +58,11 @@ public abstract static class Builder {
 
     public abstract Builder setType(TypeNode type);
 
-    public abstract Builder setNestedTypes(List<TypeNode> nestedTypes);
+    public abstract Builder setField(Field field);
 
-    public abstract Builder setIsResourceNameHelper(boolean isResourceNameHelper);
+    public abstract Builder setNestedFields(List<Field> nestedFields);
 
-    public abstract Builder setDescription(String description);
+    public abstract Builder setIsResourceNameHelper(boolean isResourceNameHelper);
 
     public abstract MethodArgument build();
   }

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

@@ -65,17 +65,17 @@ public static List<List<MethodArgument>> parseMethodSignatures(
       // stringSig.split: ["content", "error"].
       for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) {
         // For resource names, this will be empty.
-        List<TypeNode> argumentTypePathAcc = new ArrayList<>();
+        List<Field> argumentFieldPathAcc = new ArrayList<>();
         // There should be more than one type returned only when we encounter a reousrce name.
-        Map<TypeNode, String> argumentTypes =
-            parseTypeAndCommentFromArgumentName(
+        Map<TypeNode, Field> argumentTypes =
+            parseTypeFromArgumentName(
                 argumentName,
                 servicePackage,
                 inputMessage,
                 messageTypes,
                 resourceNames,
                 patternsToResourceNames,
-                argumentTypePathAcc,
+                argumentFieldPathAcc,
                 outputArgResourceNames);
         int dotLastIndex = argumentName.lastIndexOf(DOT);
         String actualArgumentName =
@@ -88,11 +88,11 @@ public static List<List<MethodArgument>> parseMethodSignatures(
                     e ->
                         MethodArgument.builder()
                             .setName(actualArgumentName)
-                            .setDescription(e.getValue()) // May be null.
                             .setType(e.getKey())
+                            .setField(e.getValue())
                             .setIsResourceNameHelper(
                                 argumentTypes.size() > 1 && !e.getKey().equals(TypeNode.STRING))
-                            .setNestedTypes(argumentTypePathAcc)
+                            .setNestedFields(argumentFieldPathAcc)
                             .build())
                 .collect(Collectors.toList()));
       }
@@ -143,18 +143,17 @@ private static List<List<MethodArgument>> flattenMethodSignatureVariants(
     return methodArgs;
   }
 
-  private static Map<TypeNode, String> parseTypeAndCommentFromArgumentName(
+  private static Map<TypeNode, Field> parseTypeFromArgumentName(
       String argumentName,
       String servicePackage,
       Message inputMessage,
       Map<String, Message> messageTypes,
       Map<String, ResourceName> resourceNames,
       Map<String, ResourceName> patternsToResourceNames,
-      List<TypeNode> argumentTypePathAcc,
+      List<Field> argumentFieldPathAcc,
       Set<ResourceName> outputArgResourceNames) {
 
-    // Comment values may be null.
-    Map<TypeNode, String> typeToComment = new HashMap<>();
+    Map<TypeNode, Field> typeToField = new HashMap<>();
     int dotIndex = argumentName.indexOf(DOT);
     if (dotIndex < 1) {
       Field field = inputMessage.fieldMap().get(argumentName);
@@ -164,8 +163,8 @@ private static Map<TypeNode, String> parseTypeAndCommentFromArgumentName(
               "Field %s not found from input message %s values %s",
               argumentName, inputMessage.name(), inputMessage.fieldMap().keySet()));
       if (!field.hasResourceReference()) {
-        typeToComment.put(field.type(), field.description());
-        return typeToComment;
+        typeToField.put(field.type(), field);
+        return typeToField;
       }
 
       // Parse the resource name tyeps.
@@ -177,14 +176,10 @@ private static Map<TypeNode, String> parseTypeAndCommentFromArgumentName(
               resourceNames,
               patternsToResourceNames);
       outputArgResourceNames.addAll(resourceNameArgs);
-      typeToComment.put(
-          TypeNode.STRING,
-          resourceNameArgs.isEmpty() ? null : resourceNameArgs.get(0).description());
-      typeToComment.putAll(
-          resourceNameArgs.stream()
-              .collect(
-                  Collectors.toMap(r -> r.type(), r -> r.hasDescription() ? r.description() : "")));
-      return typeToComment;
+      typeToField.put(TypeNode.STRING, field);
+      typeToField.putAll(
+          resourceNameArgs.stream().collect(Collectors.toMap(r -> r.type(), r -> field)));
+      return typeToField;
     }
 
     Preconditions.checkState(
@@ -197,6 +192,7 @@ private static Map<TypeNode, String> parseTypeAndCommentFromArgumentName(
     // Must be a sub-message for a type's subfield to be valid.
     Field firstField = inputMessage.fieldMap().get(firstFieldName);
 
+    // Validate the field into which we're descending.
     Preconditions.checkState(
         !firstField.isRepeated(),
         String.format("Cannot descend into repeated field %s", firstField.name()));
@@ -213,15 +209,15 @@ private static Map<TypeNode, String> parseTypeAndCommentFromArgumentName(
         String.format(
             "Message type %s for field reference %s invalid", firstFieldTypeName, firstFieldName));
 
-    argumentTypePathAcc.add(firstFieldType);
-    return parseTypeAndCommentFromArgumentName(
+    argumentFieldPathAcc.add(firstField);
+    return parseTypeFromArgumentName(
         remainingArgumentName,
         servicePackage,
         firstFieldMessage,
         messageTypes,
         resourceNames,
         patternsToResourceNames,
-        argumentTypePathAcc,
+        argumentFieldPathAcc,
         outputArgResourceNames);
   }
 

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

@@ -402,6 +402,8 @@ private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor mess
     return fieldBuilder
         .setName(fieldDescriptor.getName())
         .setType(TypeParser.parseType(fieldDescriptor))
+        .setIsMessage(fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE)
+        .setIsEnum(fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.ENUM)
         .setIsRepeated(fieldDescriptor.isRepeated())
         .setIsMap(fieldDescriptor.isMapField())
         .setResourceReference(resourceReference)

src/test/java/com/google/api/generator/gapic/model/MethodArgumentTest.java

@@ -25,7 +25,12 @@ public class MethodArgumentTest {
   @Test
   public void compareMethodArguments() {
     BiFunction<String, TypeNode, MethodArgument> methodArgFn =
-        (name, type) -> MethodArgument.builder().setName(name).setType(type).build();
+        (name, type) ->
+            MethodArgument.builder()
+                .setName(name)
+                .setType(type)
+                .setField(Field.builder().setName("foobar").setType(type).build())
+                .build();
 
     // Cursory sanity-check of type-only differences, since these are already covered in the
     // TypeNode test.

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

@@ -19,6 +19,7 @@
 
 import com.google.api.generator.engine.ast.TypeNode;
 import com.google.api.generator.engine.ast.VaporReference;
+import com.google.api.generator.gapic.model.Field;
 import com.google.api.generator.gapic.model.MethodArgument;
 import com.google.protobuf.Descriptors.FileDescriptor;
 import com.google.protobuf.Descriptors.ServiceDescriptor;
@@ -58,7 +59,12 @@ public void flattenMethodSignatures_basic() {
     List<String> argumentNames = Arrays.asList(fooName);
 
     BiFunction<String, TypeNode, MethodArgument> methodArgFn =
-        (name, type) -> MethodArgument.builder().setName(name).setType(type).build();
+        (name, type) ->
+            MethodArgument.builder()
+                .setName(name)
+                .setType(type)
+                .setField(Field.builder().setName(name).setType(type).build())
+                .build();
     List<MethodArgument> fooArgs =
         Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream()
             .map(t -> methodArgFn.apply(fooName, t))
@@ -92,7 +98,12 @@ public void flattenMethodSignatures_oneToMany() {
     List<String> argumentNames = Arrays.asList(anInt, fooName);
 
     BiFunction<String, TypeNode, MethodArgument> methodArgFn =
-        (name, type) -> MethodArgument.builder().setName(name).setType(type).build();
+        (name, type) ->
+            MethodArgument.builder()
+                .setName(name)
+                .setType(type)
+                .setField(Field.builder().setName(name).setType(type).build())
+                .build();
     List<MethodArgument> fooArgs =
         Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream()
             .map(t -> methodArgFn.apply(fooName, t))
@@ -127,7 +138,12 @@ public void flattenMethodSignatures_manyToOne() {
     List<String> argumentNames = Arrays.asList(fooName, anInt);
 
     BiFunction<String, TypeNode, MethodArgument> methodArgFn =
-        (name, type) -> MethodArgument.builder().setName(name).setType(type).build();
+        (name, type) ->
+            MethodArgument.builder()
+                .setName(name)
+                .setType(type)
+                .setField(Field.builder().setName(name).setType(type).build())
+                .build();
     List<MethodArgument> fooArgs =
         Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream()
             .map(t -> methodArgFn.apply(fooName, t))
@@ -173,7 +189,12 @@ public void flattenMethodSignatures_manyToMany() {
     List<String> argumentNames = Arrays.asList(fooName, anInt, barName, anotherInt);
 
     BiFunction<String, TypeNode, MethodArgument> methodArgFn =
-        (name, type) -> MethodArgument.builder().setName(name).setType(type).build();
+        (name, type) ->
+            MethodArgument.builder()
+                .setName(name)
+                .setType(type)
+                .setField(Field.builder().setName(name).setType(type).build())
+                .build();
     List<MethodArgument> fooArgs =
         Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream()
             .map(t -> methodArgFn.apply(fooName, t))

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

@@ -76,6 +76,7 @@ public void parseMessages_basic() {
             .setType(
                 TypeNode.withReference(
                     VaporReference.builder().setName("Severity").setPakkage(ECHO_PACKAGE).build()))
+            .setIsEnum(true)
             .build();
     TypeNode echoResponseType =
         TypeNode.withReference(
@@ -87,7 +88,8 @@ public void parseMessages_basic() {
             .setName(echoResponseName)
             .setFields(Arrays.asList(echoResponseContentField, echoResponseSeverityField))
             .build();
-    assertThat(messageTypes.get(echoResponseName)).isEqualTo(echoResponseMessage);
+
+    assertEquals(echoResponseMessage, messageTypes.get(echoResponseName));
   }
 
   @Test
@@ -375,10 +377,10 @@ public void sanitizeDefaultHost_basic() {
   }
 
   private void assertMethodArgumentEquals(
-      String name, TypeNode type, List<TypeNode> nestedTypes, MethodArgument argument) {
+      String name, TypeNode type, List<TypeNode> nestedFields, MethodArgument argument) {
     assertEquals(name, argument.name());
     assertEquals(type, argument.type());
-    assertEquals(nestedTypes, argument.nestedTypes());
+    assertEquals(nestedFields, argument.nestedFields());
   }
 
   private static Reference createStatusReference() {