Normalize if/else blocks

dev/core/src/com/google/gwt/dev/jjs/ast/JBlock.java

@@ -33,6 +33,17 @@ public JBlock(SourceInfo info, JStatement... statements) {
     this.statements.addAll(Arrays.asList(statements));
   }
 
+  public static JBlock ensureBlock(SourceInfo info, JStatement statement) {
+    if (statement == null) {
+      return new JBlock(info);
+    }
+    if (statement instanceof JBlock block) {
+      return block;
+    }
+
+    return new JBlock(statement.getSourceInfo(), statement);
+  }
+
   /**
    * Insert a statement into this block.
    */
@@ -105,4 +116,32 @@ public boolean unconditionalControlBreak() {
     }
     return false;
   }
+
+  /**
+   * If possible, expresses this block as a single statement.
+   * <ul>
+   *   <li>If the block is empty, return null</li>
+   *   <li>If there are multiple items in this block, return the block</li>
+   *   <li>If the only item is another block, invoke {@code singleStatement()} on it</li>
+   *   <li>Otherwise, return the only item as is.</li>
+   * </ul>
+   *
+   * @return the contents of this block as a single nullable statement
+   */
+  public JStatement singleStatement() {
+    if (statements.isEmpty()) {
+      return null;
+    }
+
+    if (statements.size() > 1) {
+      return this;
+    }
+
+    JStatement onlyStatement = statements.get(0);
+    if (onlyStatement instanceof JBlock block) {
+      return block.singleStatement();
+    }
+
+    return onlyStatement;
+  }
 }

dev/core/src/com/google/gwt/dev/jjs/ast/JDoStatement.java

@@ -22,16 +22,16 @@
  */
 public class JDoStatement extends JStatement {
 
-  private JStatement body;
+  private JBlock body;
   private JExpression testExpr;
 
   public JDoStatement(SourceInfo info, JExpression testExpr, JStatement body) {
     super(info);
     this.testExpr = testExpr;
-    this.body = body;
+    this.body = JBlock.ensureBlock(info, body);
   }
 
-  public JStatement getBody() {
+  public JBlock getBody() {
     return body;
   }
 
@@ -43,9 +43,7 @@ public JExpression getTestExpr() {
   public void traverse(JVisitor visitor, Context ctx) {
     if (visitor.visit(this, ctx)) {
       testExpr = visitor.accept(testExpr);
-      if (body != null) {
-        body = visitor.accept(body, true);
-      }
+      body = JBlock.ensureBlock(getSourceInfo(), visitor.accept(body, false));
     }
     visitor.endVisit(this, ctx);
   }

dev/core/src/com/google/gwt/dev/jjs/ast/JForStatement.java

@@ -25,7 +25,7 @@
  */
 public class JForStatement extends JStatement {
 
-  private JStatement body;
+  private JBlock body;
   private List<JStatement> initializers;
   private JExpression condition;
   private JExpression increments;
@@ -40,13 +40,13 @@
     this.initializers = Lists.newArrayList(initializers);
     this.condition = condition;
     this.increments = increments;
-    this.body = body;
+    this.body = JBlock.ensureBlock(info, body);
   }
 
   /**
    * Returns the {@code for} statement body.
    */
-  public JStatement getBody() {
+  public JBlock getBody() {
     return body;
   }
 
@@ -81,9 +81,7 @@
       if (increments != null) {
         increments = visitor.accept(increments);
       }
-      if (body != null) {
-        body = visitor.accept(body, true);
-      }
+      body = JBlock.ensureBlock(getSourceInfo(), visitor.accept(body, false));//TODO no tests fail without this change...
     }
     visitor.endVisit(this, ctx);
   }

dev/core/src/com/google/gwt/dev/jjs/ast/JIfStatement.java

@@ -22,39 +22,35 @@
  */
 public class JIfStatement extends JStatement {
 
-  private JStatement elseStmt;
+  private JBlock elseStmt;
   private JExpression ifExpr;
-  private JStatement thenStmt;
+  private JBlock thenStmt;
 
   public JIfStatement(SourceInfo info, JExpression ifExpr, JStatement thenStmt, JStatement elseStmt) {
     super(info);
     this.ifExpr = ifExpr;
-    this.thenStmt = thenStmt;
-    this.elseStmt = elseStmt;
+    this.thenStmt = JBlock.ensureBlock(info, thenStmt);
+    this.elseStmt = JBlock.ensureBlock(info, elseStmt);
   }
 
-  public JStatement getElseStmt() {
+  public JBlock getElseStmt() {
     return elseStmt;
   }
 
   public JExpression getIfExpr() {
     return ifExpr;
   }
 
-  public JStatement getThenStmt() {
+  public JBlock getThenStmt() {
     return thenStmt;
   }
 
   @Override
   public void traverse(JVisitor visitor, Context ctx) {
     if (visitor.visit(this, ctx)) {
       ifExpr = visitor.accept(ifExpr);
-      if (thenStmt != null) {
-        thenStmt = visitor.accept(thenStmt, true);
-      }
-      if (elseStmt != null) {
-        elseStmt = visitor.accept(elseStmt, true);
-      }
+      thenStmt = JBlock.ensureBlock(getSourceInfo(), visitor.accept(thenStmt, false));
+      elseStmt = JBlock.ensureBlock(getSourceInfo(), visitor.accept(elseStmt, false));
     }
     visitor.endVisit(this, ctx);
   }

dev/core/src/com/google/gwt/dev/jjs/ast/JWhileStatement.java

@@ -22,16 +22,16 @@
  */
 public class JWhileStatement extends JStatement {
 
-  private JStatement body;
+  private JBlock body;
   private JExpression testExpr;
 
   public JWhileStatement(SourceInfo info, JExpression testExpr, JStatement body) {
     super(info);
     this.testExpr = testExpr;
-    this.body = body;
+    this.body = JBlock.ensureBlock(info, body);
   }
 
-  public JStatement getBody() {
+  public JBlock getBody() {
     return body;
   }
 
@@ -43,9 +43,7 @@
   public void traverse(JVisitor visitor, Context ctx) {
     if (visitor.visit(this, ctx)) {
       testExpr = visitor.accept(testExpr);
-      if (body != null) {
-        body = visitor.accept(body, true);
-      }
+      body = JBlock.ensureBlock(getSourceInfo(), visitor.accept(body, false));//TODO no tests fail without this change...
     }
     visitor.endVisit(this, ctx);
   }

dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java

@@ -336,9 +336,9 @@ public void endVisit(JDoStatement x, Context ctx) {
       if (expression instanceof JBooleanLiteral) {
         JBooleanLiteral booleanLiteral = (JBooleanLiteral) expression;
 
-        // If false, replace do with do's body
         if (!booleanLiteral.getValue()) {
-          if (JjsUtils.isEmptyBlock(x.getBody())) {
+          // If false, replace do with do's body
+          if (x.getBody().isEmpty()) {
             ctx.removeMe();
           } else { // Unless it contains break/continue statements
             FindBreakContinueStatementsVisitor visitor = new FindBreakContinueStatementsVisitor();
@@ -1964,10 +1964,8 @@ private void tryRemoveSwitch(JSwitchStatement x, Context ctx) {
         } else {
           // Create an if statement equivalent to the single-case switch.
           JBinaryOperation compareOperation = caseStatement.convertToCompareExpression(x.getExpr());
-          JBlock block = new JBlock(x.getSourceInfo());
-          block.addStmt(statement);
           JIfStatement ifStatement =
-              new JIfStatement(x.getSourceInfo(), compareOperation, block, null);
+              new JIfStatement(x.getSourceInfo(), compareOperation, statement, null);
           replaceMe(ifStatement, ctx);
         }
       }

dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java

@@ -574,7 +574,7 @@ public JsExpression transformBinaryOperation(JBinaryOperation binaryOperation) {
     }
 
     @Override
-    public JsStatement transformBlock(JBlock block) {
+    public JsBlock transformBlock(JBlock block) {
       JsBlock jsBlock = new JsBlock(block.getSourceInfo());
       List<JsStatement> stmts = jsBlock.getStatements();
 
@@ -692,7 +692,8 @@ public JsNode transformDeclarationStatement(JDeclarationStatement declarationSta
     public JsNode transformDoStatement(JDoStatement doStatement) {
       JsDoWhile stmt = new JsDoWhile(doStatement.getSourceInfo());
       stmt.setCondition(transform(doStatement.getTestExpr()));
-      stmt.setBody(jsEmptyIfNull(doStatement.getSourceInfo(), transform(doStatement.getBody())));
+      stmt.setBody(jsEmptyIfNull(doStatement.getSourceInfo(),
+          transform(doStatement.getBody().singleStatement())));
       return stmt;
     }
 
@@ -742,7 +743,8 @@ public JsNode transformForStatement(JForStatement forStatement) {
       result.setInitExpr(initExpr);
       result.setCondition(transform(forStatement.getCondition()));
       result.setIncrExpr(transform(forStatement.getIncrements()));
-      result.setBody(jsEmptyIfNull(forStatement.getSourceInfo(), transform(forStatement.getBody())));
+      result.setBody(jsEmptyIfNull(forStatement.getSourceInfo(),
+          transform(forStatement.getBody().singleStatement())));
 
       return result;
     }
@@ -753,8 +755,8 @@ public JsNode transformIfStatement(JIfStatement ifStatement) {
 
       result.setIfExpr(transform(ifStatement.getIfExpr()));
       result.setThenStmt(jsEmptyIfNull(ifStatement.getSourceInfo(),
-          transform(ifStatement.getThenStmt())));
-      result.setElseStmt(transform(ifStatement.getElseStmt()));
+          transform(ifStatement.getThenStmt().singleStatement())));
+      result.setElseStmt(transform(ifStatement.getElseStmt().singleStatement()));
 
       return result;
     }
@@ -1168,7 +1170,7 @@ public JsNode transformWhileStatement(JWhileStatement whileStatement) {
       SourceInfo info = whileStatement.getSourceInfo();
       JsWhile stmt = new JsWhile(info);
       stmt.setCondition(transform(whileStatement.getTestExpr()));
-      stmt.setBody(jsEmptyIfNull(info, transform(whileStatement.getBody())));
+      stmt.setBody(jsEmptyIfNull(info, transform(whileStatement.getBody().singleStatement())));
       return stmt;
     }
 

dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java

@@ -1093,8 +1093,8 @@ public JExpression apply(JStatement statement) {
     public void endVisit(IfStatement x, BlockScope scope) {
       try {
         SourceInfo info = makeSourceInfo(x);
-        JStatement elseStatement = pop(x.elseStatement);
-        JStatement thenStatement = pop(x.thenStatement);
+        JBlock elseStatement = popBlock(info, x.elseStatement);
+        JBlock thenStatement = popBlock(info, x.thenStatement);
         JExpression condition = pop(x.condition);
         push(new JIfStatement(info, condition, thenStatement, elseStatement));
       } catch (Throwable e) {

dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java

@@ -240,8 +240,8 @@ public static JExpression simplifyConditional(JConditional expression) {
   public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType methodReturnType) {
     SourceInfo info = ifStatement.getSourceInfo();
     JExpression conditionExpression = ifStatement.getIfExpr();
-    JStatement thenStmt = ifStatement.getThenStmt();
-    JStatement elseStmt = ifStatement.getElseStmt();
+    final JBlock thenStmt = ifStatement.getThenStmt();
+    final JBlock elseStmt = ifStatement.getElseStmt();
     if (conditionExpression instanceof JMultiExpression) {
       // if(a,b,c) d else e -> {a; b; if(c) d else e; }
       JMultiExpression condMulti = (JMultiExpression) conditionExpression;
@@ -258,10 +258,10 @@ public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType met
 
     if (conditionExpression instanceof JBooleanLiteral) {
       boolean conditionValue = ((JBooleanLiteral) conditionExpression).getValue();
-      if (conditionValue && !JjsUtils.isEmptyBlock(thenStmt)) {
+      if (conditionValue && !thenStmt.isEmpty()) {
         // If true, replace myself with then statement
         return thenStmt;
-      } else if (!conditionValue && !JjsUtils.isEmptyBlock(elseStmt)) {
+      } else if (!conditionValue && !elseStmt.isEmpty()) {
         // If false, replace myself with else statement
         return elseStmt;
       } else {
@@ -270,19 +270,15 @@ public static JStatement simplifyIfStatement(JIfStatement ifStatement, JType met
       }
     }
 
-    if (JjsUtils.isEmptyBlock(thenStmt) && JjsUtils.isEmptyBlock(elseStmt)) {
+    if (thenStmt.isEmpty() && elseStmt.isEmpty()) {
       return conditionExpression.makeStatement();
     }
 
-    if (!JjsUtils.isEmptyBlock(elseStmt)) {
+    if (!elseStmt.isEmpty()) {
       // if (!cond) foo else bar -> if (cond) bar else foo
       JExpression negationArugment =
           Simplifier.maybeGetNegatedExpressionArgument(conditionExpression);
       if (negationArugment != null) {
-        // Force sub-parts to blocks, otherwise we break else-if chains.
-        // TODO: this goes away when we normalize the Java AST properly.
-        thenStmt = ensureBlock(thenStmt);
-        elseStmt = ensureBlock(elseStmt);
         return simplifyIfStatement(
             new JIfStatement(info, negationArugment, elseStmt, thenStmt), methodReturnType);
       }
@@ -520,22 +516,11 @@ private static JExpression extractExpression(JStatement statement) {
     return null;
   }
 
-  private static JStatement extractSingleStatement(JStatement statement) {
-    if (statement instanceof JBlock) {
-      JBlock block = (JBlock) statement;
-      if (block.getStatements().size() == 1) {
-        return extractSingleStatement(block.getStatements().get(0));
-      }
-    }
-
-    return statement;
-  }
-
   private static JStatement rewriteIfStatementAsExpression(SourceInfo sourceInfo,
-      JExpression conditionExpression, JStatement thenStmt, JStatement elseStmt,
+      JExpression conditionExpression, JBlock thenBlock, JBlock elseBlock,
       JType methodReturnType) {
-    thenStmt = extractSingleStatement(thenStmt);
-    elseStmt = extractSingleStatement(elseStmt);
+    JStatement thenStmt = thenBlock.singleStatement();
+    JStatement elseStmt = elseBlock.singleStatement();
 
     if (thenStmt instanceof JReturnStatement && elseStmt instanceof JReturnStatement
         && methodReturnType != null) {