Java: Make loop classes extend LoopStmt and use getBody instead of getStmt.

Author: aschackmull

java/ql/lib/semmle/code/java/ControlFlowGraph.qll

@@ -1427,7 +1427,7 @@ private module ControlFlowGraphImpl {
       condentry = first(for.getCondition())
       or
       // ...or the body if the for doesn't include a condition.
-      not exists(for.getCondition()) and condentry = first(for.getStmt())
+      not exists(for.getCondition()) and condentry = first(for.getBody())
     |
       // From the entry point, which is the for statement itself, control goes to either the first init expression...
       n.asStmt() = for and result = first(for.getInit(0)) and completion = NormalCompletion()
@@ -1448,7 +1448,7 @@ private module ControlFlowGraphImpl {
       // The true-successor of the condition is the body of the for loop.
       last(for.getCondition(), n, completion) and
       completion = BooleanCompletion(true, _) and
-      result = first(for.getStmt())
+      result = first(for.getBody())
       or
       // The updates execute sequentially, after which control is transferred to the condition.
       exists(int i | last(for.getUpdate(i), n, completion) and completion = NormalCompletion() |
@@ -1458,7 +1458,7 @@ private module ControlFlowGraphImpl {
       )
       or
       // The back edge of the loop: control goes to either the first update or the condition if no updates exist.
-      last(for.getStmt(), n, completion) and
+      last(for.getBody(), n, completion) and
       continues(completion, for) and
       (
         result = first(for.getUpdate(0))
@@ -1479,11 +1479,11 @@ private module ControlFlowGraphImpl {
       or
       // ...and then control goes to the body of the loop.
       n.asExpr() = for.getVariable() and
-      result = first(for.getStmt()) and
+      result = first(for.getBody()) and
       completion = NormalCompletion()
       or
       // Finally, the back edge of the loop goes to reassign the variable.
-      last(for.getStmt(), n, completion) and
+      last(for.getBody(), n, completion) and
       continues(completion, for) and
       result.asExpr() = for.getVariable()
     )
@@ -1492,7 +1492,7 @@ private module ControlFlowGraphImpl {
     result = first(n.asStmt().(WhileStmt).getCondition()) and completion = NormalCompletion()
     or
     // ...and do-while loops start at the body.
-    result = first(n.asStmt().(DoStmt).getStmt()) and completion = NormalCompletion()
+    result = first(n.asStmt().(DoStmt).getBody()) and completion = NormalCompletion()
     or
     exists(LoopStmt loop | loop instanceof WhileStmt or loop instanceof DoStmt |
       // Control goes from the condition via a true-completion to the body...

java/ql/lib/semmle/code/java/PrettyPrintAst.qll

@@ -577,7 +577,7 @@ private class PpForStmt extends PpAst, ForStmt {
     or
     i = 1 + this.lastUpdateIndex() and result = ")"
     or
-    i = 2 + this.lastUpdateIndex() and result = " " and this.getStmt() instanceof BlockStmt
+    i = 2 + this.lastUpdateIndex() and result = " " and this.getBody() instanceof BlockStmt
   }
 
   private int lastInitIndex() { result = 3 + 2 * max(int j | exists(this.getInit(j))) }
@@ -587,7 +587,7 @@ private class PpForStmt extends PpAst, ForStmt {
   }
 
   override predicate newline(int i) {
-    i = 2 + this.lastUpdateIndex() and not this.getStmt() instanceof BlockStmt
+    i = 2 + this.lastUpdateIndex() and not this.getBody() instanceof BlockStmt
   }
 
   override PpAst getChild(int i) {
@@ -599,11 +599,11 @@ private class PpForStmt extends PpAst, ForStmt {
     or
     exists(int j | result = this.getUpdate(j) and i = 4 + this.lastInitIndex() + 2 * j)
     or
-    i = 3 + this.lastUpdateIndex() and result = this.getStmt()
+    i = 3 + this.lastUpdateIndex() and result = this.getBody()
   }
 
   override predicate indents(int i) {
-    i = 3 + this.lastUpdateIndex() and not this.getStmt() instanceof BlockStmt
+    i = 3 + this.lastUpdateIndex() and not this.getBody() instanceof BlockStmt
   }
 }
 
@@ -616,7 +616,7 @@ private class PpEnhancedForStmt extends PpAst, EnhancedForStmt {
     i = 4 and result = " : "
     or
     i = 6 and
-    if this.getStmt() instanceof BlockStmt then result = ") " else result = ")"
+    if this.getBody() instanceof BlockStmt then result = ") " else result = ")"
   }
 
   override PpAst getChild(int i) {
@@ -626,10 +626,10 @@ private class PpEnhancedForStmt extends PpAst, EnhancedForStmt {
     or
     i = 5 and result = this.getExpr()
     or
-    i = 7 and result = this.getStmt()
+    i = 7 and result = this.getBody()
   }
 
-  override predicate indents(int i) { i = 7 and not this.getStmt() instanceof BlockStmt }
+  override predicate indents(int i) { i = 7 and not this.getBody() instanceof BlockStmt }
 }
 
 private class PpWhileStmt extends PpAst, WhileStmt {
@@ -638,40 +638,40 @@ private class PpWhileStmt extends PpAst, WhileStmt {
     or
     i = 2 and result = ")"
     or
-    i = 3 and result = " " and this.getStmt() instanceof BlockStmt
+    i = 3 and result = " " and this.getBody() instanceof BlockStmt
   }
 
-  override predicate newline(int i) { i = 3 and not this.getStmt() instanceof BlockStmt }
+  override predicate newline(int i) { i = 3 and not this.getBody() instanceof BlockStmt }
 
   override PpAst getChild(int i) {
     i = 1 and result = this.getCondition()
     or
-    i = 4 and result = this.getStmt()
+    i = 4 and result = this.getBody()
   }
 
-  override predicate indents(int i) { i = 4 and not this.getStmt() instanceof BlockStmt }
+  override predicate indents(int i) { i = 4 and not this.getBody() instanceof BlockStmt }
 }
 
 private class PpDoStmt extends PpAst, DoStmt {
   override string getPart(int i) {
     i = 0 and result = "do"
     or
-    i in [1, 3] and result = " " and this.getStmt() instanceof BlockStmt
+    i in [1, 3] and result = " " and this.getBody() instanceof BlockStmt
     or
     i = 4 and result = "while ("
     or
     i = 6 and result = ");"
   }
 
-  override predicate newline(int i) { i in [1, 3] and not this.getStmt() instanceof BlockStmt }
+  override predicate newline(int i) { i in [1, 3] and not this.getBody() instanceof BlockStmt }
 
   override PpAst getChild(int i) {
-    i = 2 and result = this.getStmt()
+    i = 2 and result = this.getBody()
     or
     i = 5 and result = this.getCondition()
   }
 
-  override predicate indents(int i) { i = 2 and not this.getStmt() instanceof BlockStmt }
+  override predicate indents(int i) { i = 2 and not this.getBody() instanceof BlockStmt }
 }
 
 private class PpTryStmt extends PpAst, TryStmt {

java/ql/lib/semmle/code/java/Statement.qll

@@ -140,7 +140,7 @@ class IfStmt extends ConditionalStmt, @ifstmt {
 }
 
 /** A `for` loop. */
-class ForStmt extends ConditionalStmt, @forstmt {
+class ForStmt extends ConditionalStmt, LoopStmtImpl, @forstmt {
   /**
    * Gets an initializer expression of the loop.
    *
@@ -167,8 +167,15 @@ class ForStmt extends ConditionalStmt, @forstmt {
     index = result.getIndex() - 3
   }
 
+  /**
+   * DEPRECATED: Use getBody() instead.
+   *
+   * Gets the body of this `for` loop.
+   */
+  deprecated Stmt getStmt() { result.getParent() = this and result.getIndex() = 2 }
+
   /** Gets the body of this `for` loop. */
-  Stmt getStmt() { result.getParent() = this and result.getIndex() = 2 }
+  override Stmt getBody() { result.getParent() = this and result.getIndex() = 2 }
 
   /**
    * Gets a variable that is used as an iteration variable: it is defined,
@@ -191,7 +198,7 @@ class ForStmt extends ConditionalStmt, @forstmt {
     this.getCondition().getAChildExpr*() = result.getAnAccess()
   }
 
-  override string pp() { result = "for (...;...;...) " + this.getStmt().pp() }
+  override string pp() { result = "for (...;...;...) " + this.getBody().pp() }
 
   override string toString() { result = "for (...;...;...)" }
 
@@ -201,17 +208,24 @@ class ForStmt extends ConditionalStmt, @forstmt {
 }
 
 /** An enhanced `for` loop. (Introduced in Java 5.) */
-class EnhancedForStmt extends Stmt, @enhancedforstmt {
+class EnhancedForStmt extends LoopStmtImpl, @enhancedforstmt {
   /** Gets the local variable declaration expression of this enhanced `for` loop. */
   LocalVariableDeclExpr getVariable() { result.getParent() = this }
 
   /** Gets the expression over which this enhanced `for` loop iterates. */
   Expr getExpr() { result.isNthChildOf(this, 1) }
 
+  /**
+   * DEPRECATED: Use getBody() instead.
+   *
+   * Gets the body of this enhanced `for` loop.
+   */
+  deprecated Stmt getStmt() { result.getParent() = this }
+
   /** Gets the body of this enhanced `for` loop. */
-  Stmt getStmt() { result.getParent() = this }
+  override Stmt getBody() { result.getParent() = this }
 
-  override string pp() { result = "for (... : ...) " + this.getStmt().pp() }
+  override string pp() { result = "for (... : ...) " + this.getBody().pp() }
 
   override string toString() { result = "for (... : ...)" }
 
@@ -221,14 +235,21 @@ class EnhancedForStmt extends Stmt, @enhancedforstmt {
 }
 
 /** A `while` loop. */
-class WhileStmt extends ConditionalStmt, @whilestmt {
+class WhileStmt extends ConditionalStmt, LoopStmtImpl, @whilestmt {
   /** Gets the boolean condition of this `while` loop. */
   override Expr getCondition() { result.getParent() = this }
 
+  /**
+   * DEPRECATED: Use getBody() instead.
+   *
+   * Gets the body of this `while` loop.
+   */
+  deprecated Stmt getStmt() { result.getParent() = this }
+
   /** Gets the body of this `while` loop. */
-  Stmt getStmt() { result.getParent() = this }
+  override Stmt getBody() { result.getParent() = this }
 
-  override string pp() { result = "while (...) " + this.getStmt().pp() }
+  override string pp() { result = "while (...) " + this.getBody().pp() }
 
   override string toString() { result = "while (...)" }
 
@@ -238,14 +259,21 @@ class WhileStmt extends ConditionalStmt, @whilestmt {
 }
 
 /** A `do` loop. */
-class DoStmt extends ConditionalStmt, @dostmt {
+class DoStmt extends ConditionalStmt, LoopStmtImpl, @dostmt {
   /** Gets the condition of this `do` loop. */
   override Expr getCondition() { result.getParent() = this }
 
+  /**
+   * DEPRECATED: Use getBody() instead.
+   *
+   * Gets the body of this `do` loop.
+   */
+  deprecated Stmt getStmt() { result.getParent() = this }
+
   /** Gets the body of this `do` loop. */
-  Stmt getStmt() { result.getParent() = this }
+  override Stmt getBody() { result.getParent() = this }
 
-  override string pp() { result = "do " + this.getStmt().pp() + " while (...)" }
+  override string pp() { result = "do " + this.getBody().pp() + " while (...)" }
 
   override string toString() { result = "do ... while (...)" }
 
@@ -258,30 +286,16 @@ class DoStmt extends ConditionalStmt, @dostmt {
  * A loop statement, including `for`, enhanced `for`,
  * `while` and `do` statements.
  */
-class LoopStmt extends Stmt {
-  LoopStmt() {
-    this instanceof ForStmt or
-    this instanceof EnhancedForStmt or
-    this instanceof WhileStmt or
-    this instanceof DoStmt
-  }
-
+abstract private class LoopStmtImpl extends Stmt {
   /** Gets the body of this loop statement. */
-  Stmt getBody() {
-    result = this.(ForStmt).getStmt() or
-    result = this.(EnhancedForStmt).getStmt() or
-    result = this.(WhileStmt).getStmt() or
-    result = this.(DoStmt).getStmt()
-  }
+  abstract Stmt getBody();
 
   /** Gets the boolean condition of this loop statement. */
-  Expr getCondition() {
-    result = this.(ForStmt).getCondition() or
-    result = this.(WhileStmt).getCondition() or
-    result = this.(DoStmt).getCondition()
-  }
+  Expr getCondition() { none() }
 }
 
+final class LoopStmt = LoopStmtImpl;
+
 /** A `try` statement. */
 class TryStmt extends Stmt, @trystmt {
   /** Gets the block of the `try` statement. */

java/ql/lib/semmle/code/java/security/internal/ArraySizing.qll

@@ -49,7 +49,7 @@ class PointlessLoop extends WhileStmt {
     this.getCondition().(BooleanLiteral).getBooleanValue() = true and
     // The only `break` must be the last statement.
     forall(BreakStmt break | break.getTarget() = this |
-      this.getStmt().(BlockStmt).getLastStmt() = break
+      this.getBody().(BlockStmt).getLastStmt() = break
     ) and
     // No `continue` statements.
     not exists(ContinueStmt continue | continue.getTarget() = this)

java/ql/src/Likely Bugs/Termination/SpinOnField.ql

@@ -37,12 +37,12 @@ class EmptyLoop extends Stmt {
     exists(ForStmt stmt | stmt = this |
       not exists(stmt.getAnInit()) and
       not exists(stmt.getAnUpdate()) and
-      stmt.getStmt() instanceof Empty
+      stmt.getBody() instanceof Empty
     )
     or
-    this.(WhileStmt).getStmt() instanceof Empty
+    this.(WhileStmt).getBody() instanceof Empty
     or
-    this.(DoStmt).getStmt() instanceof Empty
+    this.(DoStmt).getBody() instanceof Empty
   }
 
   Expr getCondition() {