Skip to content

Commit

Permalink
feat(ast): Add support for multi-catch blocks [ggj] (#811)
Browse files Browse the repository at this point in the history 
* feat(ast): support throwing all kinds of expressions

* fix: call backgroundResources.close() on stub.close()

* fix: update goldens

* feat(ast): Add support for multi-catch blocks

* fix: isolate stub.close change to another PR
  • Loading branch information
@miraleung
miraleung committed Aug 2, 2021
1 parent 0817650 commit 55ef1a6
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 45 deletions.
71 changes: 52 additions & 19 deletions src/main/java/com/google/api/generator/engine/ast/TryCatchStatement.java
View file
Expand Up @@ -17,20 +17,23 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

@AutoValue
public abstract class TryCatchStatement implements Statement {

// Required.
public abstract ImmutableList<Statement> tryBody();

// Optional only if the sample code bit is set (i.e. this is sample code).
@Nullable
public abstract VariableExpr catchVariableExpr();
public abstract List<VariableExpr> catchVariableExprs();
// Optional only if the sample code bit is set (i.e. this is sample code).
public abstract ImmutableList<Statement> catchBody();
public abstract List<List<Statement>> catchBlocks();

// Optional.
@Nullable
public abstract AssignmentExpr tryResourceExpr();
Expand All @@ -44,8 +47,9 @@ public void accept(AstNodeVisitor visitor) {

public static Builder builder() {
return new AutoValue_TryCatchStatement.Builder()
.setIsSampleCode(false)
.setCatchBody(Collections.emptyList());
.setCatchVariableExprs(Collections.emptyList())
.setCatchBlocks(Collections.emptyList())
.setIsSampleCode(false);
}

@AutoValue.Builder
Expand All @@ -54,32 +58,61 @@ public abstract static class Builder {

public abstract Builder setTryBody(List<Statement> body);

public abstract Builder setCatchVariableExpr(VariableExpr variableExpr);
public abstract Builder setIsSampleCode(boolean isSampleCode);

public abstract Builder setCatchBody(List<Statement> body);
public Builder addCatch(@Nonnull VariableExpr variableExpr, List<Statement> body) {
List<VariableExpr> catchVarExprs = new ArrayList<>(catchVariableExprs());
catchVarExprs.add(variableExpr);
setCatchVariableExprs(catchVarExprs);

public abstract Builder setIsSampleCode(boolean isSampleCode);
List<List<Statement>> blocks = new ArrayList<>(catchBlocks());
blocks.add(body);
return setCatchBlocks(blocks);
}

// Private.
abstract Builder setCatchVariableExprs(List<VariableExpr> variableExpr);

abstract Builder setCatchBlocks(List<List<Statement>> body);

abstract ImmutableList<Statement> tryBody();

abstract boolean isSampleCode();

abstract List<VariableExpr> catchVariableExprs();

abstract List<List<Statement>> catchBlocks();

abstract TryCatchStatement autoBuild();

public TryCatchStatement build() {
TryCatchStatement tryCatchStatement = autoBuild();
NodeValidator.checkNoNullElements(tryCatchStatement.tryBody(), "try body", "try-catch");
NodeValidator.checkNoNullElements(tryCatchStatement.catchBody(), "catch body", "try-catch");
NodeValidator.checkNoNullElements(tryBody(), "try body", "try-catch");
NodeValidator.checkNoNullElements(
catchVariableExprs(), "catch variable expressions", "try-catch");
catchBlocks()
.forEach(body -> NodeValidator.checkNoNullElements(body, "catch body", "try-catch"));

if (!tryCatchStatement.isSampleCode()) {
Preconditions.checkNotNull(
tryCatchStatement.catchVariableExpr(),
if (!isSampleCode()) {
Preconditions.checkState(
!catchVariableExprs().isEmpty(),
"Catch variable expression must be set for real, non-sample try-catch blocks.");
Preconditions.checkState(
tryCatchStatement.catchVariableExpr().isDecl(),
"Catch variable expression must be a declaration");
catchVariableExprs().stream().allMatch(v -> v.isDecl()),
"Catch variable expressions must all be declarations");
Preconditions.checkState(
TypeNode.isExceptionType(tryCatchStatement.catchVariableExpr().variable().type()),
"Catch variable must be an Exception object reference");
catchVariableExprs().stream()
.allMatch(v -> TypeNode.isExceptionType(v.variable().type())),
"Catch variables must be an Exception object references");
}

return tryCatchStatement;
// Catch any potential future breakage due to changing addCatch above.
Preconditions.checkState(
catchVariableExprs().size() == catchBlocks().size(),
String.format(
"%d catch variables found and %d blocks found, but these numbers must be equal",
catchVariableExprs().size(), catchBlocks().size()));

return autoBuild();
}
}
}
8 changes: 5 additions & 3 deletions src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java
View file
Expand Up @@ -362,11 +362,13 @@ public void visit(TryCatchStatement tryCatchStatement) {
statements(tryCatchStatement.tryBody());

Preconditions.checkState(
!tryCatchStatement.isSampleCode() && tryCatchStatement.catchVariableExpr() != null,
!tryCatchStatement.isSampleCode() && !tryCatchStatement.catchVariableExprs().isEmpty(),
"Import generation should not be invoked on sample code, but was found when visiting a"
+ " try-catch block");
tryCatchStatement.catchVariableExpr().accept(this);
statements(tryCatchStatement.catchBody());
for (int i = 0; i < tryCatchStatement.catchVariableExprs().size(); i++) {
tryCatchStatement.catchVariableExprs().get(i).accept(this);
statements(tryCatchStatement.catchBlocks().get(i));
}
}

@Override
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java
View file
Expand Up @@ -703,17 +703,17 @@ public void visit(TryCatchStatement tryCatchStatement) {
statements(tryCatchStatement.tryBody());
rightBrace();

if (tryCatchStatement.catchVariableExpr() != null) {
for (int i = 0; i < tryCatchStatement.catchVariableExprs().size(); i++) {
space();
buffer.append(CATCH);
space();
leftParen();
tryCatchStatement.catchVariableExpr().accept(this);
tryCatchStatement.catchVariableExprs().get(i).accept(this);
rightParen();
space();
leftBrace();
newline();
statements(tryCatchStatement.catchBody());
statements(tryCatchStatement.catchBlocks().get(i));
rightBrace();
}
newline();
Expand Down
6 changes: 3 additions & 3 deletions ...om/google/api/generator/gapic/composer/common/AbstractServiceClientTestClassComposer.java
View file
Expand Up @@ -536,7 +536,8 @@ private MethodDefinition createRpcTestMethod(
TypeNode.withReference(
ConcreteReference.builder()
.setClazz(List.class)
.setGenerics(Arrays.asList(repeatedPagedResultsField.type().reference()))
.setGenerics(
Arrays.asList(repeatedPagedResultsField.type().reference()))
.build()))
.setName("resources")
.build());
Expand Down Expand Up @@ -824,8 +825,7 @@ protected List<Statement> createRpcExceptionTestStatements(
tryBodyExprs.stream()
.map(e -> ExprStatement.withExpr(e))
.collect(Collectors.toList()))
.setCatchVariableExpr(catchExceptionVarExpr.toBuilder().setIsDecl(true).build())
.setCatchBody(catchBody)
.addCatch(catchExceptionVarExpr.toBuilder().setIsDecl(true).build(), catchBody)
.build();

return Arrays.asList(EMPTY_LINE_STATEMENT, tryCatchBlock);
Expand Down
5 changes: 3 additions & 2 deletions ...ain/java/com/google/api/generator/gapic/composer/grpc/ServiceClientTestClassComposer.java
View file
Expand Up @@ -1046,8 +1046,9 @@ protected List<Statement> createStreamingRpcExceptionTestStatements(
tryBodyExprs.stream()
.map(e -> ExprStatement.withExpr(e))
.collect(Collectors.toList()))
.setCatchVariableExpr(catchExceptionVarExpr.toBuilder().setIsDecl(true).build())
.setCatchBody(createRpcLroExceptionTestCatchBody(catchExceptionVarExpr, true))
.addCatch(
catchExceptionVarExpr.toBuilder().setIsDecl(true).build(),
createRpcLroExceptionTestCatchBody(catchExceptionVarExpr, true))
.build();

statements.add(tryCatchBlock);
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/com/google/api/generator/engine/JavaCodeGeneratorTest.java
View file
Expand Up @@ -606,8 +606,8 @@ private MethodDefinition createPrintShelfListToFile() {
loopShelfList,
ExprStatement.withExpr(writeToFileWriter),
ExprStatement.withExpr(closeFileWriter)))
.setCatchVariableExpr(createVarDeclExpr(ioException))
.setCatchBody(Arrays.asList(ExprStatement.withExpr(printError)))
.addCatch(
createVarDeclExpr(ioException), Arrays.asList(ExprStatement.withExpr(printError)))
.build();

return MethodDefinition.builder()
Expand Down
45 changes: 37 additions & 8 deletions src/test/java/com/google/api/generator/engine/ast/TryCatchStatementTest.java
View file
Expand Up @@ -15,9 +15,11 @@
package com.google.api.generator.engine.ast;

import static com.google.common.truth.Truth.assertThat;
import static junit.framework.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import java.util.Arrays;
import java.util.Collections;
import org.junit.Test;

public class TryCatchStatementTest {
Expand All @@ -32,9 +34,36 @@ public void validTryCatchStatement_simple() {
TryCatchStatement tryCatch =
TryCatchStatement.builder()
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
.setCatchVariableExpr(variableExpr)
.addCatch(variableExpr, Collections.emptyList())
.build();
assertThat(tryCatch.catchVariableExpr()).isEqualTo(variableExpr);
assertEquals(1, tryCatch.catchVariableExprs().size());
assertThat(tryCatch.catchVariableExprs().get(0)).isEqualTo(variableExpr);
}

@Test
public void validTryCatchStatement_simpleMultiBlock() {
VariableExpr firstCatchVarExpr =
VariableExpr.builder()
.setVariable(
createVariable("e", TypeNode.withExceptionClazz(IllegalArgumentException.class)))
.setIsDecl(true)
.build();
VariableExpr secondCatchVarExpr =
VariableExpr.builder()
.setVariable(createVariable("e", TypeNode.withExceptionClazz(RuntimeException.class)))
.setIsDecl(true)
.build();

TryCatchStatement tryCatch =
TryCatchStatement.builder()
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
.addCatch(firstCatchVarExpr, Collections.emptyList())
.addCatch(secondCatchVarExpr, Collections.emptyList())
.build();

assertEquals(2, tryCatch.catchVariableExprs().size());
assertThat(tryCatch.catchVariableExprs().get(0)).isEqualTo(firstCatchVarExpr);
assertThat(tryCatch.catchVariableExprs().get(1)).isEqualTo(secondCatchVarExpr);
}

@Test
Expand All @@ -49,9 +78,9 @@ public void validTryCatchStatement_withResources() {
TryCatchStatement.builder()
.setTryResourceExpr(assignmentExpr)
.setTryBody(Arrays.asList(ExprStatement.withExpr(assignmentExpr)))
.setCatchVariableExpr(variableExpr)
.addCatch(variableExpr, Collections.emptyList())
.build();
assertThat(tryCatch.catchVariableExpr()).isEqualTo(variableExpr);
assertThat(tryCatch.catchVariableExprs().get(0)).isEqualTo(variableExpr);
assertThat(tryCatch.tryResourceExpr()).isEqualTo(assignmentExpr);
}

Expand All @@ -67,7 +96,7 @@ public void validTryCatchStatement_sampleCode() {
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
.setIsSampleCode(true)
.build();
assertThat(tryCatch.catchVariableExpr()).isNull();
assertThat(tryCatch.catchVariableExprs()).isEmpty();
}

@Test
Expand All @@ -78,7 +107,7 @@ public void invalidTryCatchStatement_missingCatchVariable() {
VariableExpr.builder().setVariable(createVariable("e", type)).setIsDecl(true).build();

assertThrows(
NullPointerException.class,
IllegalStateException.class,
() -> {
TryCatchStatement.builder()
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
Expand All @@ -99,7 +128,7 @@ public void invalidTryCatchStatement_catchVariableNotDecl() {
TryCatchStatement tryCatch =
TryCatchStatement.builder()
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
.setCatchVariableExpr(variableExpr)
.addCatch(variableExpr, Collections.emptyList())
.build();
});
}
Expand All @@ -117,7 +146,7 @@ public void invalidTryCatchStatement_nonExceptionReference() {
TryCatchStatement tryCatch =
TryCatchStatement.builder()
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
.setCatchVariableExpr(variableExpr)
.addCatch(variableExpr, Collections.emptyList())
.build();
});
}
Expand Down

0 comments on commit 55ef1a6

Please sign in to comment.