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..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 @@ -24,17 +24,9 @@ public abstract class GeneralForStatement implements Statement { public abstract Expr 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. public abstract Expr terminationExpr(); - public abstract Expr incrementExpr(); - */ - public abstract VariableExpr localVariableExpr(); - - public abstract Expr maxSizeExpr(); + public abstract Expr updateExpr(); public abstract ImmutableList body(); @@ -43,60 +35,69 @@ 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 (unsupported): 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)) + .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 - public abstract static class Builder { - public abstract Builder setInitializationExpr(Expr initializationExpr); - - public abstract Builder setBody(List body); - - // Private. - abstract Builder setLocalVariableExpr(VariableExpr variableExpr); - - abstract Builder setMaxSizeExpr(Expr maxSizeExpr); - - abstract VariableExpr localVariableExpr(); + abstract static class Builder { + // Private setter. + abstract Builder setInitializationExpr(Expr initializationExpr); + // Private setter. + abstract Builder setTerminationExpr(Expr terminationExpr); + // Private setter. + abstract Builder setUpdateExpr(Expr incrementExpr); + // Private setter. + abstract Builder setBody(List body); abstract GeneralForStatement autoBuild(); // Type-checking will be done in the sub-expressions. - public GeneralForStatement build() { - VariableExpr varExpr = localVariableExpr(); - 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())); - 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()); + GeneralForStatement build() { + GeneralForStatement generalForStatement = autoBuild(); + 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. + // 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(); } } 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..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 @@ -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.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 538d4a62b0..431b125594 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.updateExpr().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..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 @@ -21,57 +21,178 @@ import org.junit.Test; 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 = 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 + // 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 = 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 + // 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 = 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").setReturnType(TypeNode.INT).build(); + + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); + } + @Test + // 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() + .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").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList()); + } - GeneralForStatement.incrementWith(variableExpr, 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() { - Variable variable = Variable.builder().setName("str").setType(TypeNode.STRING).build(); + // 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() + .setIsDecl(true) + .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 + // 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.builder().setVariable(variable).setIsDecl(true).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(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); + } + + @Test + // 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.builder() + .setVariable(variable) + .setIsDecl(false) + .setScope(ScopeNode.PUBLIC) + .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())); + } + + @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).setScope(ScopeNode.PRIVATE).build(); + 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").build(); + MethodInvocationExpr.builder().setMethodName("maxSize").setReturnType(TypeNode.INT).build(); assertThrows( IllegalStateException.class, () -> - GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList())); + GeneralForStatement.incrementWith( + variableExpr, initValue, maxSizeExpr, Collections.emptyList())); } 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);