Skip to content

Commit

Permalink
[ggj][codegen] feat: add comment parsing to methods and fields (#309)
Browse files Browse the repository at this point in the history 
* 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

* feat: add comment parsing to methods and fields

* fix: test

* fix: solidify codegen method order with TypeNode/MethodArg and Comparable

* fix: clean up tests

* fix: merge
  • Loading branch information
@miraleung
miraleung authored Sep 19, 2020
1 parent 35793a7 commit 95ca83e
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 20 deletions.
9 changes: 9 additions & 0 deletions src/main/java/com/google/api/generator/gapic/model/Field.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ public abstract class Field {
@Nullable
public abstract ResourceReference resourceReference();

@Nullable
public abstract String description();

public boolean hasDescription() {
return description() != null;
}

public boolean hasResourceReference() {
return type().equals(TypeNode.STRING) && resourceReference() != null;
}
Expand All @@ -51,6 +58,8 @@ public abstract static class Builder {

public abstract Builder setResourceReference(ResourceReference resourceReference);

public abstract Builder setDescription(String description);

public abstract Field build();
}
}
9 changes: 8 additions & 1 deletion src/main/java/com/google/api/generator/gapic/model/Method.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public enum Stream {
@Nullable
public abstract LongrunningOperation lro();

@Nullable
public abstract String description();

// Example from Expand in echo.proto: Thet TypeNodes that map to
// [["content", "error"], ["content", "error", "info"]].
public abstract ImmutableList<List<MethodArgument>> methodSignatures();
Expand All @@ -50,7 +53,9 @@ public boolean hasLro() {
return lro() != null;
}

// TODO(miraleung): Parse annotations, comments.
public boolean hasDescription() {
return description() != null;
}

public static Builder builder() {
return new AutoValue_Method.Builder()
Expand Down Expand Up @@ -84,6 +89,8 @@ public abstract static class Builder {

public abstract Builder setLro(LongrunningOperation lro);

public abstract Builder setDescription(String description);

public abstract Builder setMethodSignatures(List<List<MethodArgument>> methodSignature);

public abstract Builder setIsPaged(boolean isPaged);
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/com/google/api/generator/gapic/model/MethodArgument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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> {
Expand All @@ -32,6 +33,13 @@ public abstract class MethodArgument implements Comparable<MethodArgument> {
// Returns true if this is a resource name helper tyep.
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());
Expand All @@ -57,6 +65,8 @@ public abstract static class Builder {

public abstract Builder setIsResourceNameHelper(boolean isResourceNameHelper);

public abstract Builder setDescription(String description);

public abstract MethodArgument build();
}
}
10 changes: 10 additions & 0 deletions src/main/java/com/google/api/generator/gapic/model/ResourceName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,17 @@ public abstract class ResourceName {
@Nullable
public abstract String parentMessageName();

@Nullable
public abstract String description();

public boolean hasParentMessageName() {
return parentMessageName() != null;
}

public boolean hasDescription() {
return description() != null;
}

public String resourceTypeName() {
return resourceTypeString().substring(resourceTypeString().indexOf(SLASH) + 1);
}
Expand Down Expand Up @@ -92,6 +99,7 @@ public boolean equals(Object o) {
}

ResourceName other = (ResourceName) o;
// Exclude the description from the resource name because it's just a comment.
return variableName().equals(other.variableName())
&& pakkage().equals(other.pakkage())
&& resourceTypeString().equals(other.resourceTypeString())
Expand Down Expand Up @@ -125,6 +133,8 @@ public abstract static class Builder {

public abstract Builder setParentMessageName(String parentMessageName);

public abstract Builder setDescription(String description);

// Private setters.
abstract Builder setType(TypeNode type);

Expand Down
42 changes: 26 additions & 16 deletions src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.google.common.collect.Lists;
import com.google.protobuf.Descriptors.MethodDescriptor;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -68,8 +67,8 @@ public static List<List<MethodArgument>> parseMethodSignatures(
// For resource names, this will be empty.
List<TypeNode> argumentTypePathAcc = new ArrayList<>();
// There should be more than one type returned only when we encounter a reousrce name.
List<TypeNode> argumentTypes =
parseTypeFromArgumentName(
Map<TypeNode, String> argumentTypes =
parseTypeAndCommentFromArgumentName(
argumentName,
servicePackage,
inputMessage,
Expand All @@ -84,14 +83,15 @@ public static List<List<MethodArgument>> parseMethodSignatures(
argumentNames.add(actualArgumentName);
argumentNameToOverloads.put(
actualArgumentName,
argumentTypes.stream()
argumentTypes.entrySet().stream()
.map(
type ->
e ->
MethodArgument.builder()
.setName(actualArgumentName)
.setType(type)
.setDescription(e.getValue()) // May be null.
.setType(e.getKey())
.setIsResourceNameHelper(
argumentTypes.size() > 1 && !type.equals(TypeNode.STRING))
argumentTypes.size() > 1 && !e.getKey().equals(TypeNode.STRING))
.setNestedTypes(argumentTypePathAcc)
.build())
.collect(Collectors.toList()));
Expand Down Expand Up @@ -143,7 +143,7 @@ private static List<List<MethodArgument>> flattenMethodSignatureVariants(
return methodArgs;
}

private static List<TypeNode> parseTypeFromArgumentName(
private static Map<TypeNode, String> parseTypeAndCommentFromArgumentName(
String argumentName,
String servicePackage,
Message inputMessage,
Expand All @@ -153,6 +153,8 @@ private static List<TypeNode> parseTypeFromArgumentName(
List<TypeNode> argumentTypePathAcc,
Set<ResourceName> outputArgResourceNames) {

// Comment values may be null.
Map<TypeNode, String> typeToComment = new HashMap<>();
int dotIndex = argumentName.indexOf(DOT);
if (dotIndex < 1) {
Field field = inputMessage.fieldMap().get(argumentName);
Expand All @@ -162,19 +164,27 @@ private static List<TypeNode> parseTypeFromArgumentName(
"Field %s not found from input message %s values %s",
argumentName, inputMessage.name(), inputMessage.fieldMap().keySet()));
if (!field.hasResourceReference()) {
return Arrays.asList(field.type());
typeToComment.put(field.type(), field.description());
return typeToComment;
}

// Parse the resource name tyeps.
List<ResourceName> resourceNameArgs =
ResourceReferenceParser.parseResourceNames(
field.resourceReference(), servicePackage, resourceNames, patternsToResourceNames);
field.resourceReference(),
servicePackage,
field.description(),
resourceNames,
patternsToResourceNames);
outputArgResourceNames.addAll(resourceNameArgs);
List<TypeNode> allFieldTypes = new ArrayList<>();
allFieldTypes.add(TypeNode.STRING);
allFieldTypes.addAll(
resourceNameArgs.stream().map(r -> r.type()).collect(Collectors.toList()));
return allFieldTypes;
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;
}

Preconditions.checkState(
Expand Down Expand Up @@ -204,7 +214,7 @@ private static List<TypeNode> parseTypeFromArgumentName(
"Message type %s for field reference %s invalid", firstFieldTypeName, firstFieldName));

argumentTypePathAcc.add(firstFieldType);
return parseTypeFromArgumentName(
return parseTypeAndCommentFromArgumentName(
remainingArgumentName,
servicePackage,
firstFieldMessage,
Expand Down
41 changes: 38 additions & 3 deletions src/main/java/com/google/api/generator/gapic/protoparser/Parser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.model.ResourceReference;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.model.SourceCodeInfoLocation;
import com.google.api.generator.gapic.utils.ResourceNameConstants;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand All @@ -55,6 +56,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
Expand All @@ -64,6 +66,9 @@ public class Parser {
private static final String COLON = ":";
private static final String DEFAULT_PORT = "443";

// Allow other parsers to access this.
protected static final SourceCodeInfoParser SOURCE_CODE_INFO_PARSER = new SourceCodeInfoParser();

static class GapicParserException extends RuntimeException {
public GapicParserException(String errorMessage) {
super(errorMessage);
Expand Down Expand Up @@ -147,7 +152,17 @@ public static List<Service> parseService(
List<String> oauthScopes =
Arrays.asList(serviceOptions.getExtension(ClientProto.oauthScopes).split(COMMA));

return Service.builder()
Service.Builder serviceBuilder = Service.builder();
if (fileDescriptor.toProto().hasSourceCodeInfo()) {
SourceCodeInfoLocation protoServiceLocation =
SOURCE_CODE_INFO_PARSER.getLocation(s);
if (!Objects.isNull(protoServiceLocation)
&& !Strings.isNullOrEmpty(protoServiceLocation.getLeadingComments())) {
serviceBuilder.setDescription(protoServiceLocation.getLeadingComments());
}
}

return serviceBuilder
.setName(s.getName())
.setDefaultHost(defaultHost)
.setOauthScopes(oauthScopes)
Expand Down Expand Up @@ -244,8 +259,18 @@ static List<Method> parseMethods(
for (MethodDescriptor protoMethod : serviceDescriptor.getMethods()) {
// Parse the method.
TypeNode inputType = TypeParser.parseType(protoMethod.getInputType());
Method.Builder methodBuilder = Method.builder();
if (protoMethod.getFile().toProto().hasSourceCodeInfo()) {
SourceCodeInfoLocation protoMethodLocation =
SOURCE_CODE_INFO_PARSER.getLocation(protoMethod);
if (!Objects.isNull(protoMethodLocation)
&& !Strings.isNullOrEmpty(protoMethodLocation.getLeadingComments())) {
methodBuilder.setDescription(protoMethodLocation.getLeadingComments());
}
}

methods.add(
Method.builder()
methodBuilder
.setName(protoMethod.getName())
.setInputType(inputType)
.setOutputType(TypeParser.parseType(protoMethod.getOutputType()))
Expand Down Expand Up @@ -358,7 +383,17 @@ private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor mess
}
}

return Field.builder()
Field.Builder fieldBuilder = Field.builder();
if (fieldDescriptor.getFile().toProto().hasSourceCodeInfo()) {
SourceCodeInfoLocation protoFieldLocation =
SOURCE_CODE_INFO_PARSER.getLocation(fieldDescriptor);
if (!Objects.isNull(protoFieldLocation)
&& !Strings.isNullOrEmpty(protoFieldLocation.getLeadingComments())) {
fieldBuilder.setDescription(protoFieldLocation.getLeadingComments());
}
}

return fieldBuilder
.setName(fieldDescriptor.getName())
.setType(TypeParser.parseType(fieldDescriptor))
.setIsRepeated(fieldDescriptor.isRepeated())
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import javax.annotation.Nullable;

public class ResourceReferenceParser {
private static final String EMPTY_STRING = "";
Expand All @@ -39,6 +40,7 @@ public class ResourceReferenceParser {
public static List<ResourceName> parseResourceNames(
ResourceReference resourceReference,
String servicePackage,
@Nullable String description,
Map<String, ResourceName> resourceNames,
Map<String, ResourceName> patternsToResourceNames) {
ResourceName resourceName = resourceNames.get(resourceReference.resourceTypeString());
Expand All @@ -62,6 +64,7 @@ public static List<ResourceName> parseResourceNames(
servicePackage,
resourceName.pakkage(),
resourceName.resourceTypeString(),
description,
patternsToResourceNames);
// Prevent duplicates.
if (parentResourceNameOpt.isPresent()
Expand All @@ -80,6 +83,7 @@ static Optional<ResourceName> parseParentResourceName(
String servicePackage,
String resourcePackage,
String resourceTypeString,
@Nullable String description,
Map<String, ResourceName> patternsToResourceNames) {
Optional<String> parentPatternOpt = parseParentPattern(pattern);
if (!parentPatternOpt.isPresent()) {
Expand Down Expand Up @@ -143,6 +147,7 @@ static Optional<ResourceName> parseParentResourceName(
.setPakkage(pakkage)
.setResourceTypeString(parentResourceTypeString)
.setPatterns(Arrays.asList(parentPattern))
.setDescription(description)
.build();
patternsToResourceNames.put(parentPattern, parentResourceName);

Expand Down
5 changes: 5 additions & 0 deletions src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public void setUp() {
public void parseParentResourceName_createFromPattern() {
String resourceNamePackage = String.format("%s.common", MAIN_PACKAGE);
String domainName = "cloudbilling.googleapis.com";
String description = "This is the resource name description";
String resourceTypeString = String.format("%s/BillingAccount", domainName);
String parentResourceTypeString = String.format("%s/Project", domainName);
Map<String, ResourceName> patternsToResourceNames = new HashMap<>();
Expand All @@ -59,6 +60,7 @@ public void parseParentResourceName_createFromPattern() {
MAIN_PACKAGE,
resourceNamePackage,
resourceTypeString,
description,
patternsToResourceNames);
assertTrue(parentResourceNameOpt.isPresent());

Expand All @@ -67,6 +69,7 @@ public void parseParentResourceName_createFromPattern() {
assertEquals(Arrays.asList(parentPattern), parentResourceName.patterns());
assertEquals(parentResourceTypeString, parentResourceName.resourceTypeString());
assertEquals(resourceNamePackage, parentResourceName.pakkage());
assertEquals(description, parentResourceName.description());
assertEquals(
TypeNode.withReference(
VaporReference.builder()
Expand All @@ -93,6 +96,7 @@ public void parseParentResourceName_parentResourceNameExists() {
ResourceReferenceParser.parseParentResourceName(
"projects/{project}/folders/{folder}/documents/{document}",
MAIN_PACKAGE,
null,
MAIN_PACKAGE,
"cloudresourcemanager.googleapis.com/Document",
patternsToResourceNames);
Expand All @@ -109,6 +113,7 @@ public void parseParentResourceName_badPattern() {
ResourceReferenceParser.parseParentResourceName(
"projects/{project}/billingAccounts",
MAIN_PACKAGE,
null,
MAIN_PACKAGE,
"cloudbilling.googleapis.com/Feature",
new HashMap<String, ResourceName>());
Expand Down

0 comments on commit 95ca83e

Please sign in to comment.