Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ggj][codegen][ast] fix: solidify codegen method order with AST Comparables #311

Merged
merged 8 commits into from
Sep 19, 2020
27 changes: 26 additions & 1 deletion src/main/java/com/google/api/generator/engine/ast/TypeNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import javax.annotation.Nullable;

@AutoValue
public abstract class TypeNode implements AstNode {
public abstract class TypeNode implements AstNode, Comparable<TypeNode> {
static final Reference EXCEPTION_REFERENCE = ConcreteReference.withClazz(Exception.class);
public static final Reference WILDCARD_REFERENCE = ConcreteReference.wildcard();

Expand Down Expand Up @@ -88,6 +88,31 @@ public enum TypeKind {
@Nullable
public abstract Reference reference();

@Override
public int compareTo(TypeNode other) {
// Ascending order of name.
if (isPrimitiveType()) {
if (other.isPrimitiveType()) {
return typeKind().name().compareTo(other.typeKind().name());
}
// b is a reference type or null, so a < b.
return -1;
}

if (this.equals(TypeNode.NULL)) {
// Can't self-compare, so we don't need to check whether the other one is TypeNode.NULL.
return other.isPrimitiveType() ? 1 : -1;
}

miraleung marked this conversation as resolved.
Show resolved Hide resolved
if (other.isPrimitiveType() || other.equals(TypeNode.NULL)) {
return 1;
}

// Both are reference types.
// TODO(miraleung): Replace this with a proper reference Comaparator.
return reference().fullName().compareTo(other.reference().fullName());
}

public static Builder builder() {
return new AutoValue_TypeNode.Builder().setIsArray(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ public GapicClass generate(Service service, Map<String, Message> messageTypes) {

ClassDefinition classDef =
ClassDefinition.builder()
.setHeaderCommentStatements(
ServiceClientCommentComposer.createClassHeaderComments(service))
.setPackageString(pakkage)
.setAnnotations(createClassAnnotations(types))
.setImplementsTypes(createClassImplements(types))
Expand Down Expand Up @@ -471,7 +473,26 @@ private static List<MethodDefinition> createMethodVariants(
Preconditions.checkNotNull(
inputMessage, String.format("Message %s not found", methodInputTypeName));

for (List<MethodArgument> signature : method.methodSignatures()) {
// Make the method signature order deterministic, which helps with unit testing and per-version
// diffs.
List<List<MethodArgument>> sortedMethodSignatures =
method.methodSignatures().stream()
.sorted(
(s1, s2) -> {
if (s1.size() != s2.size()) {
return s1.size() - s2.size();
}
for (int i = 0; i < s1.size(); i++) {
int compareVal = s1.get(i).compareTo(s2.get(i));
if (compareVal != 0) {
return compareVal;
}
}
return 0;
})
.collect(Collectors.toList());

for (List<MethodArgument> signature : sortedMethodSignatures) {
// Get the argument list.
List<VariableExpr> arguments =
signature.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,62 @@
import com.google.api.generator.engine.ast.CommentStatement;
import com.google.api.generator.engine.ast.JavaDocComment;
import com.google.api.generator.engine.ast.TypeNode;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.utils.JavaStyle;
import java.util.Arrays;
import java.util.List;

class ServiceClientCommentComposer {
// Tokens.
private static final String COLON = ":";

// Constants.
private static final String SERVICE_DESCRIPTION_INTRO_STRING =
"This class provides the ability to make remote calls to the backing service through method "
+ "calls that map to API methods. Sample code to get started:";
private static final String SERVICE_DESCRIPTION_CLOSE_STRING =
"Note: close() needs to be called on the echoClient object to clean up resources such as "
+ "threads. In the example above, try-with-resources is used, which automatically calls "
+ "close().";
private static final String SERVICE_DESCRIPTION_SURFACE_SUMMARY_STRING =
"The surface of this class includes several types of Java methods for each of the API's "
+ "methods:";
private static final String SERVICE_DESCRIPTION_SURFACE_CODA_STRING =
"See the individual methods for example code.";
private static final String SERVICE_DESCRIPTION_RESOURCE_NAMES_FORMATTING_STRING =
"Many parameters require resource names to be formatted in a particular way. To assist with"
+ " these names, this class includes a format method for each type of name, and"
+ " additionally a parse method to extract the individual identifiers contained within"
+ " names that are returned.";
private static final String SERVICE_DESCRIPTION_CREDENTIALS_SUMMARY_STRING =
"To customize credentials:";
private static final String SERVICE_DESCRIPTION_ENDPOINT_SUMMARY_STRING =
"To customize the endpoint:";

private static final List<String> SERVICE_DESCRIPTION_SURFACE_DESCRIPTION =
Arrays.asList(
"A \"flattened\" method. With this type of method, the fields of the request type have"
+ " been converted into function parameters. It may be the case that not all fields"
+ " are available as parameters, and not every API method will have a flattened"
+ " method entry point.",
"A \"request object\" method. This type of method only takes one parameter, a request"
+ " object, which must be constructed before the call. Not every API method will"
+ " have a request object method.",
"A \"callable\" method. This type of method takes no parameters and returns an immutable "
+ "API callable object, which can be used to initiate calls to the service.");

// Patterns.
private static final String CREATE_METHOD_STUB_ARG_PATTERN =
"Constructs an instance of EchoClient, using the given stub for making calls. This is for"
+ " advanced usage - prefer using create(%s).";

private static final String SERVICE_DESCRIPTION_CUSTOMIZE_SUMMARY_PATTERN =
"This class can be customized by passing in a custom instance of %s to create(). For"
+ " example:";

private static final String SERVICE_DESCRIPTION_SUMMARY_PATTERN = "Service Description: %s";

// Comments.
static final CommentStatement CREATE_METHOD_NO_ARG_COMMENT =
toSimpleComment("Constructs an instance of EchoClient with default settings.");

Expand All @@ -45,6 +93,41 @@ class ServiceClientCommentComposer {
"Returns the OperationsClient that can be used to query the status of a long-running"
+ " operation returned by another API method call.");

static List<CommentStatement> createClassHeaderComments(Service service) {
JavaDocComment.Builder classHeaderJavadocBuilder = JavaDocComment.builder();
if (service.hasDescription()) {
classHeaderJavadocBuilder.addComment(
String.format(SERVICE_DESCRIPTION_SUMMARY_PATTERN, service.description()));
}

// Service introduction.
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_INTRO_STRING);
// TODO(summerji): Add sample code here.

// API surface description.
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_CLOSE_STRING);
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_SURFACE_SUMMARY_STRING);
classHeaderJavadocBuilder.addOrderedList(SERVICE_DESCRIPTION_SURFACE_DESCRIPTION);
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_SURFACE_CODA_STRING);

// Formatting resource names.
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_RESOURCE_NAMES_FORMATTING_STRING);

// Customization examples.
classHeaderJavadocBuilder.addParagraph(
String.format(
SERVICE_DESCRIPTION_CUSTOMIZE_SUMMARY_PATTERN,
String.format("%sSettings", JavaStyle.toUpperCamelCase(service.name()))));
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_CREDENTIALS_SUMMARY_STRING);
// TODO(summerji): Add credentials' customization sample code here.
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_ENDPOINT_SUMMARY_STRING);
// TODO(summerji): Add endpoint customization sample code here.

return Arrays.asList(
CommentComposer.AUTO_GENERATED_CLASS_COMMENT,
CommentStatement.withComment(classHeaderJavadocBuilder.build()));
}

static CommentStatement createCreateMethodStubArgComment(TypeNode settingsType) {
return toSimpleComment(
String.format(CREATE_METHOD_STUB_ARG_PATTERN, settingsType.reference().name()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import java.util.List;

@AutoValue
public abstract class MethodArgument {
public abstract class MethodArgument implements Comparable<MethodArgument> {
public abstract String name();

public abstract TypeNode type();
Expand All @@ -32,6 +32,15 @@ public abstract class MethodArgument {
// Returns true if this is a resource name helper tyep.
public abstract boolean isResourceNameHelper();

@Override
public int compareTo(MethodArgument other) {
int compareVal = type().compareTo(other.type());
if (compareVal == 0) {
compareVal = name().compareTo(other.name());
}
return compareVal;
}

public static Builder builder() {
return new AutoValue_MethodArgument.Builder()
.setNestedTypes(ImmutableList.of())
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/com/google/api/generator/gapic/model/Service.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
package com.google.api.generator.gapic.model;

import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import java.util.List;
import javax.annotation.Nullable;

@AutoValue
public abstract class Service {
Expand All @@ -32,7 +34,12 @@ public abstract class Service {

public abstract ImmutableList<Method> methods();

// TODO(miraleung): Get comments.
@Nullable
public abstract String description();

public boolean hasDescription() {
return !Strings.isNullOrEmpty(description());
}

public static Builder builder() {
return new AutoValue_Service.Builder().setMethods(ImmutableList.of());
Expand All @@ -52,6 +59,8 @@ public abstract static class Builder {

public abstract Builder setMethods(List<Method> methods);

public abstract Builder setDescription(String description);

public abstract Service build();
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Loading