Skip to content

Commit 55ef1a6

Browse files
miraleungmiraleung
authored
feat(ast): Add support for multi-catch blocks [ggj] (#811)
* 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
1 parent 0817650 commit 55ef1a6

8 files changed

Lines changed: 177 additions & 45 deletions

File tree

‎src/main/java/com/google/api/generator/engine/ast/TryCatchStatement.java‎

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,23 @@
1717
import com.google.auto.value.AutoValue;
1818
import com.google.common.base.Preconditions;
1919
import com.google.common.collect.ImmutableList;
20+
import java.util.ArrayList;
2021
import java.util.Collections;
2122
import java.util.List;
23+
import javax.annotation.Nonnull;
2224
import javax.annotation.Nullable;
2325

2426
@AutoValue
2527
public abstract class TryCatchStatement implements Statement {
2628

2729
// Required.
2830
public abstract ImmutableList<Statement> tryBody();
31+
2932
// Optional only if the sample code bit is set (i.e. this is sample code).
30-
@Nullable
31-
public abstract VariableExpr catchVariableExpr();
33+
public abstract List<VariableExpr> catchVariableExprs();
3234
// Optional only if the sample code bit is set (i.e. this is sample code).
33-
public abstract ImmutableList<Statement> catchBody();
35+
public abstract List<List<Statement>> catchBlocks();
36+
3437
// Optional.
3538
@Nullable
3639
public abstract AssignmentExpr tryResourceExpr();
@@ -44,8 +47,9 @@ public void accept(AstNodeVisitor visitor) {
4447

4548
public static Builder builder() {
4649
return new AutoValue_TryCatchStatement.Builder()
47-
.setIsSampleCode(false)
48-
.setCatchBody(Collections.emptyList());
50+
.setCatchVariableExprs(Collections.emptyList())
51+
.setCatchBlocks(Collections.emptyList())
52+
.setIsSampleCode(false);
4953
}
5054

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

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

57-
public abstract Builder setCatchVariableExpr(VariableExpr variableExpr);
61+
public abstract Builder setIsSampleCode(boolean isSampleCode);
5862

59-
public abstract Builder setCatchBody(List<Statement> body);
63+
public Builder addCatch(@Nonnull VariableExpr variableExpr, List<Statement> body) {
64+
List<VariableExpr> catchVarExprs = new ArrayList<>(catchVariableExprs());
65+
catchVarExprs.add(variableExpr);
66+
setCatchVariableExprs(catchVarExprs);
6067

61-
public abstract Builder setIsSampleCode(boolean isSampleCode);
68+
List<List<Statement>> blocks = new ArrayList<>(catchBlocks());
69+
blocks.add(body);
70+
return setCatchBlocks(blocks);
71+
}
72+
73+
// Private.
74+
abstract Builder setCatchVariableExprs(List<VariableExpr> variableExpr);
75+
76+
abstract Builder setCatchBlocks(List<List<Statement>> body);
77+
78+
abstract ImmutableList<Statement> tryBody();
79+
80+
abstract boolean isSampleCode();
81+
82+
abstract List<VariableExpr> catchVariableExprs();
83+
84+
abstract List<List<Statement>> catchBlocks();
6285

6386
abstract TryCatchStatement autoBuild();
6487

6588
public TryCatchStatement build() {
66-
TryCatchStatement tryCatchStatement = autoBuild();
67-
NodeValidator.checkNoNullElements(tryCatchStatement.tryBody(), "try body", "try-catch");
68-
NodeValidator.checkNoNullElements(tryCatchStatement.catchBody(), "catch body", "try-catch");
89+
NodeValidator.checkNoNullElements(tryBody(), "try body", "try-catch");
90+
NodeValidator.checkNoNullElements(
91+
catchVariableExprs(), "catch variable expressions", "try-catch");
92+
catchBlocks()
93+
.forEach(body -> NodeValidator.checkNoNullElements(body, "catch body", "try-catch"));
6994

70-
if (!tryCatchStatement.isSampleCode()) {
71-
Preconditions.checkNotNull(
72-
tryCatchStatement.catchVariableExpr(),
95+
if (!isSampleCode()) {
96+
Preconditions.checkState(
97+
!catchVariableExprs().isEmpty(),
7398
"Catch variable expression must be set for real, non-sample try-catch blocks.");
7499
Preconditions.checkState(
75-
tryCatchStatement.catchVariableExpr().isDecl(),
76-
"Catch variable expression must be a declaration");
100+
catchVariableExprs().stream().allMatch(v -> v.isDecl()),
101+
"Catch variable expressions must all be declarations");
77102
Preconditions.checkState(
78-
TypeNode.isExceptionType(tryCatchStatement.catchVariableExpr().variable().type()),
79-
"Catch variable must be an Exception object reference");
103+
catchVariableExprs().stream()
104+
.allMatch(v -> TypeNode.isExceptionType(v.variable().type())),
105+
"Catch variables must be an Exception object references");
80106
}
81107

82-
return tryCatchStatement;
108+
// Catch any potential future breakage due to changing addCatch above.
109+
Preconditions.checkState(
110+
catchVariableExprs().size() == catchBlocks().size(),
111+
String.format(
112+
"%d catch variables found and %d blocks found, but these numbers must be equal",
113+
catchVariableExprs().size(), catchBlocks().size()));
114+
115+
return autoBuild();
83116
}
84117
}
85118
}

‎src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,11 +362,13 @@ public void visit(TryCatchStatement tryCatchStatement) {
362362
statements(tryCatchStatement.tryBody());
363363

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

372374
@Override

‎src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -703,17 +703,17 @@ public void visit(TryCatchStatement tryCatchStatement) {
703703
statements(tryCatchStatement.tryBody());
704704
rightBrace();
705705

706-
if (tryCatchStatement.catchVariableExpr() != null) {
706+
for (int i = 0; i < tryCatchStatement.catchVariableExprs().size(); i++) {
707707
space();
708708
buffer.append(CATCH);
709709
space();
710710
leftParen();
711-
tryCatchStatement.catchVariableExpr().accept(this);
711+
tryCatchStatement.catchVariableExprs().get(i).accept(this);
712712
rightParen();
713713
space();
714714
leftBrace();
715715
newline();
716-
statements(tryCatchStatement.catchBody());
716+
statements(tryCatchStatement.catchBlocks().get(i));
717717
rightBrace();
718718
}
719719
newline();

‎src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientTestClassComposer.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,8 @@ private MethodDefinition createRpcTestMethod(
536536
TypeNode.withReference(
537537
ConcreteReference.builder()
538538
.setClazz(List.class)
539-
.setGenerics(Arrays.asList(repeatedPagedResultsField.type().reference()))
539+
.setGenerics(
540+
Arrays.asList(repeatedPagedResultsField.type().reference()))
540541
.build()))
541542
.setName("resources")
542543
.build());
@@ -824,8 +825,7 @@ protected List<Statement> createRpcExceptionTestStatements(
824825
tryBodyExprs.stream()
825826
.map(e -> ExprStatement.withExpr(e))
826827
.collect(Collectors.toList()))
827-
.setCatchVariableExpr(catchExceptionVarExpr.toBuilder().setIsDecl(true).build())
828-
.setCatchBody(catchBody)
828+
.addCatch(catchExceptionVarExpr.toBuilder().setIsDecl(true).build(), catchBody)
829829
.build();
830830

831831
return Arrays.asList(EMPTY_LINE_STATEMENT, tryCatchBlock);

‎src/main/java/com/google/api/generator/gapic/composer/grpc/ServiceClientTestClassComposer.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,8 +1046,9 @@ protected List<Statement> createStreamingRpcExceptionTestStatements(
10461046
tryBodyExprs.stream()
10471047
.map(e -> ExprStatement.withExpr(e))
10481048
.collect(Collectors.toList()))
1049-
.setCatchVariableExpr(catchExceptionVarExpr.toBuilder().setIsDecl(true).build())
1050-
.setCatchBody(createRpcLroExceptionTestCatchBody(catchExceptionVarExpr, true))
1049+
.addCatch(
1050+
catchExceptionVarExpr.toBuilder().setIsDecl(true).build(),
1051+
createRpcLroExceptionTestCatchBody(catchExceptionVarExpr, true))
10511052
.build();
10521053

10531054
statements.add(tryCatchBlock);

‎src/test/java/com/google/api/generator/engine/JavaCodeGeneratorTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,8 +606,8 @@ private MethodDefinition createPrintShelfListToFile() {
606606
loopShelfList,
607607
ExprStatement.withExpr(writeToFileWriter),
608608
ExprStatement.withExpr(closeFileWriter)))
609-
.setCatchVariableExpr(createVarDeclExpr(ioException))
610-
.setCatchBody(Arrays.asList(ExprStatement.withExpr(printError)))
609+
.addCatch(
610+
createVarDeclExpr(ioException), Arrays.asList(ExprStatement.withExpr(printError)))
611611
.build();
612612

613613
return MethodDefinition.builder()

‎src/test/java/com/google/api/generator/engine/ast/TryCatchStatementTest.java‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
package com.google.api.generator.engine.ast;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18+
import static junit.framework.Assert.assertEquals;
1819
import static org.junit.Assert.assertThrows;
1920

2021
import java.util.Arrays;
22+
import java.util.Collections;
2123
import org.junit.Test;
2224

2325
public class TryCatchStatementTest {
@@ -32,9 +34,36 @@ public void validTryCatchStatement_simple() {
3234
TryCatchStatement tryCatch =
3335
TryCatchStatement.builder()
3436
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
35-
.setCatchVariableExpr(variableExpr)
37+
.addCatch(variableExpr, Collections.emptyList())
3638
.build();
37-
assertThat(tryCatch.catchVariableExpr()).isEqualTo(variableExpr);
39+
assertEquals(1, tryCatch.catchVariableExprs().size());
40+
assertThat(tryCatch.catchVariableExprs().get(0)).isEqualTo(variableExpr);
41+
}
42+
43+
@Test
44+
public void validTryCatchStatement_simpleMultiBlock() {
45+
VariableExpr firstCatchVarExpr =
46+
VariableExpr.builder()
47+
.setVariable(
48+
createVariable("e", TypeNode.withExceptionClazz(IllegalArgumentException.class)))
49+
.setIsDecl(true)
50+
.build();
51+
VariableExpr secondCatchVarExpr =
52+
VariableExpr.builder()
53+
.setVariable(createVariable("e", TypeNode.withExceptionClazz(RuntimeException.class)))
54+
.setIsDecl(true)
55+
.build();
56+
57+
TryCatchStatement tryCatch =
58+
TryCatchStatement.builder()
59+
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
60+
.addCatch(firstCatchVarExpr, Collections.emptyList())
61+
.addCatch(secondCatchVarExpr, Collections.emptyList())
62+
.build();
63+
64+
assertEquals(2, tryCatch.catchVariableExprs().size());
65+
assertThat(tryCatch.catchVariableExprs().get(0)).isEqualTo(firstCatchVarExpr);
66+
assertThat(tryCatch.catchVariableExprs().get(1)).isEqualTo(secondCatchVarExpr);
3867
}
3968

4069
@Test
@@ -49,9 +78,9 @@ public void validTryCatchStatement_withResources() {
4978
TryCatchStatement.builder()
5079
.setTryResourceExpr(assignmentExpr)
5180
.setTryBody(Arrays.asList(ExprStatement.withExpr(assignmentExpr)))
52-
.setCatchVariableExpr(variableExpr)
81+
.addCatch(variableExpr, Collections.emptyList())
5382
.build();
54-
assertThat(tryCatch.catchVariableExpr()).isEqualTo(variableExpr);
83+
assertThat(tryCatch.catchVariableExprs().get(0)).isEqualTo(variableExpr);
5584
assertThat(tryCatch.tryResourceExpr()).isEqualTo(assignmentExpr);
5685
}
5786

@@ -67,7 +96,7 @@ public void validTryCatchStatement_sampleCode() {
6796
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
6897
.setIsSampleCode(true)
6998
.build();
70-
assertThat(tryCatch.catchVariableExpr()).isNull();
99+
assertThat(tryCatch.catchVariableExprs()).isEmpty();
71100
}
72101

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

80109
assertThrows(
81-
NullPointerException.class,
110+
IllegalStateException.class,
82111
() -> {
83112
TryCatchStatement.builder()
84113
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
@@ -99,7 +128,7 @@ public void invalidTryCatchStatement_catchVariableNotDecl() {
99128
TryCatchStatement tryCatch =
100129
TryCatchStatement.builder()
101130
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
102-
.setCatchVariableExpr(variableExpr)
131+
.addCatch(variableExpr, Collections.emptyList())
103132
.build();
104133
});
105134
}
@@ -117,7 +146,7 @@ public void invalidTryCatchStatement_nonExceptionReference() {
117146
TryCatchStatement tryCatch =
118147
TryCatchStatement.builder()
119148
.setTryBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
120-
.setCatchVariableExpr(variableExpr)
149+
.addCatch(variableExpr, Collections.emptyList())
121150
.build();
122151
});
123152
}

0 commit comments

Comments
 (0)