[operators][integrate]feat!: Integrate OperationExpr into GeneralForStatement

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<Statement> 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<Statement> body) {
-    // TODO(summerji): Do some integration here, in JavaWriterVisitor, in ImportWriterVisitor, and
-    // add more tests.
+      VariableExpr localVariableExpr,
+      ValueExpr initialValueExpr,
+      Expr maxSizeExpr,
+      List<Statement> 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<Statement> 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<Statement> 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();
     }
   }

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());
   }
 

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();

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))

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() {