From aeb972b27648c6e86ce04e39a13bb1139616f03a Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:22:04 -0700 Subject: [PATCH 01/32] Integrate OperationExpr into GeneralForStatement --- .../engine/ast/GeneralForStatement.java | 78 +++---- .../engine/writer/ImportWriterVisitor.java | 4 +- .../engine/writer/JavaWriterVisitor.java | 10 +- .../composer/BatchingDescriptorComposer.java | 8 +- .../engine/ast/GeneralForStatementTest.java | 191 +++++++++++++++++- .../engine/writer/JavaWriterVisitorTest.java | 31 ++- 6 files changed, 263 insertions(+), 59 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index fba06d97d1..a84e43efa9 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -19,22 +19,18 @@ import com.google.common.collect.ImmutableList; import java.util.Collections; import java.util.List; +import javax.annotation.Nullable; @AutoValue public abstract class GeneralForStatement implements Statement { - public abstract Expr initializationExpr(); + @Nullable + public abstract AssignmentExpr initializationExpr(); - // TODO(summerji): Integrate OperationExpr here. Start by uncommenting the following section to - // replace the localVariableExpr and maxSizeExpr getters after it. - /* - // Uses the same terminology as https://docs.oracle.com/javase/tutorial/java/nutsandbolts/for.html. + @Nullable public abstract Expr terminationExpr(); - public abstract Expr incrementExpr(); - */ - - public abstract VariableExpr localVariableExpr(); - public abstract Expr maxSizeExpr(); + @Nullable + public abstract Expr incrementExpr(); public abstract ImmutableList body(); @@ -43,14 +39,26 @@ public void accept(AstNodeVisitor visitor) { visitor.visit(this); } - // Convenience wrapper. + // incrementWith is convenience wrapper to generate index-base for-loop with lower and upper bound + // and post increment on variable, like code in ```for (int i = 0; i < getMax(); i++) {..}``` + // TODO (developer): Add more convenience wrapper for the future generation needs. public static GeneralForStatement incrementWith( - VariableExpr variableExpr, Expr maxSizeExpr, List body) { - // TODO(summerji): Do some integration here, in JavaWriterVisitor, in ImportWriterVisitor, and - // add more tests. + VariableExpr localVariableExpr, + ValueExpr initialValueExpr, + Expr maxSizeExpr, + List body) { return builder() - .setLocalVariableExpr(variableExpr.toBuilder().setIsDecl(false).build()) - .setMaxSizeExpr(maxSizeExpr) + .setInitializationExpr( + AssignmentExpr.builder() + .setVariableExpr(localVariableExpr) + .setValueExpr(initialValueExpr) + .build()) + .setTerminationExpr( + RelationalOperationExpr.lessThanWithExprs( + localVariableExpr.toBuilder().setIsDecl(false).build(), maxSizeExpr)) + .setIncrementExpr( + UnaryOperationExpr.postfixIncrementWithExpr( + localVariableExpr.toBuilder().setIsDecl(false).build())) .setBody(body) .build(); } @@ -61,22 +69,22 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { - public abstract Builder setInitializationExpr(Expr initializationExpr); + abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); - public abstract Builder setBody(List body); + abstract Builder setTerminationExpr(Expr terminationExpr); - // Private. - abstract Builder setLocalVariableExpr(VariableExpr variableExpr); + abstract Builder setIncrementExpr(Expr incrementExpr); - abstract Builder setMaxSizeExpr(Expr maxSizeExpr); - - abstract VariableExpr localVariableExpr(); + abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. public GeneralForStatement build() { - VariableExpr varExpr = localVariableExpr(); + GeneralForStatement generalForStatement = autoBuild(); + VariableExpr varExpr = generalForStatement.initializationExpr().variableExpr(); + Expr terminationExpr = generalForStatement.terminationExpr(); + Expr incrementExpr = generalForStatement.incrementExpr(); Preconditions.checkState( varExpr.scope().equals(ScopeNode.LOCAL), String.format( @@ -87,16 +95,18 @@ public GeneralForStatement build() { String.format( "Variable %s in a general for-loop cannot be static or final", varExpr.variable().identifier().name())); - setInitializationExpr( - AssignmentExpr.builder() - .setVariableExpr(varExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr( - ValueExpr.withValue( - PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build())) - .build()); - // TODO(summerji): Remove the following two lines. - // This temporary workaround will be removed soon, so it doesn't need a test. - setLocalVariableExpr(varExpr.toBuilder().setIsDecl(false).build()); + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be expression statement."); return autoBuild(); } } diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index fa2648a545..98fb5b5b51 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -281,8 +281,8 @@ public void visit(ForStatement forStatement) { @Override public void visit(GeneralForStatement generalForStatement) { generalForStatement.initializationExpr().accept(this); - generalForStatement.localVariableExpr().accept(this); - generalForStatement.maxSizeExpr().accept(this); + generalForStatement.terminationExpr().accept(this); + generalForStatement.incrementExpr().accept(this); statements(generalForStatement.body()); } diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 7a1e4d3b6d..2d970a0a96 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -540,17 +540,11 @@ public void visit(GeneralForStatement generalForStatement) { semicolon(); space(); - generalForStatement.localVariableExpr().accept(this); - space(); - buffer.append(LEFT_ANGLE); - space(); - generalForStatement.maxSizeExpr().accept(this); + generalForStatement.terminationExpr().accept(this); semicolon(); space(); - generalForStatement.localVariableExpr().accept(this); - // TODO(summerji): Remove the following temporary workaround. - buffer.append("++"); + generalForStatement.incrementExpr().accept(this); rightParen(); space(); leftBrace(); diff --git a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java index 588e03517b..5d15f481e0 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -354,10 +354,16 @@ private static MethodDefinition createSplitResponseMethod( // TODO(miraleung): Increment batchMessageIndexVarExpr. VariableExpr forIndexVarExpr = - VariableExpr.withVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()); + VariableExpr.builder() + .setIsDecl(true) + .setVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()) + .build(); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); GeneralForStatement innerSubresponseForStatement = GeneralForStatement.incrementWith( forIndexVarExpr, + initValueExpr, subresponseCountVarExpr, innerSubresponseForExprs.stream() .map(e -> ExprStatement.withExpr(e)) diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 4d544d91aa..558121020c 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -21,18 +21,20 @@ import org.junit.Test; public class GeneralForStatementTest { - + /** ================================== incrementWith ========================================= */ @Test public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(true).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement.incrementWith( - variableExpr, maxSizeExpr, Arrays.asList(createBodyStatement())); + variableExpr, initValue, maxSizeExpr, Arrays.asList(createBodyStatement())); } @Test @@ -40,12 +42,14 @@ public void validGeneralForStatement_basicIsNotDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(false).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement.incrementWith( - variableExpr, maxSizeExpr, Arrays.asList(createBodyStatement())); + variableExpr, initValue, maxSizeExpr, Arrays.asList(createBodyStatement())); } @Test @@ -53,25 +57,190 @@ public void validGeneralForStatement_emptyBody() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(false).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); - GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); } @Test - public void invalidForStatement() { - Variable variable = Variable.builder().setName("str").setType(TypeNode.STRING).build(); + public void invalidForStatement_privateVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setScope(ScopeNode.PRIVATE).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + + @Test + public void invalidForStatement_staticAndFinalVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsFinal(true).setIsStatic(true).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + + /** ============================== Set Three Expressions ================================== */ + @Test + public void validGeneralForState_buildExprs() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + @Test + public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + MethodInvocationExpr incrementExpr = + MethodInvocationExpr.builder() + .setMethodName("doNothing") + .setReturnType(TypeNode.INT) + .build(); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_incrementExprIsAssignmentOperationExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + AssignmentOperationExpr incrementExpr = + AssignmentOperationExpr.xorAssignmentWithExprs( + variableExpr, + ValueExpr.withValue( + PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_incrementExprIsAssignmentExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + AssignmentExpr incrementExpr = + AssignmentExpr.builder() + .setVariableExpr(variableExpr) + .setValueExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())) + .build(); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void invalidGeneralForState_buildTerminalExprNotBooleanType() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + AssignmentOperationExpr terminationExpr = + AssignmentOperationExpr.multiplyAssignmentWithExprs(variableExpr, maxValueExpr); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build()); + } + + @Test + public void invalidGeneralForState_buildIncrementExprIsNotExprStatement() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + RelationalOperationExpr incrementExpr = + RelationalOperationExpr.equalToWithExprs(variableExpr, maxValueExpr); assertThrows( IllegalStateException.class, () -> - GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList())); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build()); } private static Statement createBodyStatement() { diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index bfa090c228..03e07b7c2c 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -1363,16 +1363,19 @@ public void writeForStatement() { } @Test - public void writeGeneralForStatement_basic() { + public void writeGeneralForStatement_basicIsDecl() { AssignmentExpr assignExpr = createAssignmentExpr("x", "3", TypeNode.INT); Statement assignExprStatement = ExprStatement.withExpr(assignExpr); List body = Arrays.asList(assignExprStatement, assignExprStatement); VariableExpr localVarExpr = createVariableDeclExpr("i", TypeNode.INT); - Expr maxSizeExpr = MethodInvocationExpr.builder().setMethodName("maxSize").build(); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + Expr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement forStatement = - GeneralForStatement.incrementWith(localVarExpr, maxSizeExpr, body); + GeneralForStatement.incrementWith(localVarExpr, initValueExpr, maxSizeExpr, body); forStatement.accept(writerVisitor); assertEquals( @@ -1382,6 +1385,28 @@ public void writeGeneralForStatement_basic() { "for (int i = 0; i < maxSize(); i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); } + @Test + public void writeGeneralForStatement_basicIsNotDecl() { + AssignmentExpr assignExpr = createAssignmentExpr("x", "3", TypeNode.INT); + Statement assignExprStatement = ExprStatement.withExpr(assignExpr); + List body = Arrays.asList(assignExprStatement, assignExprStatement); + + VariableExpr localVarExpr = createVariableExpr("i", TypeNode.INT); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("1").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + + GeneralForStatement forStatement = + GeneralForStatement.incrementWith(localVarExpr, initValueExpr, maxValueExpr, body); + + forStatement.accept(writerVisitor); + assertEquals( + writerVisitor.write(), + String.format( + "%s%s%s%s", "for (i = 1; i < 10; i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + } + @Test public void writeTryCatchStatement_simple() { Reference exceptionReference = ConcreteReference.withClazz(IllegalArgumentException.class); From 5ed201c96e81169f13fa05a0cc8064d2b5bd9f53 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:39:53 -0700 Subject: [PATCH 02/32] make exprs optional --- .../engine/ast/GeneralForStatement.java | 53 +++++++++++-------- .../engine/ast/GeneralForStatementTest.java | 39 ++++++++++++++ 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index a84e43efa9..b9ba52a8b1 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -82,31 +82,38 @@ public abstract static class Builder { // Type-checking will be done in the sub-expressions. public GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); - VariableExpr varExpr = generalForStatement.initializationExpr().variableExpr(); + AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); Expr terminationExpr = generalForStatement.terminationExpr(); Expr incrementExpr = generalForStatement.incrementExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be expression statement."); + if (initializationExpr != null) { + VariableExpr varExpr = initializationExpr.variableExpr(); + Preconditions.checkState( + varExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s in a general for-loop cannot have a non-local scope", + varExpr.variable().identifier().name())); + Preconditions.checkState( + !varExpr.isStatic() && !varExpr.isFinal(), + String.format( + "Variable %s in a general for-loop cannot be static or final", + varExpr.variable().identifier().name())); + } + if (terminationExpr != null) { + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + } + if (incrementExpr != null) { + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be expression statement."); + } return autoBuild(); } } diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 558121020c..b3a509454f 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -122,6 +122,45 @@ public void validGeneralForState_buildExprs() { .build(); } + @Test + public void validGeneralForState_buildWithoutExprs() { + // Form in `for (;;;)` + GeneralForStatement.builder().build(); + } + + @Test + public void validGeneralForState_buildWithoutTerminalExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_withoutIncrementExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .build(); + } + @Test public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { VariableExpr variableExpr = From 91741f3f1051b06e03460d6f7851ab9be42ad4c8 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:42:42 -0700 Subject: [PATCH 03/32] format tests --- .../api/generator/engine/ast/GeneralForStatementTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index b3a509454f..fded3b8b67 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -21,7 +21,7 @@ import org.junit.Test; public class GeneralForStatementTest { - /** ================================== incrementWith ========================================= */ + /** ============================== incrementWith ====================================== */ @Test public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); @@ -101,7 +101,7 @@ public void invalidForStatement_staticAndFinalVariable() { variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } - /** ============================== Set Three Expressions ================================== */ + /** ============================ Set Three Expressions ================================ */ @Test public void validGeneralForState_buildExprs() { VariableExpr variableExpr = From 17c99596c3d5e2b3b0e90619bc19af3a45f900b6 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 14:26:01 -0700 Subject: [PATCH 04/32] Add comments --- .../api/generator/engine/ast/GeneralForStatement.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index b9ba52a8b1..2bf8411748 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -69,12 +69,13 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { + // Private setter. abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); - + // Private setter. abstract Builder setTerminationExpr(Expr terminationExpr); - + // Private setter. abstract Builder setIncrementExpr(Expr incrementExpr); - + // Private setter. abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); From ade35c7871f0aaf388407168749424a47614eb7c Mon Sep 17 00:00:00 2001 From: summerji Date: Thu, 1 Oct 2020 11:28:03 -0700 Subject: [PATCH 05/32] remove null case for expressions --- .../engine/ast/GeneralForStatement.java | 56 ++++++++----------- .../engine/ast/GeneralForStatementTest.java | 39 ------------- 2 files changed, 23 insertions(+), 72 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index 2bf8411748..bbb28011e4 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -19,17 +19,13 @@ import com.google.common.collect.ImmutableList; import java.util.Collections; import java.util.List; -import javax.annotation.Nullable; @AutoValue public abstract class GeneralForStatement implements Statement { - @Nullable public abstract AssignmentExpr initializationExpr(); - @Nullable public abstract Expr terminationExpr(); - @Nullable public abstract Expr incrementExpr(); public abstract ImmutableList body(); @@ -86,35 +82,29 @@ public GeneralForStatement build() { AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); Expr terminationExpr = generalForStatement.terminationExpr(); Expr incrementExpr = generalForStatement.incrementExpr(); - if (initializationExpr != null) { - VariableExpr varExpr = initializationExpr.variableExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - } - if (terminationExpr != null) { - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - } - if (incrementExpr != null) { - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be expression statement."); - } + VariableExpr varExpr = initializationExpr.variableExpr(); + Preconditions.checkState( + varExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s in a general for-loop cannot have a non-local scope", + varExpr.variable().identifier().name())); + Preconditions.checkState( + !varExpr.isStatic() && !varExpr.isFinal(), + String.format( + "Variable %s in a general for-loop cannot be static or final", + varExpr.variable().identifier().name())); + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); return autoBuild(); } } diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index fded3b8b67..30ad93c983 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -122,45 +122,6 @@ public void validGeneralForState_buildExprs() { .build(); } - @Test - public void validGeneralForState_buildWithoutExprs() { - // Form in `for (;;;)` - GeneralForStatement.builder().build(); - } - - @Test - public void validGeneralForState_buildWithoutTerminalExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - - @Test - public void validGeneralForState_withoutIncrementExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .build(); - } - @Test public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { VariableExpr variableExpr = From c5f32ee18542601990cda1d88b2e1a0b6c9e6351 Mon Sep 17 00:00:00 2001 From: summerji Date: Fri, 2 Oct 2020 02:58:05 -0700 Subject: [PATCH 06/32] fix comments --- .../engine/ast/GeneralForStatement.java | 22 +++++++++++++++---- .../engine/ast/GeneralForStatementTest.java | 5 ----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index bbb28011e4..f02539c8f4 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -14,6 +14,7 @@ package com.google.api.generator.engine.ast; +import autovalue.shaded.com.google$.common.annotations.$VisibleForTesting; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -22,6 +23,14 @@ @AutoValue public abstract class GeneralForStatement implements Statement { + private int i = 0; + for (i = 0; i < 10; i++) {} + + void foobar() { + for (i = 0; i < 10; i++) { + System.out.println("i = " + i); + } + } public abstract AssignmentExpr initializationExpr(); public abstract Expr terminationExpr(); @@ -37,7 +46,7 @@ public void accept(AstNodeVisitor visitor) { // incrementWith is convenience wrapper to generate index-base for-loop with lower and upper bound // and post increment on variable, like code in ```for (int i = 0; i < getMax(); i++) {..}``` - // TODO (developer): Add more convenience wrapper for the future generation needs. + // TODO (unsupported): Add more convenience wrapper for the future generation needs. public static GeneralForStatement incrementWith( VariableExpr localVariableExpr, ValueExpr initialValueExpr, @@ -64,19 +73,24 @@ public static Builder builder() { } @AutoValue.Builder - public abstract static class Builder { + abstract static class Builder { // Private setter. + @$VisibleForTesting abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); // Private setter. + @$VisibleForTesting abstract Builder setTerminationExpr(Expr terminationExpr); // Private setter. + @$VisibleForTesting abstract Builder setIncrementExpr(Expr incrementExpr); // Private setter. + @$VisibleForTesting abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. + @$VisibleForTesting public GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); @@ -100,8 +114,8 @@ public GeneralForStatement build() { (incrementExpr instanceof MethodInvocationExpr) || (incrementExpr instanceof AssignmentExpr) || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. + // TODO(unsupported): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needed. || (incrementExpr instanceof UnaryOperationExpr && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 30ad93c983..dec7e897d9 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -139,11 +139,6 @@ public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { .setMethodName("doNothing") .setReturnType(TypeNode.INT) .build(); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); } @Test From 1e75624499a97747c8cd8918db8e3d3776c52f3a Mon Sep 17 00:00:00 2001 From: summerji Date: Sat, 3 Oct 2020 23:57:58 -0700 Subject: [PATCH 07/32] Update second version design --- .../engine/ast/GeneralForStatement.java | 92 ++++---- .../engine/writer/ImportWriterVisitor.java | 2 +- .../engine/writer/JavaWriterVisitor.java | 2 +- .../engine/ast/GeneralForStatementTest.java | 196 ++++++------------ 4 files changed, 116 insertions(+), 176 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index f02539c8f4..6683804608 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -14,7 +14,6 @@ package com.google.api.generator.engine.ast; -import autovalue.shaded.com.google$.common.annotations.$VisibleForTesting; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -23,19 +22,11 @@ @AutoValue public abstract class GeneralForStatement implements Statement { - private int i = 0; - for (i = 0; i < 10; i++) {} - - void foobar() { - for (i = 0; i < 10; i++) { - System.out.println("i = " + i); - } - } public abstract AssignmentExpr initializationExpr(); public abstract Expr terminationExpr(); - public abstract Expr incrementExpr(); + public abstract Expr updateExpr(); public abstract ImmutableList body(); @@ -61,64 +52,75 @@ public static GeneralForStatement incrementWith( .setTerminationExpr( RelationalOperationExpr.lessThanWithExprs( localVariableExpr.toBuilder().setIsDecl(false).build(), maxSizeExpr)) - .setIncrementExpr( + .setUpdateExpr( UnaryOperationExpr.postfixIncrementWithExpr( localVariableExpr.toBuilder().setIsDecl(false).build())) .setBody(body) .build(); } - public static Builder builder() { + private static Builder builder() { return new AutoValue_GeneralForStatement.Builder().setBody(Collections.emptyList()); } @AutoValue.Builder abstract static class Builder { // Private setter. - @$VisibleForTesting abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); // Private setter. - @$VisibleForTesting abstract Builder setTerminationExpr(Expr terminationExpr); // Private setter. - @$VisibleForTesting - abstract Builder setIncrementExpr(Expr incrementExpr); + abstract Builder setUpdateExpr(Expr incrementExpr); // Private setter. - @$VisibleForTesting abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. - @$VisibleForTesting - public GeneralForStatement build() { + GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); - AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); - Expr terminationExpr = generalForStatement.terminationExpr(); - Expr incrementExpr = generalForStatement.incrementExpr(); - VariableExpr varExpr = initializationExpr.variableExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(unsupported): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needed. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); + VariableExpr localVarExpr = generalForStatement.initializationExpr().variableExpr(); + // Declare a variable inside for-loop initialization expression. + if (localVarExpr.isDecl()) { + Preconditions.checkState( + localVarExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s declare in a general for-loop cannot have a non-local scope", + localVarExpr.variable().identifier().name())); + Preconditions.checkState(!localVarExpr.isStatic(), "Modifier 'static' not allow here."); + } + // Assign a variable in for-loop initialization expression. + if (!localVarExpr.isDecl() && !localVarExpr.scope().equals(ScopeNode.LOCAL)) { + Preconditions.checkState( + !localVarExpr.isFinal(), + "Cannot assign a value to final variable %s.", + localVarExpr.variable().identifier().name()); + } + // TODO (unsupport): Uncomment the following code if public setter for the initialization, + // termination, update expressions when needed. + // Preconditions.checkState( + // (initializationExpr instanceof MethodInvocationExpr) + // || (incrementExpr instanceof AssignmentExpr) + // || (incrementExpr instanceof AssignmentOperationExpr) + // // TODO(unsupported): Currently we only support postIncrement (i++), please add + // // postDecrement, prefixIncrement, prefixIncrement if needed. + // || (incrementExpr instanceof UnaryOperationExpr + // && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + // "Initialization expression %s must be either a method invocation, assignment, or unary + // post-fix operation expression."); + // Preconditions.checkState( + // terminationExpr.type().equals(TypeNode.BOOLEAN), + // "Terminal expression %s must be boolean-type expression."); + // Preconditions.checkState( + // (incrementExpr instanceof MethodInvocationExpr) + // || (incrementExpr instanceof AssignmentExpr) + // || (incrementExpr instanceof AssignmentOperationExpr) + // // TODO(unsupported): Currently we only support postIncrement (i++), please add + // // postDecrement, prefixIncrement, prefixIncrement if needed. + // || (incrementExpr instanceof UnaryOperationExpr + // && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + // "Increment expression %s must be either a method invocation, assignment, or unary + // post-fix operation expression."); return autoBuild(); } } diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index 98fb5b5b51..03e70240c9 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -282,7 +282,7 @@ public void visit(ForStatement forStatement) { public void visit(GeneralForStatement generalForStatement) { generalForStatement.initializationExpr().accept(this); generalForStatement.terminationExpr().accept(this); - generalForStatement.incrementExpr().accept(this); + generalForStatement.updateExpr().accept(this); statements(generalForStatement.body()); } diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 2d970a0a96..1945b12366 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -544,7 +544,7 @@ public void visit(GeneralForStatement generalForStatement) { semicolon(); space(); - generalForStatement.incrementExpr().accept(this); + generalForStatement.updateExpr().accept(this); rightParen(); space(); leftBrace(); diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index dec7e897d9..ecc39890e0 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -23,6 +23,7 @@ public class GeneralForStatementTest { /** ============================== incrementWith ====================================== */ @Test + // validGeneralForStatement_basicIsDecl tests declare a variable inside in initialization expr. public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -38,6 +39,8 @@ public void validGeneralForStatement_basicIsDecl() { } @Test + // validGeneralForStatement_basicIsNotDecl tests assigning a method-level local variable in + // initialization expr. public void validGeneralForStatement_basicIsNotDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -53,6 +56,7 @@ public void validGeneralForStatement_basicIsNotDecl() { } @Test + // validGeneralForStatement_emptyBody tests set empty body in update expr. public void validGeneralForStatement_emptyBody() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -68,27 +72,55 @@ public void validGeneralForStatement_emptyBody() { } @Test - public void invalidForStatement_privateVariable() { + // validForStatement_privateNotIsDeclVariable tests assigning an instance variable with private + // scope. + public void validForStatement_privateNotIsDeclVariable() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.builder().setVariable(variable).setScope(ScopeNode.PRIVATE).build(); + VariableExpr.builder() + .setIsDecl(false) + .setVariable(variable) + .setScope(ScopeNode.PRIVATE) + .build(); ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); + } - assertThrows( - IllegalStateException.class, - () -> - GeneralForStatement.incrementWith( - variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + @Test + // validForStatement_instanceStaticVariable tests assigning an instance variable with public scope + // and static modifier. + public void validForStatement_instanceStaticVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder() + .setIsDecl(false) + .setVariable(variable) + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); } @Test - public void invalidForStatement_staticAndFinalVariable() { + // invalidForStatement_privateIsDeclVariable tests declaring a non-local variable inside of + // for-loop. + public void invalidForStatement_privateIsDeclVariable() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.builder().setVariable(variable).setIsFinal(true).setIsStatic(true).build(); + VariableExpr.builder() + .setIsDecl(true) + .setVariable(variable) + .setScope(ScopeNode.PRIVATE) + .build(); ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = @@ -101,141 +133,47 @@ public void invalidForStatement_staticAndFinalVariable() { variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } - /** ============================ Set Three Expressions ================================ */ - @Test - public void validGeneralForState_buildExprs() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - @Test - public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - MethodInvocationExpr incrementExpr = - MethodInvocationExpr.builder() - .setMethodName("doNothing") - .setReturnType(TypeNode.INT) - .build(); - } - - @Test - public void validGeneralForState_incrementExprIsAssignmentOperationExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - AssignmentOperationExpr incrementExpr = - AssignmentOperationExpr.xorAssignmentWithExprs( - variableExpr, - ValueExpr.withValue( - PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - - @Test - public void validGeneralForState_incrementExprIsAssignmentExpr() { + // invalidForStatement_staticIsDeclVariable tests declare a static local variable in + // initialization expr. + public void invalidForStatement_staticIsDeclVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(true).setIsStatic(true).build(); + ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - AssignmentExpr incrementExpr = - AssignmentExpr.builder() - .setVariableExpr(variableExpr) - .setValueExpr( - ValueExpr.withValue( - PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())) - .build(); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); - @Test - public void invalidGeneralForState_buildTerminalExprNotBooleanType() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - AssignmentOperationExpr terminationExpr = - AssignmentOperationExpr.multiplyAssignmentWithExprs(variableExpr, maxValueExpr); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); assertThrows( IllegalStateException.class, () -> - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } @Test - public void invalidGeneralForState_buildIncrementExprIsNotExprStatement() { + // invalidForStatement_finalIsNotDeclVariable tests invalid case of assigning the initial value to + // a variable which is declared as a final instance variable. + public void invalidForStatement_finalIsNotDeclVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = + VariableExpr.builder() + .setVariable(variable) + .setIsDecl(false) + .setScope(ScopeNode.PUBLIC) + .setIsFinal(true) + .build(); + ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - RelationalOperationExpr incrementExpr = - RelationalOperationExpr.equalToWithExprs(variableExpr, maxValueExpr); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + assertThrows( IllegalStateException.class, () -> - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } private static Statement createBodyStatement() { From 30b4ff49ed7239d5dcd8808d1d49200e6079c35b Mon Sep 17 00:00:00 2001 From: summerji Date: Sun, 4 Oct 2020 00:29:26 -0700 Subject: [PATCH 08/32] fix: add final keyword checking in post increment operation expr --- .../engine/ast/UnaryOperationExpr.java | 12 +++++++++ .../engine/ast/UnaryOperationExprTest.java | 25 +++++++++++++------ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/UnaryOperationExpr.java b/src/main/java/com/google/api/generator/engine/ast/UnaryOperationExpr.java index f7d65af381..7c38beff02 100644 --- a/src/main/java/com/google/api/generator/engine/ast/UnaryOperationExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/UnaryOperationExpr.java @@ -72,6 +72,18 @@ private UnaryOperationExpr build() { UnaryOperationExpr unaryOperationExpr = autoBuild(); TypeNode exprType = unaryOperationExpr.expr().type(); OperatorKind operator = unaryOperationExpr.operatorKind(); + + // TODO: (summerji) Add Decl Check for variable. + // Add final keyword checking for post/prefix ++, -- when needed. + if (operator.equals(OperatorKind.UNARY_POST_INCREMENT) + && unaryOperationExpr.expr() instanceof VariableExpr) { + Preconditions.checkState( + !((VariableExpr) unaryOperationExpr.expr()).isFinal(), + String.format( + "Cannot assign a value to final variable '%s'.", + ((VariableExpr) unaryOperationExpr.expr()).variable().name())); + } + final String errorMsg = String.format( "Unary operator %s can not be applied to %s. ", operator, exprType.toString()); diff --git a/src/test/java/com/google/api/generator/engine/ast/UnaryOperationExprTest.java b/src/test/java/com/google/api/generator/engine/ast/UnaryOperationExprTest.java index c2321d1577..734156cf9c 100644 --- a/src/test/java/com/google/api/generator/engine/ast/UnaryOperationExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/UnaryOperationExprTest.java @@ -21,7 +21,7 @@ public class UnaryOperationExprTest { /** =============================== Logic Not Operation Expr =============================== */ @Test - public void logicalNotOperationExpr_validBasic() { + public void validLogicalNotOperationExpr_basic() { VariableExpr variableExpr = VariableExpr.withVariable( Variable.builder().setName("x").setType(TypeNode.BOOLEAN).build()); @@ -30,7 +30,7 @@ public void logicalNotOperationExpr_validBasic() { } @Test - public void logicalNot_validBoxedType() { + public void validLogicalNot_boxedType() { VariableExpr variableExpr = VariableExpr.withVariable( Variable.builder().setName("x").setType(TypeNode.BOOLEAN_OBJECT).build()); @@ -39,7 +39,7 @@ public void logicalNot_validBoxedType() { } @Test - public void logicalNot_invalidNumericType() { + public void invalidLogicalNot_numericType() { VariableExpr variableExpr = VariableExpr.withVariable(Variable.builder().setName("x").setType(TypeNode.INT).build()); assertThrows( @@ -47,7 +47,7 @@ public void logicalNot_invalidNumericType() { } @Test - public void logicalNot_invalidReferenceType() { + public void invalidLogicalNot_referenceType() { VariableExpr variableExpr = VariableExpr.withVariable(Variable.builder().setName("x").setType(TypeNode.STRING).build()); assertThrows( @@ -58,7 +58,7 @@ public void logicalNot_invalidReferenceType() { * =============================== Post Increment Operation Expr =============================== */ @Test - public void postIncrement_validBasic() { + public void validPostIncrement_basic() { VariableExpr variableExpr = VariableExpr.withVariable(Variable.builder().setName("x").setType(TypeNode.INT).build()); UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); @@ -66,7 +66,7 @@ public void postIncrement_validBasic() { } @Test - public void postIncrement_validBoxedType() { + public void validPostIncrement_boxedType() { VariableExpr variableExpr = VariableExpr.withVariable( Variable.builder().setName("x").setType(TypeNode.FLOAT_OBJECT).build()); @@ -75,7 +75,7 @@ public void postIncrement_validBoxedType() { } @Test - public void postIncrement_invalidBoxedBooleanType() { + public void invalidPostIncrement_boxedBooleanType() { VariableExpr variableExpr = VariableExpr.withVariable( Variable.builder().setName("x").setType(TypeNode.BOOLEAN_OBJECT).build()); @@ -85,11 +85,20 @@ public void postIncrement_invalidBoxedBooleanType() { } @Test - public void postIncrement_invalidReferenceType() { + public void invalidPostIncrement_referenceType() { VariableExpr variableExpr = VariableExpr.withVariable(Variable.builder().setName("x").setType(TypeNode.STRING).build()); assertThrows( IllegalStateException.class, () -> UnaryOperationExpr.postfixIncrementWithExpr(variableExpr)); } + + @Test + public void invalidPostIncrement_finalVariable() { + Variable var = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = VariableExpr.builder().setIsFinal(true).setVariable(var).build(); + assertThrows( + IllegalStateException.class, + () -> UnaryOperationExpr.postfixIncrementWithExpr(variableExpr)); + } } From 5a5ec4e480d42ed721d0485febe9b98582b220e2 Mon Sep 17 00:00:00 2001 From: summerji Date: Sun, 4 Oct 2020 00:53:26 -0700 Subject: [PATCH 09/32] fix: add final keyword check on assignment expr --- .../generator/engine/ast/AssignmentExpr.java | 10 +++++++ .../engine/ast/AssignmentExprTest.java | 26 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java b/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java index 0e32d58c42..9223e41783 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java @@ -15,6 +15,7 @@ package com.google.api.generator.engine.ast; import com.google.auto.value.AutoValue; +import com.google.common.base.Preconditions; @AutoValue public abstract class AssignmentExpr implements Expr { @@ -67,6 +68,15 @@ public AssignmentExpr build() { lhsType.reference().name(), rhsType.reference().name())); } } + + if (!assignmentExpr.variableExpr().isDecl()) { + Preconditions.checkState( + !assignmentExpr.variableExpr().isFinal(), + String.format( + "Cannot assign a value to final variable '%s'.", + assignmentExpr.variableExpr().variable().name())); + } + return assignmentExpr; } } diff --git a/src/test/java/com/google/api/generator/engine/ast/AssignmentExprTest.java b/src/test/java/com/google/api/generator/engine/ast/AssignmentExprTest.java index 56f91f7fec..b40590f231 100644 --- a/src/test/java/com/google/api/generator/engine/ast/AssignmentExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/AssignmentExprTest.java @@ -137,6 +137,32 @@ public void writeAssignmentExpr_invalidBoxedPrimitiveType() { assertInvalidAssignmentExpr(lVariableExpr, rVariableExpr); } + @Test + public void writeAssignmentExpr_validIsDeclFinalVariableExpr() { + Variable lVariable = Variable.builder().setName("x").setType(TypeNode.INT).build(); + VariableExpr lVariableExpr = + VariableExpr.builder().setVariable(lVariable).setIsDecl(true).setIsFinal(true).build(); + + ValueExpr valueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build()); + assertValidAssignmentExpr(lVariableExpr, valueExpr); + } + + @Test + public void writeAssignmentExpr_invalidIsNotDeclFinalVariableExpr() { + Variable lVariable = Variable.builder().setName("x").setType(TypeNode.INT).build(); + VariableExpr lVariableExpr = + VariableExpr.builder().setVariable(lVariable).setIsDecl(false).setIsFinal(true).build(); + + ValueExpr valueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build()); + assertThrows( + IllegalStateException.class, + () -> { + AssignmentExpr.builder().setVariableExpr(lVariableExpr).setValueExpr(valueExpr).build(); + }); + } + private static void assertInvalidAssignmentExpr(VariableExpr variableExpr, Expr valueExpr) { assertThrows( TypeMismatchException.class, From 741edcaf976ce853fc06a7a62157b5b4afb2b9ec Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:22:04 -0700 Subject: [PATCH 10/32] Integrate OperationExpr into GeneralForStatement --- .../engine/ast/GeneralForStatement.java | 78 +++---- .../engine/writer/ImportWriterVisitor.java | 4 +- .../engine/writer/JavaWriterVisitor.java | 10 +- .../composer/BatchingDescriptorComposer.java | 8 +- .../engine/ast/GeneralForStatementTest.java | 191 +++++++++++++++++- .../engine/writer/JavaWriterVisitorTest.java | 31 ++- 6 files changed, 263 insertions(+), 59 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index fba06d97d1..a84e43efa9 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -19,22 +19,18 @@ import com.google.common.collect.ImmutableList; import java.util.Collections; import java.util.List; +import javax.annotation.Nullable; @AutoValue public abstract class GeneralForStatement implements Statement { - public abstract Expr initializationExpr(); + @Nullable + public abstract AssignmentExpr initializationExpr(); - // TODO(summerji): Integrate OperationExpr here. Start by uncommenting the following section to - // replace the localVariableExpr and maxSizeExpr getters after it. - /* - // Uses the same terminology as https://docs.oracle.com/javase/tutorial/java/nutsandbolts/for.html. + @Nullable public abstract Expr terminationExpr(); - public abstract Expr incrementExpr(); - */ - - public abstract VariableExpr localVariableExpr(); - public abstract Expr maxSizeExpr(); + @Nullable + public abstract Expr incrementExpr(); public abstract ImmutableList body(); @@ -43,14 +39,26 @@ public void accept(AstNodeVisitor visitor) { visitor.visit(this); } - // Convenience wrapper. + // incrementWith is convenience wrapper to generate index-base for-loop with lower and upper bound + // and post increment on variable, like code in ```for (int i = 0; i < getMax(); i++) {..}``` + // TODO (developer): Add more convenience wrapper for the future generation needs. public static GeneralForStatement incrementWith( - VariableExpr variableExpr, Expr maxSizeExpr, List body) { - // TODO(summerji): Do some integration here, in JavaWriterVisitor, in ImportWriterVisitor, and - // add more tests. + VariableExpr localVariableExpr, + ValueExpr initialValueExpr, + Expr maxSizeExpr, + List body) { return builder() - .setLocalVariableExpr(variableExpr.toBuilder().setIsDecl(false).build()) - .setMaxSizeExpr(maxSizeExpr) + .setInitializationExpr( + AssignmentExpr.builder() + .setVariableExpr(localVariableExpr) + .setValueExpr(initialValueExpr) + .build()) + .setTerminationExpr( + RelationalOperationExpr.lessThanWithExprs( + localVariableExpr.toBuilder().setIsDecl(false).build(), maxSizeExpr)) + .setIncrementExpr( + UnaryOperationExpr.postfixIncrementWithExpr( + localVariableExpr.toBuilder().setIsDecl(false).build())) .setBody(body) .build(); } @@ -61,22 +69,22 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { - public abstract Builder setInitializationExpr(Expr initializationExpr); + abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); - public abstract Builder setBody(List body); + abstract Builder setTerminationExpr(Expr terminationExpr); - // Private. - abstract Builder setLocalVariableExpr(VariableExpr variableExpr); + abstract Builder setIncrementExpr(Expr incrementExpr); - abstract Builder setMaxSizeExpr(Expr maxSizeExpr); - - abstract VariableExpr localVariableExpr(); + abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. public GeneralForStatement build() { - VariableExpr varExpr = localVariableExpr(); + GeneralForStatement generalForStatement = autoBuild(); + VariableExpr varExpr = generalForStatement.initializationExpr().variableExpr(); + Expr terminationExpr = generalForStatement.terminationExpr(); + Expr incrementExpr = generalForStatement.incrementExpr(); Preconditions.checkState( varExpr.scope().equals(ScopeNode.LOCAL), String.format( @@ -87,16 +95,18 @@ public GeneralForStatement build() { String.format( "Variable %s in a general for-loop cannot be static or final", varExpr.variable().identifier().name())); - setInitializationExpr( - AssignmentExpr.builder() - .setVariableExpr(varExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr( - ValueExpr.withValue( - PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build())) - .build()); - // TODO(summerji): Remove the following two lines. - // This temporary workaround will be removed soon, so it doesn't need a test. - setLocalVariableExpr(varExpr.toBuilder().setIsDecl(false).build()); + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be expression statement."); return autoBuild(); } } diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index fa2648a545..98fb5b5b51 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -281,8 +281,8 @@ public void visit(ForStatement forStatement) { @Override public void visit(GeneralForStatement generalForStatement) { generalForStatement.initializationExpr().accept(this); - generalForStatement.localVariableExpr().accept(this); - generalForStatement.maxSizeExpr().accept(this); + generalForStatement.terminationExpr().accept(this); + generalForStatement.incrementExpr().accept(this); statements(generalForStatement.body()); } diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 7a1e4d3b6d..2d970a0a96 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -540,17 +540,11 @@ public void visit(GeneralForStatement generalForStatement) { semicolon(); space(); - generalForStatement.localVariableExpr().accept(this); - space(); - buffer.append(LEFT_ANGLE); - space(); - generalForStatement.maxSizeExpr().accept(this); + generalForStatement.terminationExpr().accept(this); semicolon(); space(); - generalForStatement.localVariableExpr().accept(this); - // TODO(summerji): Remove the following temporary workaround. - buffer.append("++"); + generalForStatement.incrementExpr().accept(this); rightParen(); space(); leftBrace(); diff --git a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java index 588e03517b..5d15f481e0 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -354,10 +354,16 @@ private static MethodDefinition createSplitResponseMethod( // TODO(miraleung): Increment batchMessageIndexVarExpr. VariableExpr forIndexVarExpr = - VariableExpr.withVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()); + VariableExpr.builder() + .setIsDecl(true) + .setVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()) + .build(); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); GeneralForStatement innerSubresponseForStatement = GeneralForStatement.incrementWith( forIndexVarExpr, + initValueExpr, subresponseCountVarExpr, innerSubresponseForExprs.stream() .map(e -> ExprStatement.withExpr(e)) diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 4d544d91aa..558121020c 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -21,18 +21,20 @@ import org.junit.Test; public class GeneralForStatementTest { - + /** ================================== incrementWith ========================================= */ @Test public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(true).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement.incrementWith( - variableExpr, maxSizeExpr, Arrays.asList(createBodyStatement())); + variableExpr, initValue, maxSizeExpr, Arrays.asList(createBodyStatement())); } @Test @@ -40,12 +42,14 @@ public void validGeneralForStatement_basicIsNotDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(false).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement.incrementWith( - variableExpr, maxSizeExpr, Arrays.asList(createBodyStatement())); + variableExpr, initValue, maxSizeExpr, Arrays.asList(createBodyStatement())); } @Test @@ -53,25 +57,190 @@ public void validGeneralForStatement_emptyBody() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(false).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); - GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); } @Test - public void invalidForStatement() { - Variable variable = Variable.builder().setName("str").setType(TypeNode.STRING).build(); + public void invalidForStatement_privateVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setScope(ScopeNode.PRIVATE).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + + @Test + public void invalidForStatement_staticAndFinalVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsFinal(true).setIsStatic(true).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + + /** ============================== Set Three Expressions ================================== */ + @Test + public void validGeneralForState_buildExprs() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + @Test + public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + MethodInvocationExpr incrementExpr = + MethodInvocationExpr.builder() + .setMethodName("doNothing") + .setReturnType(TypeNode.INT) + .build(); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_incrementExprIsAssignmentOperationExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + AssignmentOperationExpr incrementExpr = + AssignmentOperationExpr.xorAssignmentWithExprs( + variableExpr, + ValueExpr.withValue( + PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_incrementExprIsAssignmentExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + AssignmentExpr incrementExpr = + AssignmentExpr.builder() + .setVariableExpr(variableExpr) + .setValueExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())) + .build(); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void invalidGeneralForState_buildTerminalExprNotBooleanType() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + AssignmentOperationExpr terminationExpr = + AssignmentOperationExpr.multiplyAssignmentWithExprs(variableExpr, maxValueExpr); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build()); + } + + @Test + public void invalidGeneralForState_buildIncrementExprIsNotExprStatement() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + RelationalOperationExpr incrementExpr = + RelationalOperationExpr.equalToWithExprs(variableExpr, maxValueExpr); assertThrows( IllegalStateException.class, () -> - GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList())); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build()); } private static Statement createBodyStatement() { diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index bfa090c228..03e07b7c2c 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -1363,16 +1363,19 @@ public void writeForStatement() { } @Test - public void writeGeneralForStatement_basic() { + public void writeGeneralForStatement_basicIsDecl() { AssignmentExpr assignExpr = createAssignmentExpr("x", "3", TypeNode.INT); Statement assignExprStatement = ExprStatement.withExpr(assignExpr); List body = Arrays.asList(assignExprStatement, assignExprStatement); VariableExpr localVarExpr = createVariableDeclExpr("i", TypeNode.INT); - Expr maxSizeExpr = MethodInvocationExpr.builder().setMethodName("maxSize").build(); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + Expr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement forStatement = - GeneralForStatement.incrementWith(localVarExpr, maxSizeExpr, body); + GeneralForStatement.incrementWith(localVarExpr, initValueExpr, maxSizeExpr, body); forStatement.accept(writerVisitor); assertEquals( @@ -1382,6 +1385,28 @@ public void writeGeneralForStatement_basic() { "for (int i = 0; i < maxSize(); i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); } + @Test + public void writeGeneralForStatement_basicIsNotDecl() { + AssignmentExpr assignExpr = createAssignmentExpr("x", "3", TypeNode.INT); + Statement assignExprStatement = ExprStatement.withExpr(assignExpr); + List body = Arrays.asList(assignExprStatement, assignExprStatement); + + VariableExpr localVarExpr = createVariableExpr("i", TypeNode.INT); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("1").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + + GeneralForStatement forStatement = + GeneralForStatement.incrementWith(localVarExpr, initValueExpr, maxValueExpr, body); + + forStatement.accept(writerVisitor); + assertEquals( + writerVisitor.write(), + String.format( + "%s%s%s%s", "for (i = 1; i < 10; i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + } + @Test public void writeTryCatchStatement_simple() { Reference exceptionReference = ConcreteReference.withClazz(IllegalArgumentException.class); From b6817203de0bf39386f174a166b85ce2bc70b2da Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:39:53 -0700 Subject: [PATCH 11/32] make exprs optional --- .../engine/ast/GeneralForStatement.java | 53 +++++++++++-------- .../engine/ast/GeneralForStatementTest.java | 39 ++++++++++++++ 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index a84e43efa9..b9ba52a8b1 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -82,31 +82,38 @@ public abstract static class Builder { // Type-checking will be done in the sub-expressions. public GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); - VariableExpr varExpr = generalForStatement.initializationExpr().variableExpr(); + AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); Expr terminationExpr = generalForStatement.terminationExpr(); Expr incrementExpr = generalForStatement.incrementExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be expression statement."); + if (initializationExpr != null) { + VariableExpr varExpr = initializationExpr.variableExpr(); + Preconditions.checkState( + varExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s in a general for-loop cannot have a non-local scope", + varExpr.variable().identifier().name())); + Preconditions.checkState( + !varExpr.isStatic() && !varExpr.isFinal(), + String.format( + "Variable %s in a general for-loop cannot be static or final", + varExpr.variable().identifier().name())); + } + if (terminationExpr != null) { + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + } + if (incrementExpr != null) { + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be expression statement."); + } return autoBuild(); } } diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 558121020c..b3a509454f 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -122,6 +122,45 @@ public void validGeneralForState_buildExprs() { .build(); } + @Test + public void validGeneralForState_buildWithoutExprs() { + // Form in `for (;;;)` + GeneralForStatement.builder().build(); + } + + @Test + public void validGeneralForState_buildWithoutTerminalExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_withoutIncrementExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .build(); + } + @Test public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { VariableExpr variableExpr = From 5aeae698b9cf76184feb4d440e8f943217cb226f Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:42:42 -0700 Subject: [PATCH 12/32] format tests --- .../api/generator/engine/ast/GeneralForStatementTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index b3a509454f..fded3b8b67 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -21,7 +21,7 @@ import org.junit.Test; public class GeneralForStatementTest { - /** ================================== incrementWith ========================================= */ + /** ============================== incrementWith ====================================== */ @Test public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); @@ -101,7 +101,7 @@ public void invalidForStatement_staticAndFinalVariable() { variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } - /** ============================== Set Three Expressions ================================== */ + /** ============================ Set Three Expressions ================================ */ @Test public void validGeneralForState_buildExprs() { VariableExpr variableExpr = From c04297dbdcb29384a315de8c4c5d534ed74d1f8e Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 14:26:01 -0700 Subject: [PATCH 13/32] Add comments --- .../api/generator/engine/ast/GeneralForStatement.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index b9ba52a8b1..2bf8411748 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -69,12 +69,13 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { + // Private setter. abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); - + // Private setter. abstract Builder setTerminationExpr(Expr terminationExpr); - + // Private setter. abstract Builder setIncrementExpr(Expr incrementExpr); - + // Private setter. abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); From 9375d9bd10e5214c7912d6e2ad8e93eef005b41c Mon Sep 17 00:00:00 2001 From: summerji Date: Thu, 1 Oct 2020 11:28:03 -0700 Subject: [PATCH 14/32] remove null case for expressions --- .../engine/ast/GeneralForStatement.java | 56 ++++++++----------- .../engine/ast/GeneralForStatementTest.java | 39 ------------- 2 files changed, 23 insertions(+), 72 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index 2bf8411748..bbb28011e4 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -19,17 +19,13 @@ import com.google.common.collect.ImmutableList; import java.util.Collections; import java.util.List; -import javax.annotation.Nullable; @AutoValue public abstract class GeneralForStatement implements Statement { - @Nullable public abstract AssignmentExpr initializationExpr(); - @Nullable public abstract Expr terminationExpr(); - @Nullable public abstract Expr incrementExpr(); public abstract ImmutableList body(); @@ -86,35 +82,29 @@ public GeneralForStatement build() { AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); Expr terminationExpr = generalForStatement.terminationExpr(); Expr incrementExpr = generalForStatement.incrementExpr(); - if (initializationExpr != null) { - VariableExpr varExpr = initializationExpr.variableExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - } - if (terminationExpr != null) { - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - } - if (incrementExpr != null) { - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be expression statement."); - } + VariableExpr varExpr = initializationExpr.variableExpr(); + Preconditions.checkState( + varExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s in a general for-loop cannot have a non-local scope", + varExpr.variable().identifier().name())); + Preconditions.checkState( + !varExpr.isStatic() && !varExpr.isFinal(), + String.format( + "Variable %s in a general for-loop cannot be static or final", + varExpr.variable().identifier().name())); + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); return autoBuild(); } } diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index fded3b8b67..30ad93c983 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -122,45 +122,6 @@ public void validGeneralForState_buildExprs() { .build(); } - @Test - public void validGeneralForState_buildWithoutExprs() { - // Form in `for (;;;)` - GeneralForStatement.builder().build(); - } - - @Test - public void validGeneralForState_buildWithoutTerminalExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - - @Test - public void validGeneralForState_withoutIncrementExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .build(); - } - @Test public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { VariableExpr variableExpr = From dbfb6ed162e4ae7fbab3130e8d9fb781309186b5 Mon Sep 17 00:00:00 2001 From: summerji Date: Fri, 2 Oct 2020 02:58:05 -0700 Subject: [PATCH 15/32] fix comments --- .../engine/ast/GeneralForStatement.java | 22 +++++++++++++++---- .../engine/ast/GeneralForStatementTest.java | 5 ----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index bbb28011e4..f02539c8f4 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -14,6 +14,7 @@ package com.google.api.generator.engine.ast; +import autovalue.shaded.com.google$.common.annotations.$VisibleForTesting; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -22,6 +23,14 @@ @AutoValue public abstract class GeneralForStatement implements Statement { + private int i = 0; + for (i = 0; i < 10; i++) {} + + void foobar() { + for (i = 0; i < 10; i++) { + System.out.println("i = " + i); + } + } public abstract AssignmentExpr initializationExpr(); public abstract Expr terminationExpr(); @@ -37,7 +46,7 @@ public void accept(AstNodeVisitor visitor) { // incrementWith is convenience wrapper to generate index-base for-loop with lower and upper bound // and post increment on variable, like code in ```for (int i = 0; i < getMax(); i++) {..}``` - // TODO (developer): Add more convenience wrapper for the future generation needs. + // TODO (unsupported): Add more convenience wrapper for the future generation needs. public static GeneralForStatement incrementWith( VariableExpr localVariableExpr, ValueExpr initialValueExpr, @@ -64,19 +73,24 @@ public static Builder builder() { } @AutoValue.Builder - public abstract static class Builder { + abstract static class Builder { // Private setter. + @$VisibleForTesting abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); // Private setter. + @$VisibleForTesting abstract Builder setTerminationExpr(Expr terminationExpr); // Private setter. + @$VisibleForTesting abstract Builder setIncrementExpr(Expr incrementExpr); // Private setter. + @$VisibleForTesting abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. + @$VisibleForTesting public GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); @@ -100,8 +114,8 @@ public GeneralForStatement build() { (incrementExpr instanceof MethodInvocationExpr) || (incrementExpr instanceof AssignmentExpr) || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. + // TODO(unsupported): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needed. || (incrementExpr instanceof UnaryOperationExpr && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 30ad93c983..dec7e897d9 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -139,11 +139,6 @@ public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { .setMethodName("doNothing") .setReturnType(TypeNode.INT) .build(); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); } @Test From d89978279e9ba972262c00919482b1ceb1f2e409 Mon Sep 17 00:00:00 2001 From: summerji Date: Sat, 3 Oct 2020 23:57:58 -0700 Subject: [PATCH 16/32] Update second version design --- .../engine/ast/GeneralForStatement.java | 92 ++++---- .../engine/writer/ImportWriterVisitor.java | 2 +- .../engine/writer/JavaWriterVisitor.java | 2 +- .../engine/ast/GeneralForStatementTest.java | 196 ++++++------------ 4 files changed, 116 insertions(+), 176 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index f02539c8f4..6683804608 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -14,7 +14,6 @@ package com.google.api.generator.engine.ast; -import autovalue.shaded.com.google$.common.annotations.$VisibleForTesting; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -23,19 +22,11 @@ @AutoValue public abstract class GeneralForStatement implements Statement { - private int i = 0; - for (i = 0; i < 10; i++) {} - - void foobar() { - for (i = 0; i < 10; i++) { - System.out.println("i = " + i); - } - } public abstract AssignmentExpr initializationExpr(); public abstract Expr terminationExpr(); - public abstract Expr incrementExpr(); + public abstract Expr updateExpr(); public abstract ImmutableList body(); @@ -61,64 +52,75 @@ public static GeneralForStatement incrementWith( .setTerminationExpr( RelationalOperationExpr.lessThanWithExprs( localVariableExpr.toBuilder().setIsDecl(false).build(), maxSizeExpr)) - .setIncrementExpr( + .setUpdateExpr( UnaryOperationExpr.postfixIncrementWithExpr( localVariableExpr.toBuilder().setIsDecl(false).build())) .setBody(body) .build(); } - public static Builder builder() { + private static Builder builder() { return new AutoValue_GeneralForStatement.Builder().setBody(Collections.emptyList()); } @AutoValue.Builder abstract static class Builder { // Private setter. - @$VisibleForTesting abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); // Private setter. - @$VisibleForTesting abstract Builder setTerminationExpr(Expr terminationExpr); // Private setter. - @$VisibleForTesting - abstract Builder setIncrementExpr(Expr incrementExpr); + abstract Builder setUpdateExpr(Expr incrementExpr); // Private setter. - @$VisibleForTesting abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. - @$VisibleForTesting - public GeneralForStatement build() { + GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); - AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); - Expr terminationExpr = generalForStatement.terminationExpr(); - Expr incrementExpr = generalForStatement.incrementExpr(); - VariableExpr varExpr = initializationExpr.variableExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(unsupported): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needed. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); + VariableExpr localVarExpr = generalForStatement.initializationExpr().variableExpr(); + // Declare a variable inside for-loop initialization expression. + if (localVarExpr.isDecl()) { + Preconditions.checkState( + localVarExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s declare in a general for-loop cannot have a non-local scope", + localVarExpr.variable().identifier().name())); + Preconditions.checkState(!localVarExpr.isStatic(), "Modifier 'static' not allow here."); + } + // Assign a variable in for-loop initialization expression. + if (!localVarExpr.isDecl() && !localVarExpr.scope().equals(ScopeNode.LOCAL)) { + Preconditions.checkState( + !localVarExpr.isFinal(), + "Cannot assign a value to final variable %s.", + localVarExpr.variable().identifier().name()); + } + // TODO (unsupport): Uncomment the following code if public setter for the initialization, + // termination, update expressions when needed. + // Preconditions.checkState( + // (initializationExpr instanceof MethodInvocationExpr) + // || (incrementExpr instanceof AssignmentExpr) + // || (incrementExpr instanceof AssignmentOperationExpr) + // // TODO(unsupported): Currently we only support postIncrement (i++), please add + // // postDecrement, prefixIncrement, prefixIncrement if needed. + // || (incrementExpr instanceof UnaryOperationExpr + // && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + // "Initialization expression %s must be either a method invocation, assignment, or unary + // post-fix operation expression."); + // Preconditions.checkState( + // terminationExpr.type().equals(TypeNode.BOOLEAN), + // "Terminal expression %s must be boolean-type expression."); + // Preconditions.checkState( + // (incrementExpr instanceof MethodInvocationExpr) + // || (incrementExpr instanceof AssignmentExpr) + // || (incrementExpr instanceof AssignmentOperationExpr) + // // TODO(unsupported): Currently we only support postIncrement (i++), please add + // // postDecrement, prefixIncrement, prefixIncrement if needed. + // || (incrementExpr instanceof UnaryOperationExpr + // && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + // "Increment expression %s must be either a method invocation, assignment, or unary + // post-fix operation expression."); return autoBuild(); } } diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index 98fb5b5b51..03e70240c9 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -282,7 +282,7 @@ public void visit(ForStatement forStatement) { public void visit(GeneralForStatement generalForStatement) { generalForStatement.initializationExpr().accept(this); generalForStatement.terminationExpr().accept(this); - generalForStatement.incrementExpr().accept(this); + generalForStatement.updateExpr().accept(this); statements(generalForStatement.body()); } diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 2d970a0a96..1945b12366 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -544,7 +544,7 @@ public void visit(GeneralForStatement generalForStatement) { semicolon(); space(); - generalForStatement.incrementExpr().accept(this); + generalForStatement.updateExpr().accept(this); rightParen(); space(); leftBrace(); diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index dec7e897d9..ecc39890e0 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -23,6 +23,7 @@ public class GeneralForStatementTest { /** ============================== incrementWith ====================================== */ @Test + // validGeneralForStatement_basicIsDecl tests declare a variable inside in initialization expr. public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -38,6 +39,8 @@ public void validGeneralForStatement_basicIsDecl() { } @Test + // validGeneralForStatement_basicIsNotDecl tests assigning a method-level local variable in + // initialization expr. public void validGeneralForStatement_basicIsNotDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -53,6 +56,7 @@ public void validGeneralForStatement_basicIsNotDecl() { } @Test + // validGeneralForStatement_emptyBody tests set empty body in update expr. public void validGeneralForStatement_emptyBody() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -68,27 +72,55 @@ public void validGeneralForStatement_emptyBody() { } @Test - public void invalidForStatement_privateVariable() { + // validForStatement_privateNotIsDeclVariable tests assigning an instance variable with private + // scope. + public void validForStatement_privateNotIsDeclVariable() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.builder().setVariable(variable).setScope(ScopeNode.PRIVATE).build(); + VariableExpr.builder() + .setIsDecl(false) + .setVariable(variable) + .setScope(ScopeNode.PRIVATE) + .build(); ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); + } - assertThrows( - IllegalStateException.class, - () -> - GeneralForStatement.incrementWith( - variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + @Test + // validForStatement_instanceStaticVariable tests assigning an instance variable with public scope + // and static modifier. + public void validForStatement_instanceStaticVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder() + .setIsDecl(false) + .setVariable(variable) + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); } @Test - public void invalidForStatement_staticAndFinalVariable() { + // invalidForStatement_privateIsDeclVariable tests declaring a non-local variable inside of + // for-loop. + public void invalidForStatement_privateIsDeclVariable() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.builder().setVariable(variable).setIsFinal(true).setIsStatic(true).build(); + VariableExpr.builder() + .setIsDecl(true) + .setVariable(variable) + .setScope(ScopeNode.PRIVATE) + .build(); ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = @@ -101,141 +133,47 @@ public void invalidForStatement_staticAndFinalVariable() { variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } - /** ============================ Set Three Expressions ================================ */ - @Test - public void validGeneralForState_buildExprs() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - @Test - public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - MethodInvocationExpr incrementExpr = - MethodInvocationExpr.builder() - .setMethodName("doNothing") - .setReturnType(TypeNode.INT) - .build(); - } - - @Test - public void validGeneralForState_incrementExprIsAssignmentOperationExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - AssignmentOperationExpr incrementExpr = - AssignmentOperationExpr.xorAssignmentWithExprs( - variableExpr, - ValueExpr.withValue( - PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - - @Test - public void validGeneralForState_incrementExprIsAssignmentExpr() { + // invalidForStatement_staticIsDeclVariable tests declare a static local variable in + // initialization expr. + public void invalidForStatement_staticIsDeclVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(true).setIsStatic(true).build(); + ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - AssignmentExpr incrementExpr = - AssignmentExpr.builder() - .setVariableExpr(variableExpr) - .setValueExpr( - ValueExpr.withValue( - PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())) - .build(); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); - @Test - public void invalidGeneralForState_buildTerminalExprNotBooleanType() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - AssignmentOperationExpr terminationExpr = - AssignmentOperationExpr.multiplyAssignmentWithExprs(variableExpr, maxValueExpr); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); assertThrows( IllegalStateException.class, () -> - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } @Test - public void invalidGeneralForState_buildIncrementExprIsNotExprStatement() { + // invalidForStatement_finalIsNotDeclVariable tests invalid case of assigning the initial value to + // a variable which is declared as a final instance variable. + public void invalidForStatement_finalIsNotDeclVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = + VariableExpr.builder() + .setVariable(variable) + .setIsDecl(false) + .setScope(ScopeNode.PUBLIC) + .setIsFinal(true) + .build(); + ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - RelationalOperationExpr incrementExpr = - RelationalOperationExpr.equalToWithExprs(variableExpr, maxValueExpr); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + assertThrows( IllegalStateException.class, () -> - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } private static Statement createBodyStatement() { From 8d647cd5aa6d260c9e37e1f2a6bbdb9a694a636f Mon Sep 17 00:00:00 2001 From: summerji Date: Sun, 4 Oct 2020 01:11:17 -0700 Subject: [PATCH 17/32] rebase fix in assignment and increment expr --- .../engine/ast/GeneralForStatement.java | 7 ------- .../engine/ast/GeneralForStatementTest.java | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index 6683804608..b6622996a1 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -89,13 +89,6 @@ GeneralForStatement build() { localVarExpr.variable().identifier().name())); Preconditions.checkState(!localVarExpr.isStatic(), "Modifier 'static' not allow here."); } - // Assign a variable in for-loop initialization expression. - if (!localVarExpr.isDecl() && !localVarExpr.scope().equals(ScopeNode.LOCAL)) { - Preconditions.checkState( - !localVarExpr.isFinal(), - "Cannot assign a value to final variable %s.", - localVarExpr.variable().identifier().name()); - } // TODO (unsupport): Uncomment the following code if public setter for the initialization, // termination, update expressions when needed. // Preconditions.checkState( diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index ecc39890e0..4d14429c1a 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -176,6 +176,25 @@ public void invalidForStatement_finalIsNotDeclVariable() { variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } + @Test + // invalidForStatement_localFinalVariable tests declare a final variable in initialization expr, + // updateExpr should throw error. + public void invalidForStatement_localFinalVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(true).setIsFinal(true).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + private static Statement createBodyStatement() { Variable variable = Variable.builder().setName("x").setType(TypeNode.INT).build(); VariableExpr variableExpr = From c5b02b45aafc25303d20de54a5b83dad501a09b5 Mon Sep 17 00:00:00 2001 From: summerji Date: Sun, 4 Oct 2020 00:29:26 -0700 Subject: [PATCH 18/32] fix: add final keyword checking in post increment operation expr --- .../engine/ast/UnaryOperationExpr.java | 12 +++++++++ .../engine/ast/UnaryOperationExprTest.java | 25 +++++++++++++------ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/UnaryOperationExpr.java b/src/main/java/com/google/api/generator/engine/ast/UnaryOperationExpr.java index f7d65af381..7c38beff02 100644 --- a/src/main/java/com/google/api/generator/engine/ast/UnaryOperationExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/UnaryOperationExpr.java @@ -72,6 +72,18 @@ private UnaryOperationExpr build() { UnaryOperationExpr unaryOperationExpr = autoBuild(); TypeNode exprType = unaryOperationExpr.expr().type(); OperatorKind operator = unaryOperationExpr.operatorKind(); + + // TODO: (summerji) Add Decl Check for variable. + // Add final keyword checking for post/prefix ++, -- when needed. + if (operator.equals(OperatorKind.UNARY_POST_INCREMENT) + && unaryOperationExpr.expr() instanceof VariableExpr) { + Preconditions.checkState( + !((VariableExpr) unaryOperationExpr.expr()).isFinal(), + String.format( + "Cannot assign a value to final variable '%s'.", + ((VariableExpr) unaryOperationExpr.expr()).variable().name())); + } + final String errorMsg = String.format( "Unary operator %s can not be applied to %s. ", operator, exprType.toString()); diff --git a/src/test/java/com/google/api/generator/engine/ast/UnaryOperationExprTest.java b/src/test/java/com/google/api/generator/engine/ast/UnaryOperationExprTest.java index c2321d1577..734156cf9c 100644 --- a/src/test/java/com/google/api/generator/engine/ast/UnaryOperationExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/UnaryOperationExprTest.java @@ -21,7 +21,7 @@ public class UnaryOperationExprTest { /** =============================== Logic Not Operation Expr =============================== */ @Test - public void logicalNotOperationExpr_validBasic() { + public void validLogicalNotOperationExpr_basic() { VariableExpr variableExpr = VariableExpr.withVariable( Variable.builder().setName("x").setType(TypeNode.BOOLEAN).build()); @@ -30,7 +30,7 @@ public void logicalNotOperationExpr_validBasic() { } @Test - public void logicalNot_validBoxedType() { + public void validLogicalNot_boxedType() { VariableExpr variableExpr = VariableExpr.withVariable( Variable.builder().setName("x").setType(TypeNode.BOOLEAN_OBJECT).build()); @@ -39,7 +39,7 @@ public void logicalNot_validBoxedType() { } @Test - public void logicalNot_invalidNumericType() { + public void invalidLogicalNot_numericType() { VariableExpr variableExpr = VariableExpr.withVariable(Variable.builder().setName("x").setType(TypeNode.INT).build()); assertThrows( @@ -47,7 +47,7 @@ public void logicalNot_invalidNumericType() { } @Test - public void logicalNot_invalidReferenceType() { + public void invalidLogicalNot_referenceType() { VariableExpr variableExpr = VariableExpr.withVariable(Variable.builder().setName("x").setType(TypeNode.STRING).build()); assertThrows( @@ -58,7 +58,7 @@ public void logicalNot_invalidReferenceType() { * =============================== Post Increment Operation Expr =============================== */ @Test - public void postIncrement_validBasic() { + public void validPostIncrement_basic() { VariableExpr variableExpr = VariableExpr.withVariable(Variable.builder().setName("x").setType(TypeNode.INT).build()); UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); @@ -66,7 +66,7 @@ public void postIncrement_validBasic() { } @Test - public void postIncrement_validBoxedType() { + public void validPostIncrement_boxedType() { VariableExpr variableExpr = VariableExpr.withVariable( Variable.builder().setName("x").setType(TypeNode.FLOAT_OBJECT).build()); @@ -75,7 +75,7 @@ public void postIncrement_validBoxedType() { } @Test - public void postIncrement_invalidBoxedBooleanType() { + public void invalidPostIncrement_boxedBooleanType() { VariableExpr variableExpr = VariableExpr.withVariable( Variable.builder().setName("x").setType(TypeNode.BOOLEAN_OBJECT).build()); @@ -85,11 +85,20 @@ public void postIncrement_invalidBoxedBooleanType() { } @Test - public void postIncrement_invalidReferenceType() { + public void invalidPostIncrement_referenceType() { VariableExpr variableExpr = VariableExpr.withVariable(Variable.builder().setName("x").setType(TypeNode.STRING).build()); assertThrows( IllegalStateException.class, () -> UnaryOperationExpr.postfixIncrementWithExpr(variableExpr)); } + + @Test + public void invalidPostIncrement_finalVariable() { + Variable var = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = VariableExpr.builder().setIsFinal(true).setVariable(var).build(); + assertThrows( + IllegalStateException.class, + () -> UnaryOperationExpr.postfixIncrementWithExpr(variableExpr)); + } } From 9ac46dd38be03cd09072624e69ae73ec0b94609d Mon Sep 17 00:00:00 2001 From: summerji Date: Sun, 4 Oct 2020 00:53:26 -0700 Subject: [PATCH 19/32] fix: add final keyword check on assignment expr --- .../generator/engine/ast/AssignmentExpr.java | 10 +++++++ .../engine/ast/AssignmentExprTest.java | 26 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java b/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java index 0e32d58c42..9223e41783 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java @@ -15,6 +15,7 @@ package com.google.api.generator.engine.ast; import com.google.auto.value.AutoValue; +import com.google.common.base.Preconditions; @AutoValue public abstract class AssignmentExpr implements Expr { @@ -67,6 +68,15 @@ public AssignmentExpr build() { lhsType.reference().name(), rhsType.reference().name())); } } + + if (!assignmentExpr.variableExpr().isDecl()) { + Preconditions.checkState( + !assignmentExpr.variableExpr().isFinal(), + String.format( + "Cannot assign a value to final variable '%s'.", + assignmentExpr.variableExpr().variable().name())); + } + return assignmentExpr; } } diff --git a/src/test/java/com/google/api/generator/engine/ast/AssignmentExprTest.java b/src/test/java/com/google/api/generator/engine/ast/AssignmentExprTest.java index 56f91f7fec..b40590f231 100644 --- a/src/test/java/com/google/api/generator/engine/ast/AssignmentExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/AssignmentExprTest.java @@ -137,6 +137,32 @@ public void writeAssignmentExpr_invalidBoxedPrimitiveType() { assertInvalidAssignmentExpr(lVariableExpr, rVariableExpr); } + @Test + public void writeAssignmentExpr_validIsDeclFinalVariableExpr() { + Variable lVariable = Variable.builder().setName("x").setType(TypeNode.INT).build(); + VariableExpr lVariableExpr = + VariableExpr.builder().setVariable(lVariable).setIsDecl(true).setIsFinal(true).build(); + + ValueExpr valueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build()); + assertValidAssignmentExpr(lVariableExpr, valueExpr); + } + + @Test + public void writeAssignmentExpr_invalidIsNotDeclFinalVariableExpr() { + Variable lVariable = Variable.builder().setName("x").setType(TypeNode.INT).build(); + VariableExpr lVariableExpr = + VariableExpr.builder().setVariable(lVariable).setIsDecl(false).setIsFinal(true).build(); + + ValueExpr valueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build()); + assertThrows( + IllegalStateException.class, + () -> { + AssignmentExpr.builder().setVariableExpr(lVariableExpr).setValueExpr(valueExpr).build(); + }); + } + private static void assertInvalidAssignmentExpr(VariableExpr variableExpr, Expr valueExpr) { assertThrows( TypeMismatchException.class, From 097ca659d334dabd03a05439ed3606879634c7f2 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:22:04 -0700 Subject: [PATCH 20/32] Integrate OperationExpr into GeneralForStatement --- .../engine/ast/GeneralForStatement.java | 78 +++---- .../engine/writer/ImportWriterVisitor.java | 4 +- .../engine/writer/JavaWriterVisitor.java | 10 +- .../composer/BatchingDescriptorComposer.java | 8 +- .../engine/ast/GeneralForStatementTest.java | 191 +++++++++++++++++- .../engine/writer/JavaWriterVisitorTest.java | 31 ++- 6 files changed, 263 insertions(+), 59 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index fba06d97d1..a84e43efa9 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -19,22 +19,18 @@ import com.google.common.collect.ImmutableList; import java.util.Collections; import java.util.List; +import javax.annotation.Nullable; @AutoValue public abstract class GeneralForStatement implements Statement { - public abstract Expr initializationExpr(); + @Nullable + public abstract AssignmentExpr initializationExpr(); - // TODO(summerji): Integrate OperationExpr here. Start by uncommenting the following section to - // replace the localVariableExpr and maxSizeExpr getters after it. - /* - // Uses the same terminology as https://docs.oracle.com/javase/tutorial/java/nutsandbolts/for.html. + @Nullable public abstract Expr terminationExpr(); - public abstract Expr incrementExpr(); - */ - - public abstract VariableExpr localVariableExpr(); - public abstract Expr maxSizeExpr(); + @Nullable + public abstract Expr incrementExpr(); public abstract ImmutableList body(); @@ -43,14 +39,26 @@ public void accept(AstNodeVisitor visitor) { visitor.visit(this); } - // Convenience wrapper. + // incrementWith is convenience wrapper to generate index-base for-loop with lower and upper bound + // and post increment on variable, like code in ```for (int i = 0; i < getMax(); i++) {..}``` + // TODO (developer): Add more convenience wrapper for the future generation needs. public static GeneralForStatement incrementWith( - VariableExpr variableExpr, Expr maxSizeExpr, List body) { - // TODO(summerji): Do some integration here, in JavaWriterVisitor, in ImportWriterVisitor, and - // add more tests. + VariableExpr localVariableExpr, + ValueExpr initialValueExpr, + Expr maxSizeExpr, + List body) { return builder() - .setLocalVariableExpr(variableExpr.toBuilder().setIsDecl(false).build()) - .setMaxSizeExpr(maxSizeExpr) + .setInitializationExpr( + AssignmentExpr.builder() + .setVariableExpr(localVariableExpr) + .setValueExpr(initialValueExpr) + .build()) + .setTerminationExpr( + RelationalOperationExpr.lessThanWithExprs( + localVariableExpr.toBuilder().setIsDecl(false).build(), maxSizeExpr)) + .setIncrementExpr( + UnaryOperationExpr.postfixIncrementWithExpr( + localVariableExpr.toBuilder().setIsDecl(false).build())) .setBody(body) .build(); } @@ -61,22 +69,22 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { - public abstract Builder setInitializationExpr(Expr initializationExpr); + abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); - public abstract Builder setBody(List body); + abstract Builder setTerminationExpr(Expr terminationExpr); - // Private. - abstract Builder setLocalVariableExpr(VariableExpr variableExpr); + abstract Builder setIncrementExpr(Expr incrementExpr); - abstract Builder setMaxSizeExpr(Expr maxSizeExpr); - - abstract VariableExpr localVariableExpr(); + abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. public GeneralForStatement build() { - VariableExpr varExpr = localVariableExpr(); + GeneralForStatement generalForStatement = autoBuild(); + VariableExpr varExpr = generalForStatement.initializationExpr().variableExpr(); + Expr terminationExpr = generalForStatement.terminationExpr(); + Expr incrementExpr = generalForStatement.incrementExpr(); Preconditions.checkState( varExpr.scope().equals(ScopeNode.LOCAL), String.format( @@ -87,16 +95,18 @@ public GeneralForStatement build() { String.format( "Variable %s in a general for-loop cannot be static or final", varExpr.variable().identifier().name())); - setInitializationExpr( - AssignmentExpr.builder() - .setVariableExpr(varExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr( - ValueExpr.withValue( - PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build())) - .build()); - // TODO(summerji): Remove the following two lines. - // This temporary workaround will be removed soon, so it doesn't need a test. - setLocalVariableExpr(varExpr.toBuilder().setIsDecl(false).build()); + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be expression statement."); return autoBuild(); } } diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index fa2648a545..98fb5b5b51 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -281,8 +281,8 @@ public void visit(ForStatement forStatement) { @Override public void visit(GeneralForStatement generalForStatement) { generalForStatement.initializationExpr().accept(this); - generalForStatement.localVariableExpr().accept(this); - generalForStatement.maxSizeExpr().accept(this); + generalForStatement.terminationExpr().accept(this); + generalForStatement.incrementExpr().accept(this); statements(generalForStatement.body()); } diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 7a1e4d3b6d..2d970a0a96 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -540,17 +540,11 @@ public void visit(GeneralForStatement generalForStatement) { semicolon(); space(); - generalForStatement.localVariableExpr().accept(this); - space(); - buffer.append(LEFT_ANGLE); - space(); - generalForStatement.maxSizeExpr().accept(this); + generalForStatement.terminationExpr().accept(this); semicolon(); space(); - generalForStatement.localVariableExpr().accept(this); - // TODO(summerji): Remove the following temporary workaround. - buffer.append("++"); + generalForStatement.incrementExpr().accept(this); rightParen(); space(); leftBrace(); diff --git a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java index 588e03517b..5d15f481e0 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -354,10 +354,16 @@ private static MethodDefinition createSplitResponseMethod( // TODO(miraleung): Increment batchMessageIndexVarExpr. VariableExpr forIndexVarExpr = - VariableExpr.withVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()); + VariableExpr.builder() + .setIsDecl(true) + .setVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()) + .build(); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); GeneralForStatement innerSubresponseForStatement = GeneralForStatement.incrementWith( forIndexVarExpr, + initValueExpr, subresponseCountVarExpr, innerSubresponseForExprs.stream() .map(e -> ExprStatement.withExpr(e)) diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 4d544d91aa..558121020c 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -21,18 +21,20 @@ import org.junit.Test; public class GeneralForStatementTest { - + /** ================================== incrementWith ========================================= */ @Test public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(true).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement.incrementWith( - variableExpr, maxSizeExpr, Arrays.asList(createBodyStatement())); + variableExpr, initValue, maxSizeExpr, Arrays.asList(createBodyStatement())); } @Test @@ -40,12 +42,14 @@ public void validGeneralForStatement_basicIsNotDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(false).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement.incrementWith( - variableExpr, maxSizeExpr, Arrays.asList(createBodyStatement())); + variableExpr, initValue, maxSizeExpr, Arrays.asList(createBodyStatement())); } @Test @@ -53,25 +57,190 @@ public void validGeneralForStatement_emptyBody() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setIsDecl(false).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); - GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); } @Test - public void invalidForStatement() { - Variable variable = Variable.builder().setName("str").setType(TypeNode.STRING).build(); + public void invalidForStatement_privateVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).setScope(ScopeNode.PRIVATE).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + + @Test + public void invalidForStatement_staticAndFinalVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsFinal(true).setIsStatic(true).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = - MethodInvocationExpr.builder().setMethodName("maxSize").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + + /** ============================== Set Three Expressions ================================== */ + @Test + public void validGeneralForState_buildExprs() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + @Test + public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + MethodInvocationExpr incrementExpr = + MethodInvocationExpr.builder() + .setMethodName("doNothing") + .setReturnType(TypeNode.INT) + .build(); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_incrementExprIsAssignmentOperationExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + AssignmentOperationExpr incrementExpr = + AssignmentOperationExpr.xorAssignmentWithExprs( + variableExpr, + ValueExpr.withValue( + PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_incrementExprIsAssignmentExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + AssignmentExpr incrementExpr = + AssignmentExpr.builder() + .setVariableExpr(variableExpr) + .setValueExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())) + .build(); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void invalidGeneralForState_buildTerminalExprNotBooleanType() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + AssignmentOperationExpr terminationExpr = + AssignmentOperationExpr.multiplyAssignmentWithExprs(variableExpr, maxValueExpr); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build()); + } + + @Test + public void invalidGeneralForState_buildIncrementExprIsNotExprStatement() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + RelationalOperationExpr incrementExpr = + RelationalOperationExpr.equalToWithExprs(variableExpr, maxValueExpr); assertThrows( IllegalStateException.class, () -> - GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList())); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .setIncrementExpr(incrementExpr) + .build()); } private static Statement createBodyStatement() { diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index bfa090c228..03e07b7c2c 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -1363,16 +1363,19 @@ public void writeForStatement() { } @Test - public void writeGeneralForStatement_basic() { + public void writeGeneralForStatement_basicIsDecl() { AssignmentExpr assignExpr = createAssignmentExpr("x", "3", TypeNode.INT); Statement assignExprStatement = ExprStatement.withExpr(assignExpr); List body = Arrays.asList(assignExprStatement, assignExprStatement); VariableExpr localVarExpr = createVariableDeclExpr("i", TypeNode.INT); - Expr maxSizeExpr = MethodInvocationExpr.builder().setMethodName("maxSize").build(); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + Expr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); GeneralForStatement forStatement = - GeneralForStatement.incrementWith(localVarExpr, maxSizeExpr, body); + GeneralForStatement.incrementWith(localVarExpr, initValueExpr, maxSizeExpr, body); forStatement.accept(writerVisitor); assertEquals( @@ -1382,6 +1385,28 @@ public void writeGeneralForStatement_basic() { "for (int i = 0; i < maxSize(); i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); } + @Test + public void writeGeneralForStatement_basicIsNotDecl() { + AssignmentExpr assignExpr = createAssignmentExpr("x", "3", TypeNode.INT); + Statement assignExprStatement = ExprStatement.withExpr(assignExpr); + List body = Arrays.asList(assignExprStatement, assignExprStatement); + + VariableExpr localVarExpr = createVariableExpr("i", TypeNode.INT); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("1").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + + GeneralForStatement forStatement = + GeneralForStatement.incrementWith(localVarExpr, initValueExpr, maxValueExpr, body); + + forStatement.accept(writerVisitor); + assertEquals( + writerVisitor.write(), + String.format( + "%s%s%s%s", "for (i = 1; i < 10; i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + } + @Test public void writeTryCatchStatement_simple() { Reference exceptionReference = ConcreteReference.withClazz(IllegalArgumentException.class); From c958696a727bd67d814a2b04000db1d0039d48a2 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:39:53 -0700 Subject: [PATCH 21/32] make exprs optional --- .../engine/ast/GeneralForStatement.java | 53 +++++++++++-------- .../engine/ast/GeneralForStatementTest.java | 39 ++++++++++++++ 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index a84e43efa9..b9ba52a8b1 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -82,31 +82,38 @@ public abstract static class Builder { // Type-checking will be done in the sub-expressions. public GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); - VariableExpr varExpr = generalForStatement.initializationExpr().variableExpr(); + AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); Expr terminationExpr = generalForStatement.terminationExpr(); Expr incrementExpr = generalForStatement.incrementExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be expression statement."); + if (initializationExpr != null) { + VariableExpr varExpr = initializationExpr.variableExpr(); + Preconditions.checkState( + varExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s in a general for-loop cannot have a non-local scope", + varExpr.variable().identifier().name())); + Preconditions.checkState( + !varExpr.isStatic() && !varExpr.isFinal(), + String.format( + "Variable %s in a general for-loop cannot be static or final", + varExpr.variable().identifier().name())); + } + if (terminationExpr != null) { + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + } + if (incrementExpr != null) { + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be expression statement."); + } return autoBuild(); } } diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 558121020c..b3a509454f 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -122,6 +122,45 @@ public void validGeneralForState_buildExprs() { .build(); } + @Test + public void validGeneralForState_buildWithoutExprs() { + // Form in `for (;;;)` + GeneralForStatement.builder().build(); + } + + @Test + public void validGeneralForState_buildWithoutTerminalExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setIncrementExpr(incrementExpr) + .build(); + } + + @Test + public void validGeneralForState_withoutIncrementExpr() { + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + ValueExpr maxValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); + AssignmentExpr initializationExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); + RelationalOperationExpr terminationExpr = + RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); + GeneralForStatement.builder() + .setInitializationExpr(initializationExpr) + .setTerminationExpr(terminationExpr) + .build(); + } + @Test public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { VariableExpr variableExpr = From 747b3b4ccd257cf82ce777b7d3278636ad404780 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 11:42:42 -0700 Subject: [PATCH 22/32] format tests --- .../api/generator/engine/ast/GeneralForStatementTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index b3a509454f..fded3b8b67 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -21,7 +21,7 @@ import org.junit.Test; public class GeneralForStatementTest { - /** ================================== incrementWith ========================================= */ + /** ============================== incrementWith ====================================== */ @Test public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); @@ -101,7 +101,7 @@ public void invalidForStatement_staticAndFinalVariable() { variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } - /** ============================== Set Three Expressions ================================== */ + /** ============================ Set Three Expressions ================================ */ @Test public void validGeneralForState_buildExprs() { VariableExpr variableExpr = From 2d7bbf4e414b9b9cb615c5a0b549cd4027154e79 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 29 Sep 2020 14:26:01 -0700 Subject: [PATCH 23/32] Add comments --- .../api/generator/engine/ast/GeneralForStatement.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index b9ba52a8b1..2bf8411748 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -69,12 +69,13 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { + // Private setter. abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); - + // Private setter. abstract Builder setTerminationExpr(Expr terminationExpr); - + // Private setter. abstract Builder setIncrementExpr(Expr incrementExpr); - + // Private setter. abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); From 08f4b2df2d8dd8489520989a75b4ae5ada2e6b81 Mon Sep 17 00:00:00 2001 From: summerji Date: Thu, 1 Oct 2020 11:28:03 -0700 Subject: [PATCH 24/32] remove null case for expressions --- .../engine/ast/GeneralForStatement.java | 56 ++++++++----------- .../engine/ast/GeneralForStatementTest.java | 39 ------------- 2 files changed, 23 insertions(+), 72 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index 2bf8411748..bbb28011e4 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -19,17 +19,13 @@ import com.google.common.collect.ImmutableList; import java.util.Collections; import java.util.List; -import javax.annotation.Nullable; @AutoValue public abstract class GeneralForStatement implements Statement { - @Nullable public abstract AssignmentExpr initializationExpr(); - @Nullable public abstract Expr terminationExpr(); - @Nullable public abstract Expr incrementExpr(); public abstract ImmutableList body(); @@ -86,35 +82,29 @@ public GeneralForStatement build() { AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); Expr terminationExpr = generalForStatement.terminationExpr(); Expr incrementExpr = generalForStatement.incrementExpr(); - if (initializationExpr != null) { - VariableExpr varExpr = initializationExpr.variableExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - } - if (terminationExpr != null) { - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - } - if (incrementExpr != null) { - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be expression statement."); - } + VariableExpr varExpr = initializationExpr.variableExpr(); + Preconditions.checkState( + varExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s in a general for-loop cannot have a non-local scope", + varExpr.variable().identifier().name())); + Preconditions.checkState( + !varExpr.isStatic() && !varExpr.isFinal(), + String.format( + "Variable %s in a general for-loop cannot be static or final", + varExpr.variable().identifier().name())); + Preconditions.checkState( + terminationExpr.type().equals(TypeNode.BOOLEAN), + "Terminal expression %s must be boolean-type expression."); + Preconditions.checkState( + (incrementExpr instanceof MethodInvocationExpr) + || (incrementExpr instanceof AssignmentExpr) + || (incrementExpr instanceof AssignmentOperationExpr) + // TODO(developer): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needs. + || (incrementExpr instanceof UnaryOperationExpr + && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); return autoBuild(); } } diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index fded3b8b67..30ad93c983 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -122,45 +122,6 @@ public void validGeneralForState_buildExprs() { .build(); } - @Test - public void validGeneralForState_buildWithoutExprs() { - // Form in `for (;;;)` - GeneralForStatement.builder().build(); - } - - @Test - public void validGeneralForState_buildWithoutTerminalExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - - @Test - public void validGeneralForState_withoutIncrementExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .build(); - } - @Test public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { VariableExpr variableExpr = From 83176f91c4c32fddf9d91117b88a64d51356f197 Mon Sep 17 00:00:00 2001 From: summerji Date: Fri, 2 Oct 2020 02:58:05 -0700 Subject: [PATCH 25/32] fix comments --- .../engine/ast/GeneralForStatement.java | 22 +++++++++++++++---- .../engine/ast/GeneralForStatementTest.java | 5 ----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index bbb28011e4..f02539c8f4 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -14,6 +14,7 @@ package com.google.api.generator.engine.ast; +import autovalue.shaded.com.google$.common.annotations.$VisibleForTesting; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -22,6 +23,14 @@ @AutoValue public abstract class GeneralForStatement implements Statement { + private int i = 0; + for (i = 0; i < 10; i++) {} + + void foobar() { + for (i = 0; i < 10; i++) { + System.out.println("i = " + i); + } + } public abstract AssignmentExpr initializationExpr(); public abstract Expr terminationExpr(); @@ -37,7 +46,7 @@ public void accept(AstNodeVisitor visitor) { // incrementWith is convenience wrapper to generate index-base for-loop with lower and upper bound // and post increment on variable, like code in ```for (int i = 0; i < getMax(); i++) {..}``` - // TODO (developer): Add more convenience wrapper for the future generation needs. + // TODO (unsupported): Add more convenience wrapper for the future generation needs. public static GeneralForStatement incrementWith( VariableExpr localVariableExpr, ValueExpr initialValueExpr, @@ -64,19 +73,24 @@ public static Builder builder() { } @AutoValue.Builder - public abstract static class Builder { + abstract static class Builder { // Private setter. + @$VisibleForTesting abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); // Private setter. + @$VisibleForTesting abstract Builder setTerminationExpr(Expr terminationExpr); // Private setter. + @$VisibleForTesting abstract Builder setIncrementExpr(Expr incrementExpr); // Private setter. + @$VisibleForTesting abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. + @$VisibleForTesting public GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); @@ -100,8 +114,8 @@ public GeneralForStatement build() { (incrementExpr instanceof MethodInvocationExpr) || (incrementExpr instanceof AssignmentExpr) || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(developer): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needs. + // TODO(unsupported): Currently we only support postIncrement (i++), please add + // postDecrement, prefixIncrement, prefixIncrement if needed. || (incrementExpr instanceof UnaryOperationExpr && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index 30ad93c983..dec7e897d9 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -139,11 +139,6 @@ public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { .setMethodName("doNothing") .setReturnType(TypeNode.INT) .build(); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); } @Test From 2a6fd7285715f0c315ca1c69a7ec332d28f7ada4 Mon Sep 17 00:00:00 2001 From: summerji Date: Sat, 3 Oct 2020 23:57:58 -0700 Subject: [PATCH 26/32] Update second version design --- .../engine/ast/GeneralForStatement.java | 92 ++++---- .../engine/writer/ImportWriterVisitor.java | 2 +- .../engine/writer/JavaWriterVisitor.java | 2 +- .../engine/ast/GeneralForStatementTest.java | 196 ++++++------------ 4 files changed, 116 insertions(+), 176 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index f02539c8f4..6683804608 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -14,7 +14,6 @@ package com.google.api.generator.engine.ast; -import autovalue.shaded.com.google$.common.annotations.$VisibleForTesting; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -23,19 +22,11 @@ @AutoValue public abstract class GeneralForStatement implements Statement { - private int i = 0; - for (i = 0; i < 10; i++) {} - - void foobar() { - for (i = 0; i < 10; i++) { - System.out.println("i = " + i); - } - } public abstract AssignmentExpr initializationExpr(); public abstract Expr terminationExpr(); - public abstract Expr incrementExpr(); + public abstract Expr updateExpr(); public abstract ImmutableList body(); @@ -61,64 +52,75 @@ public static GeneralForStatement incrementWith( .setTerminationExpr( RelationalOperationExpr.lessThanWithExprs( localVariableExpr.toBuilder().setIsDecl(false).build(), maxSizeExpr)) - .setIncrementExpr( + .setUpdateExpr( UnaryOperationExpr.postfixIncrementWithExpr( localVariableExpr.toBuilder().setIsDecl(false).build())) .setBody(body) .build(); } - public static Builder builder() { + private static Builder builder() { return new AutoValue_GeneralForStatement.Builder().setBody(Collections.emptyList()); } @AutoValue.Builder abstract static class Builder { // Private setter. - @$VisibleForTesting abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); // Private setter. - @$VisibleForTesting abstract Builder setTerminationExpr(Expr terminationExpr); // Private setter. - @$VisibleForTesting - abstract Builder setIncrementExpr(Expr incrementExpr); + abstract Builder setUpdateExpr(Expr incrementExpr); // Private setter. - @$VisibleForTesting abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. - @$VisibleForTesting - public GeneralForStatement build() { + GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); - AssignmentExpr initializationExpr = generalForStatement.initializationExpr(); - Expr terminationExpr = generalForStatement.terminationExpr(); - Expr incrementExpr = generalForStatement.incrementExpr(); - VariableExpr varExpr = initializationExpr.variableExpr(); - Preconditions.checkState( - varExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s in a general for-loop cannot have a non-local scope", - varExpr.variable().identifier().name())); - Preconditions.checkState( - !varExpr.isStatic() && !varExpr.isFinal(), - String.format( - "Variable %s in a general for-loop cannot be static or final", - varExpr.variable().identifier().name())); - Preconditions.checkState( - terminationExpr.type().equals(TypeNode.BOOLEAN), - "Terminal expression %s must be boolean-type expression."); - Preconditions.checkState( - (incrementExpr instanceof MethodInvocationExpr) - || (incrementExpr instanceof AssignmentExpr) - || (incrementExpr instanceof AssignmentOperationExpr) - // TODO(unsupported): Currently we only support postIncrement (i++), please add - // postDecrement, prefixIncrement, prefixIncrement if needed. - || (incrementExpr instanceof UnaryOperationExpr - && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - "Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression."); + VariableExpr localVarExpr = generalForStatement.initializationExpr().variableExpr(); + // Declare a variable inside for-loop initialization expression. + if (localVarExpr.isDecl()) { + Preconditions.checkState( + localVarExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s declare in a general for-loop cannot have a non-local scope", + localVarExpr.variable().identifier().name())); + Preconditions.checkState(!localVarExpr.isStatic(), "Modifier 'static' not allow here."); + } + // Assign a variable in for-loop initialization expression. + if (!localVarExpr.isDecl() && !localVarExpr.scope().equals(ScopeNode.LOCAL)) { + Preconditions.checkState( + !localVarExpr.isFinal(), + "Cannot assign a value to final variable %s.", + localVarExpr.variable().identifier().name()); + } + // TODO (unsupport): Uncomment the following code if public setter for the initialization, + // termination, update expressions when needed. + // Preconditions.checkState( + // (initializationExpr instanceof MethodInvocationExpr) + // || (incrementExpr instanceof AssignmentExpr) + // || (incrementExpr instanceof AssignmentOperationExpr) + // // TODO(unsupported): Currently we only support postIncrement (i++), please add + // // postDecrement, prefixIncrement, prefixIncrement if needed. + // || (incrementExpr instanceof UnaryOperationExpr + // && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + // "Initialization expression %s must be either a method invocation, assignment, or unary + // post-fix operation expression."); + // Preconditions.checkState( + // terminationExpr.type().equals(TypeNode.BOOLEAN), + // "Terminal expression %s must be boolean-type expression."); + // Preconditions.checkState( + // (incrementExpr instanceof MethodInvocationExpr) + // || (incrementExpr instanceof AssignmentExpr) + // || (incrementExpr instanceof AssignmentOperationExpr) + // // TODO(unsupported): Currently we only support postIncrement (i++), please add + // // postDecrement, prefixIncrement, prefixIncrement if needed. + // || (incrementExpr instanceof UnaryOperationExpr + // && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), + // "Increment expression %s must be either a method invocation, assignment, or unary + // post-fix operation expression."); return autoBuild(); } } diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index 98fb5b5b51..03e70240c9 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -282,7 +282,7 @@ public void visit(ForStatement forStatement) { public void visit(GeneralForStatement generalForStatement) { generalForStatement.initializationExpr().accept(this); generalForStatement.terminationExpr().accept(this); - generalForStatement.incrementExpr().accept(this); + generalForStatement.updateExpr().accept(this); statements(generalForStatement.body()); } diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 2d970a0a96..1945b12366 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -544,7 +544,7 @@ public void visit(GeneralForStatement generalForStatement) { semicolon(); space(); - generalForStatement.incrementExpr().accept(this); + generalForStatement.updateExpr().accept(this); rightParen(); space(); leftBrace(); diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index dec7e897d9..ecc39890e0 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -23,6 +23,7 @@ public class GeneralForStatementTest { /** ============================== incrementWith ====================================== */ @Test + // validGeneralForStatement_basicIsDecl tests declare a variable inside in initialization expr. public void validGeneralForStatement_basicIsDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -38,6 +39,8 @@ public void validGeneralForStatement_basicIsDecl() { } @Test + // validGeneralForStatement_basicIsNotDecl tests assigning a method-level local variable in + // initialization expr. public void validGeneralForStatement_basicIsNotDecl() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -53,6 +56,7 @@ public void validGeneralForStatement_basicIsNotDecl() { } @Test + // validGeneralForStatement_emptyBody tests set empty body in update expr. public void validGeneralForStatement_emptyBody() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = @@ -68,27 +72,55 @@ public void validGeneralForStatement_emptyBody() { } @Test - public void invalidForStatement_privateVariable() { + // validForStatement_privateNotIsDeclVariable tests assigning an instance variable with private + // scope. + public void validForStatement_privateNotIsDeclVariable() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.builder().setVariable(variable).setScope(ScopeNode.PRIVATE).build(); + VariableExpr.builder() + .setIsDecl(false) + .setVariable(variable) + .setScope(ScopeNode.PRIVATE) + .build(); ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); + } - assertThrows( - IllegalStateException.class, - () -> - GeneralForStatement.incrementWith( - variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + @Test + // validForStatement_instanceStaticVariable tests assigning an instance variable with public scope + // and static modifier. + public void validForStatement_instanceStaticVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder() + .setIsDecl(false) + .setVariable(variable) + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); } @Test - public void invalidForStatement_staticAndFinalVariable() { + // invalidForStatement_privateIsDeclVariable tests declaring a non-local variable inside of + // for-loop. + public void invalidForStatement_privateIsDeclVariable() { Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.builder().setVariable(variable).setIsFinal(true).setIsStatic(true).build(); + VariableExpr.builder() + .setIsDecl(true) + .setVariable(variable) + .setScope(ScopeNode.PRIVATE) + .build(); ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); MethodInvocationExpr maxSizeExpr = @@ -101,141 +133,47 @@ public void invalidForStatement_staticAndFinalVariable() { variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } - /** ============================ Set Three Expressions ================================ */ - @Test - public void validGeneralForState_buildExprs() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - @Test - public void validGeneralForState_incrementExprIsMethodInvocationEpxr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - MethodInvocationExpr incrementExpr = - MethodInvocationExpr.builder() - .setMethodName("doNothing") - .setReturnType(TypeNode.INT) - .build(); - } - - @Test - public void validGeneralForState_incrementExprIsAssignmentOperationExpr() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - AssignmentOperationExpr incrementExpr = - AssignmentOperationExpr.xorAssignmentWithExprs( - variableExpr, - ValueExpr.withValue( - PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } - - @Test - public void validGeneralForState_incrementExprIsAssignmentExpr() { + // invalidForStatement_staticIsDeclVariable tests declare a static local variable in + // initialization expr. + public void invalidForStatement_staticIsDeclVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(true).setIsStatic(true).build(); + ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - AssignmentExpr incrementExpr = - AssignmentExpr.builder() - .setVariableExpr(variableExpr) - .setValueExpr( - ValueExpr.withValue( - PrimitiveValue.builder().setValue("5").setType(TypeNode.INT).build())) - .build(); - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build(); - } + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); - @Test - public void invalidGeneralForState_buildTerminalExprNotBooleanType() { - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - AssignmentOperationExpr terminationExpr = - AssignmentOperationExpr.multiplyAssignmentWithExprs(variableExpr, maxValueExpr); - UnaryOperationExpr incrementExpr = UnaryOperationExpr.postfixIncrementWithExpr(variableExpr); assertThrows( IllegalStateException.class, () -> - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } @Test - public void invalidGeneralForState_buildIncrementExprIsNotExprStatement() { + // invalidForStatement_finalIsNotDeclVariable tests invalid case of assigning the initial value to + // a variable which is declared as a final instance variable. + public void invalidForStatement_finalIsNotDeclVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("i").setType(TypeNode.INT).build()); - ValueExpr initValueExpr = + VariableExpr.builder() + .setVariable(variable) + .setIsDecl(false) + .setScope(ScopeNode.PUBLIC) + .setIsFinal(true) + .build(); + ValueExpr initValue = ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - ValueExpr maxValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("10").setType(TypeNode.INT).build()); - AssignmentExpr initializationExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(initValueExpr).build(); - RelationalOperationExpr terminationExpr = - RelationalOperationExpr.lessThanWithExprs(variableExpr, maxValueExpr); - RelationalOperationExpr incrementExpr = - RelationalOperationExpr.equalToWithExprs(variableExpr, maxValueExpr); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + assertThrows( IllegalStateException.class, () -> - GeneralForStatement.builder() - .setInitializationExpr(initializationExpr) - .setTerminationExpr(terminationExpr) - .setIncrementExpr(incrementExpr) - .build()); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } private static Statement createBodyStatement() { From d7931b07da096843756518077fe5ae4213c87e3b Mon Sep 17 00:00:00 2001 From: summerji Date: Sun, 4 Oct 2020 01:11:17 -0700 Subject: [PATCH 27/32] rebase fix in assignment and increment expr --- .../engine/ast/GeneralForStatement.java | 7 ------- .../engine/ast/GeneralForStatementTest.java | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index 6683804608..b6622996a1 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -89,13 +89,6 @@ GeneralForStatement build() { localVarExpr.variable().identifier().name())); Preconditions.checkState(!localVarExpr.isStatic(), "Modifier 'static' not allow here."); } - // Assign a variable in for-loop initialization expression. - if (!localVarExpr.isDecl() && !localVarExpr.scope().equals(ScopeNode.LOCAL)) { - Preconditions.checkState( - !localVarExpr.isFinal(), - "Cannot assign a value to final variable %s.", - localVarExpr.variable().identifier().name()); - } // TODO (unsupport): Uncomment the following code if public setter for the initialization, // termination, update expressions when needed. // Preconditions.checkState( diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java index ecc39890e0..4d14429c1a 100644 --- a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -176,6 +176,25 @@ public void invalidForStatement_finalIsNotDeclVariable() { variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } + @Test + // invalidForStatement_localFinalVariable tests declare a final variable in initialization expr, + // updateExpr should throw error. + public void invalidForStatement_localFinalVariable() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(true).setIsFinal(true).build(); + ValueExpr initValue = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + private static Statement createBodyStatement() { Variable variable = Variable.builder().setName("x").setType(TypeNode.INT).build(); VariableExpr variableExpr = From ac9904f08e41748889bdcf7a3fd29b515b88e8b0 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 6 Oct 2020 12:16:42 -0700 Subject: [PATCH 28/32] remove .circleci/config.yml --- .circleci/config.yml | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index cfee57f5a4..5a5afd7c9d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,21 +3,46 @@ version: 2.1 # ======================= JOBS ======================= jobs: gapic-generator-java-tests: - docker: - - image: l.gcr.io/google/bazel - working_directory: /home/circleci/project/gapic-generator-java + working_directory: /tmp/ + environment: + TEST_REPORTS_DIR: /tmp/workspace/bazel/reports/gapic-generator-java + BAZEL_VERSION: 3.5.1 + PYTHON_VERSION: 3.5.2 + machine: true steps: - - checkout + - checkout: + path: gapic-generator-java + - attach_workspace: + at: workspace + - run: + name: Set Python version + command: | + pyenv global ${PYTHON_VERSION} + - run: + name: Install Bazel + command: | + wget https://github.com/bazelbuild/bazel/releases/download/${BAZEL_VERSION}/bazel-${BAZEL_VERSION}-installer-linux-x86_64.sh -O bazel_installer.sh + chmod +x bazel_installer.sh + ./bazel_installer.sh --user + - run: + name: Make reports directory + command: | + mkdir -p ${TEST_REPORTS_DIR} - run: - name: Build targets + name: Build targets for gapic-generator-java command: | + cd /tmp/gapic-generator-java bazel --batch build //... - run: - name: Test targets + name: Run unit tests for gapic-generator-java command: | + cd /tmp/gapic-generator-java bazel --batch test //... --noshow_progress + find . -type f -regex ".*/bazel-testlogs/.*xml" -exec cp {} ${TEST_REPORTS_DIR} \; - store_test_results: - path: ~/.cache/bazel + path: bazel/reports/gapic-generator-java + - store_artifacts: + path: bazel/reports/gapic-generator-java google-java-format: docker: - image: l.gcr.io/google/bazel From 11245491959f5b6c311d37b0e799b2eb2bf5b6ed Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 6 Oct 2020 12:20:26 -0700 Subject: [PATCH 29/32] checkout master with files --- BUILD.bazel | 44 -------- rules_bazel/java/java_diff_test.bzl | 103 +++++++++--------- .../composer/BatchingDescriptorComposer.java | 8 +- .../google/api/generator/engine/BUILD.bazel | 29 ++--- .../api/generator/gapic/composer/BUILD.bazel | 81 +++++++------- .../api/generator/gapic/dummy/BUILD.bazel | 33 +++--- .../api/generator/test/framework/BUILD.bazel | 22 +++- 7 files changed, 142 insertions(+), 178 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 7edfa6b8ac..a767045a39 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -46,50 +46,6 @@ java_binary( ], ) -# JUnit runner binary, this is used to generate test output for updating goldens files. -# Run `bazel run testTarget_update` will trigger this runner. -java_binary( - name = "junit_runner", - srcs = [ - "//src/test/java/com/google/api/generator/gapic/dummy:dummy_files", - "//src/test/java/com/google/api/generator/engine:engine_files", - "//src/test/java/com/google/api/generator/gapic/composer:composer_files", - "//src/test/java/com/google/api/generator/test/framework:framework_files", - ], - data = [ - "//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files", - "//src/test/java/com/google/api/generator/engine/goldens:goldens_files", - "//src/test/java/com/google/api/generator/gapic/composer/goldens:goldens_files", - "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files", - "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", - ], - jvm_flags = ["-Xmx512m"], - main_class = "com.google.api.generator.test.framework.SingleJUnitTestRunner", - visibility = ["//visibility:public"], - deps = [ - "//src/main/java/com/google/api/generator/engine/ast", - "//src/main/java/com/google/api/generator/engine/writer", - "//src/test/java/com/google/api/generator/test/framework", - "//src/main/java/com/google/api/generator/engine/lexicon", - "//src/main/java/com/google/api/generator/gapic/composer", - "//src/main/java/com/google/api/generator/gapic/model", - "//src/main/java/com/google/api/generator/gapic/protoparser", - "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", - "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto", - "//src/test/java/com/google/api/generator/gapic/composer:common_resources_java_proto", - "@com_google_guava_guava//:com_google_guava_guava", - "@com_google_api_gax_java//gax", - "@com_google_googleapis//google/logging/v2:logging_java_proto", - "@com_google_googleapis//google/pubsub/v1:pubsub_java_proto", - "@com_google_googleapis//google/rpc:rpc_java_proto", - "@com_google_protobuf//:protobuf_java", - "//:service_config_java_proto", - "@com_google_truth_truth//jar", - "@io_github_java_diff_utils//jar", - "@junit_junit//jar", - "@org_hamcrest_hamcrest_core//jar", - ], -) # google-java-format java_binary( diff --git a/rules_bazel/java/java_diff_test.bzl b/rules_bazel/java/java_diff_test.bzl index 41ba278117..626c33b251 100644 --- a/rules_bazel/java/java_diff_test.bzl +++ b/rules_bazel/java/java_diff_test.bzl @@ -1,87 +1,84 @@ -def _junit_output_impl(ctx): - test_class_name = ctx.attr.test_class_name +def _overwrite_golden_impl(ctx): + test_class = ctx.attr.test_class inputs = ctx.files.srcs - output = ctx.outputs.output + goldens_output_zip = ctx.outputs.goldens_output_zip test_runner = ctx.executable.test_runner - command = """ - mkdir local_tmp - TEST_OUTPUT_HOME="$(pwd)/local_tmp" \ - {test_runner_path} $@ - cd local_tmp + # Generate the goldens from tests. + generate_goldens_script = """ + mkdir local_tmp + TEST_OUTPUT_HOME="$(pwd)/local_tmp" {test_runner_path} $@ + cd local_tmp # Zip all files under local_tmp with all nested parent folders except for local_tmp itself. # Zip files because there are cases that one Junit test can produce multiple goldens. - zip -r ../{output} . + zip -r ../{goldens_output_zip} . """.format( test_runner_path = test_runner.path, - output=output.path, + goldens_output_zip = goldens_output_zip.path, ) ctx.actions.run_shell( inputs = inputs, - outputs = [output], - arguments = [test_class_name], + outputs = [goldens_output_zip], + arguments = [test_class], tools = [test_runner], - command = command, + command = generate_goldens_script, ) -junit_output_zip = rule( - attrs = { - "test_class_name": attr.string(mandatory=True), - "srcs": attr.label_list( - allow_files = True, - mandatory = True, - ), - "test_runner": attr.label( - mandatory = True, - executable = True, - cfg = "host", - ), - }, - outputs = { - "output": "%{name}.zip", - }, - implementation = _junit_output_impl, -) - -def _overwritten_golden_impl(ctx): - script_content = """ + # Overwrite the goldens. + golden_update_script_content = """ #!/bin/bash cd ${{BUILD_WORKSPACE_DIRECTORY}} - unzip -ao {unit_test_results} -d src/test/java + unzip -ao {goldens_output_zip} -d src/test/java """.format( - unit_test_results = ctx.file.unit_test_results.path, + goldens_output_zip = goldens_output_zip.path, ) ctx.actions.write( - output = ctx.outputs.bin, - content = script_content, + output = ctx.outputs.golden_update_script, + content = golden_update_script_content, is_executable = True, ) - return [DefaultInfo(executable = ctx.outputs.bin)] - + return [DefaultInfo(executable = ctx.outputs.golden_update_script)] -overwritten_golden = rule( +overwrite_golden = rule( attrs = { - "unit_test_results": attr.label( + "test_class": attr.string(mandatory = True), + "srcs": attr.label_list( + allow_files = True, + mandatory = True, + ), + "test_runner": attr.label( mandatory = True, - allow_single_file = True), + executable = True, + cfg = "host", + ), }, outputs = { - "bin": "%{name}.sh", + "goldens_output_zip": "%{name}.zip", + "golden_update_script": "%{name}.sh", }, executable = True, - implementation = _overwritten_golden_impl, + implementation = _overwrite_golden_impl, ) -def golden_update(name, test_class_name, srcs): - junit_output_name = "%s_output" % name - junit_output_zip( - name = junit_output_name, - test_class_name = test_class_name, - test_runner = "//:junit_runner", +def golden_update( + name, + srcs, + test_class, + data = [], + deps = []): + golden_junit_runner_name = "%s_junit_runner" % name + native.java_binary( + name = golden_junit_runner_name, srcs = srcs, + data = data, + main_class = "com.google.api.generator.test.framework.SingleJUnitTestRunner", + deps = ["//src/test/java/com/google/api/generator/test/framework:junit_runner"] + deps, ) - overwritten_golden( + + overwrite_golden( name = name, - unit_test_results = ":%s" % junit_output_name + test_class = test_class, + test_runner = ":%s" % golden_junit_runner_name, + srcs = srcs + data, ) diff --git a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java index 5d15f481e0..588e03517b 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -354,16 +354,10 @@ private static MethodDefinition createSplitResponseMethod( // TODO(miraleung): Increment batchMessageIndexVarExpr. VariableExpr forIndexVarExpr = - VariableExpr.builder() - .setIsDecl(true) - .setVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()) - .build(); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); + VariableExpr.withVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()); GeneralForStatement innerSubresponseForStatement = GeneralForStatement.incrementWith( forIndexVarExpr, - initValueExpr, subresponseCountVarExpr, innerSubresponseForExprs.stream() .map(e -> ExprStatement.withExpr(e)) diff --git a/src/test/java/com/google/api/generator/engine/BUILD.bazel b/src/test/java/com/google/api/generator/engine/BUILD.bazel index 1bcbd20f5f..f1b9c96ae0 100644 --- a/src/test/java/com/google/api/generator/engine/BUILD.bazel +++ b/src/test/java/com/google/api/generator/engine/BUILD.bazel @@ -15,27 +15,30 @@ TESTS = [ "JavaCodeGeneratorTest", ] +TEST_DEPS = [ + "//src/main/java/com/google/api/generator/engine/ast", + "//src/main/java/com/google/api/generator/engine/writer", + "//src/test/java/com/google/api/generator/test/framework:asserts", + "//src/test/java/com/google/api/generator/test/framework:utils", + "@junit_junit//jar", +] + [java_test( name = test_name, srcs = ["{0}.java".format(test_name)], data = ["//src/test/java/com/google/api/generator/engine/goldens:goldens_files"], test_class = "com.google.api.generator.engine.{0}".format(test_name), - deps = [ - "//src/main/java/com/google/api/generator/engine/ast", - "//src/main/java/com/google/api/generator/engine/writer", - "//src/test/java/com/google/api/generator/test/framework", - "@junit_junit//jar", - ], + deps = TEST_DEPS, ) for test_name in TESTS] TEST_CLASS_NAME = "com.google.api.generator.engine.JavaCodeGeneratorTest" # Run `bazel run src/test/java/com/google/api/generator/engine:JavaCodeGeneratorTest_update` # to update goldens as expected generated code. -golden_update( - name = "JavaCodeGeneratorTest_update", - srcs = [ - ":engine_files", - ], - test_class_name = TEST_CLASS_NAME, -) +[golden_update( + name = "{0}_update".format(test_name), + srcs = ["{0}.java".format(test_name)], + data = ["//src/test/java/com/google/api/generator/engine/goldens:goldens_files"], + test_class = "com.google.api.generator.engine.{0}".format(test_name), + deps = TEST_DEPS, +) for test_name in TESTS] diff --git a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel index 6e02d520d4..9211d8b5a1 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -2,35 +2,47 @@ load("//:rules_bazel/java/java_diff_test.bzl", "golden_update") package(default_visibility = ["//visibility:public"]) -TESTS = [ +UPDATE_GOLDENS_TESTS = [ "BatchingDescriptorComposerTest", "ComposerTest", - "DefaultValueComposerTest", "GrpcServiceCallableFactoryClassComposerTest", "GrpcServiceStubClassComposerTest", "MockServiceClassComposerTest", "MockServiceImplClassComposerTest", "ResourceNameHelperClassComposerTest", + "ServiceSettingsClassComposerTest", + "ServiceStubSettingsClassComposerTest", + "ServiceStubClassComposerTest", +] + +TESTS = UPDATE_GOLDENS_TESTS + [ + "DefaultValueComposerTest", "ResourceNameTokenizerTest", "RetrySettingsComposerTest", "ServiceClientClassComposerTest", "ServiceClientTestClassComposerTest", - "ServiceSettingsClassComposerTest", - "ServiceStubSettingsClassComposerTest", - "ServiceStubClassComposerTest", ] -UPDATE_GOLDENS_TESTS = [ - "BatchingDescriptorComposerTest", - "ComposerTest", - "GrpcServiceCallableFactoryClassComposerTest", - "GrpcServiceStubClassComposerTest", - "MockServiceClassComposerTest", - "MockServiceImplClassComposerTest", - "ResourceNameHelperClassComposerTest", - "ServiceSettingsClassComposerTest", - "ServiceStubSettingsClassComposerTest", - "ServiceStubClassComposerTest", +TEST_DEPS = [ + ":common_resources_java_proto", + "//:service_config_java_proto", + "//src/main/java/com/google/api/generator/engine/ast", + "//src/main/java/com/google/api/generator/engine/writer", + "//src/main/java/com/google/api/generator/gapic/composer", + "//src/test/java/com/google/api/generator/test/framework:asserts", + "//src/test/java/com/google/api/generator/test/framework:utils", + "//src/main/java/com/google/api/generator/gapic/model", + "//src/main/java/com/google/api/generator/gapic/protoparser", + "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", + "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto", + "@com_google_api_gax_java//gax", + "@com_google_googleapis//google/logging/v2:logging_java_proto", + "@com_google_googleapis//google/pubsub/v1:pubsub_java_proto", + "@com_google_googleapis//google/rpc:rpc_java_proto", + "@com_google_guava_guava", + "@com_google_protobuf//:protobuf_java", + "@com_google_truth_truth//jar", + "@junit_junit//jar", ] filegroup( @@ -55,7 +67,7 @@ java_proto_library( [java_test( name = test_name, srcs = [ - "{0}.java".format(test_name), + "{0}.java".format(test_name), "ComposerConstants.java", ], data = [ @@ -64,40 +76,25 @@ java_proto_library( "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", ], test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name), - deps = [ - ":common_resources_java_proto", - "//:service_config_java_proto", - "//src/main/java/com/google/api/generator/engine/ast", - "//src/main/java/com/google/api/generator/engine/writer", - "//src/main/java/com/google/api/generator/gapic/composer", - "//src/test/java/com/google/api/generator/test/framework", - "//src/main/java/com/google/api/generator/gapic/model", - "//src/main/java/com/google/api/generator/gapic/protoparser", - "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", - "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto", - "@com_google_api_gax_java//gax", - "@com_google_googleapis//google/logging/v2:logging_java_proto", - "@com_google_googleapis//google/pubsub/v1:pubsub_java_proto", - "@com_google_googleapis//google/rpc:rpc_java_proto", - "@com_google_protobuf//:protobuf_java", - "@com_google_truth_truth//jar", - "@junit_junit//jar", - ], + deps = TEST_DEPS, ) for test_name in TESTS] - - TEST_CLASS_DIR = "com.google.api.generator.gapic.composer." # Run `bazel run src/test/java/com/google/api/generator/gapic/composer:testTargetName_update` # to update goldens as expected generated code. # `ServiceClient*` tests are not supported now since they are still in active development. [golden_update( - name = test_name + "_update", + name = "{0}_update".format(test_name), srcs = [ - ":composer_files", + "{0}.java".format(test_name), + "ComposerConstants.java", + ], + data = [ + "//src/test/java/com/google/api/generator/gapic/composer/goldens:goldens_files", "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files", "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", ], - test_class_name = TEST_CLASS_DIR + test_name, -) for test_name in UPDATE_GOLDENS_TESTS] + test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name), + deps = TEST_DEPS, +) for test_name in UPDATE_GOLDENS_TESTS] diff --git a/src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel index 3caa66fb99..0a4ae41759 100644 --- a/src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel @@ -6,6 +6,14 @@ TESTS = [ "FileDiffInfraDummyTest", ] +TEST_DEPS = [ + "//src/main/java/com/google/api/generator/engine/ast", + "//src/main/java/com/google/api/generator/engine/writer", + "//src/test/java/com/google/api/generator/test/framework:asserts", + "//src/test/java/com/google/api/generator/test/framework:utils", + "@junit_junit//jar", +] + filegroup( name = "dummy_files", srcs = ["{0}.java".format(f) for f in TESTS], @@ -14,26 +22,19 @@ filegroup( [java_test( name = test_name, srcs = ["{0}.java".format(test_name)], - data = glob(["goldens/*.golden"]), + data = ["//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files"], test_class = "com.google.api.generator.gapic.dummy.{0}".format(test_name), - deps = [ - "//src/main/java/com/google/api/generator/engine/ast", - "//src/main/java/com/google/api/generator/engine/writer", - "//src/test/java/com/google/api/generator/test/framework", - "@junit_junit//jar", - ], + deps = TEST_DEPS, ) for test_name in TESTS] TEST_CLASS_NAME = "com.google.api.generator.gapic.dummy.FileDiffInfraDummyTest" # Run `bazel run src/test/java/com/google/api/generator/gapic/dummy:FileDiffInfraDummyTest_update` # to update goldens as expected generated code. -golden_update( - name = "FileDiffInfraDummyTest_update", - srcs = [ - ":dummy_files", - "//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files", - "//src/test/java/com/google/api/generator/test/framework:framework_files", - ], - test_class_name = TEST_CLASS_NAME, -) +[golden_update( + name = "{0}_update".format(test_name), + srcs = ["{0}.java".format(test_name)], + data = ["//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files"], + test_class = "com.google.api.generator.gapic.dummy.{0}".format(test_name), + deps = TEST_DEPS, +) for test_name in TESTS] diff --git a/src/test/java/com/google/api/generator/test/framework/BUILD.bazel b/src/test/java/com/google/api/generator/test/framework/BUILD.bazel index 31b6d7d6f7..5d167df4bb 100644 --- a/src/test/java/com/google/api/generator/test/framework/BUILD.bazel +++ b/src/test/java/com/google/api/generator/test/framework/BUILD.bazel @@ -6,14 +6,30 @@ filegroup( ) java_library( - name = "framework", + name = "asserts", srcs = [ - ":framework_files", + "Assert.java", + "Differ.java", ], - data = ["//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files"], deps = [ "@io_github_java_diff_utils//jar", "@junit_junit//jar", "@org_hamcrest_hamcrest_core//jar", ], ) + +java_library( + name = "utils", + srcs = ["Utils.java"], +) + +java_library( + name = "junit_runner", + srcs = [ + "SingleJUnitTestRunner.java", + ], + deps = [ + ":utils", + "@junit_junit//jar", + ], +) From ea276a0d9da16c04638f3de2fa5f38e0da76c9e2 Mon Sep 17 00:00:00 2001 From: summerji Date: Tue, 6 Oct 2020 14:09:50 -0700 Subject: [PATCH 30/32] git batching --- .../gapic/composer/BatchingDescriptorComposer.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java index 588e03517b..5d15f481e0 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -354,10 +354,16 @@ private static MethodDefinition createSplitResponseMethod( // TODO(miraleung): Increment batchMessageIndexVarExpr. VariableExpr forIndexVarExpr = - VariableExpr.withVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()); + VariableExpr.builder() + .setIsDecl(true) + .setVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()) + .build(); + ValueExpr initValueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); GeneralForStatement innerSubresponseForStatement = GeneralForStatement.incrementWith( forIndexVarExpr, + initValueExpr, subresponseCountVarExpr, innerSubresponseForExprs.stream() .map(e -> ExprStatement.withExpr(e)) From 1e7c8384cd1879d191332520754d7f16b5ed48d5 Mon Sep 17 00:00:00 2001 From: summerji Date: Wed, 7 Oct 2020 00:15:41 -0700 Subject: [PATCH 31/32] delete comment code --- .../engine/ast/GeneralForStatement.java | 31 ++++--------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index b6622996a1..bced6aad40 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -89,31 +89,12 @@ GeneralForStatement build() { localVarExpr.variable().identifier().name())); Preconditions.checkState(!localVarExpr.isStatic(), "Modifier 'static' not allow here."); } - // TODO (unsupport): Uncomment the following code if public setter for the initialization, - // termination, update expressions when needed. - // Preconditions.checkState( - // (initializationExpr instanceof MethodInvocationExpr) - // || (incrementExpr instanceof AssignmentExpr) - // || (incrementExpr instanceof AssignmentOperationExpr) - // // TODO(unsupported): Currently we only support postIncrement (i++), please add - // // postDecrement, prefixIncrement, prefixIncrement if needed. - // || (incrementExpr instanceof UnaryOperationExpr - // && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - // "Initialization expression %s must be either a method invocation, assignment, or unary - // post-fix operation expression."); - // Preconditions.checkState( - // terminationExpr.type().equals(TypeNode.BOOLEAN), - // "Terminal expression %s must be boolean-type expression."); - // Preconditions.checkState( - // (incrementExpr instanceof MethodInvocationExpr) - // || (incrementExpr instanceof AssignmentExpr) - // || (incrementExpr instanceof AssignmentOperationExpr) - // // TODO(unsupported): Currently we only support postIncrement (i++), please add - // // postDecrement, prefixIncrement, prefixIncrement if needed. - // || (incrementExpr instanceof UnaryOperationExpr - // && ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()), - // "Increment expression %s must be either a method invocation, assignment, or unary - // post-fix operation expression."); + // TODO (unsupport): Add type-checking for initialization, termination, update exprs if public + // setters for users for future needs. + // Initialization and update expr should belong to StatementExpressionList. + // Termination expr must have type boolean or Boolean. And these three exprs are optional. + // More details at + // https://docs.oracle.com/javase/specs/jls/se10/html/jls-14.html#jls-StatementExpressionList return autoBuild(); } } From 06dd281f2e536e02aa9da71dcbe5c642f2de9a4a Mon Sep 17 00:00:00 2001 From: summerji Date: Wed, 7 Oct 2020 21:09:18 -0700 Subject: [PATCH 32/32] Initialization is expr --- .../engine/ast/GeneralForStatement.java | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index bced6aad40..a1a9cb3073 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -22,7 +22,7 @@ @AutoValue public abstract class GeneralForStatement implements Statement { - public abstract AssignmentExpr initializationExpr(); + public abstract Expr initializationExpr(); public abstract Expr terminationExpr(); @@ -66,7 +66,7 @@ private static Builder builder() { @AutoValue.Builder abstract static class Builder { // Private setter. - abstract Builder setInitializationExpr(AssignmentExpr initializationExpr); + abstract Builder setInitializationExpr(Expr initializationExpr); // Private setter. abstract Builder setTerminationExpr(Expr terminationExpr); // Private setter. @@ -79,15 +79,18 @@ abstract static class Builder { // Type-checking will be done in the sub-expressions. GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); - VariableExpr localVarExpr = generalForStatement.initializationExpr().variableExpr(); - // Declare a variable inside for-loop initialization expression. - if (localVarExpr.isDecl()) { - Preconditions.checkState( - localVarExpr.scope().equals(ScopeNode.LOCAL), - String.format( - "Variable %s declare in a general for-loop cannot have a non-local scope", - localVarExpr.variable().identifier().name())); - Preconditions.checkState(!localVarExpr.isStatic(), "Modifier 'static' not allow here."); + Expr initExpr = generalForStatement.initializationExpr(); + if (initExpr instanceof AssignmentExpr) { + VariableExpr localVarExpr = ((AssignmentExpr) initExpr).variableExpr(); + // Declare a variable inside for-loop initialization expression. + if (localVarExpr.isDecl()) { + Preconditions.checkState( + localVarExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s declare in a general for-loop cannot have a non-local scope", + localVarExpr.variable().identifier().name())); + Preconditions.checkState(!localVarExpr.isStatic(), "Modifier 'static' not allow here."); + } } // TODO (unsupport): Add type-checking for initialization, termination, update exprs if public // setters for users for future needs.