From 4fcf3fbff8dc643c0d7b7410bf404bcb877bae12 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 4 Feb 2026 14:30:00 +0100 Subject: [PATCH] Java: Make loop classes extend LoopStmt and use getBody instead of getStmt. --- .../lib/semmle/code/java/ControlFlowGraph.qll | 12 +-- .../lib/semmle/code/java/PrettyPrintAst.qll | 30 +++---- java/ql/lib/semmle/code/java/Statement.qll | 82 +++++++++++-------- .../java/security/internal/ArraySizing.qll | 2 +- .../Likely Bugs/Termination/SpinOnField.ql | 6 +- 5 files changed, 73 insertions(+), 59 deletions(-) diff --git a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll index bb3690dbbfc..6d91788ba15 100644 --- a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll +++ b/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... diff --git a/java/ql/lib/semmle/code/java/PrettyPrintAst.qll b/java/ql/lib/semmle/code/java/PrettyPrintAst.qll index ac707c849dd..59a59cd9cdd 100644 --- a/java/ql/lib/semmle/code/java/PrettyPrintAst.qll +++ b/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 { diff --git a/java/ql/lib/semmle/code/java/Statement.qll b/java/ql/lib/semmle/code/java/Statement.qll index 3f138ac0fa2..d71a678d669 100644 --- a/java/ql/lib/semmle/code/java/Statement.qll +++ b/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) } - /** Gets the body of this enhanced `for` loop. */ - Stmt getStmt() { result.getParent() = this } + /** + * DEPRECATED: Use getBody() instead. + * + * Gets the body of this enhanced `for` loop. + */ + deprecated Stmt getStmt() { result.getParent() = this } - override string pp() { result = "for (... : ...) " + this.getStmt().pp() } + /** Gets the body of this enhanced `for` loop. */ + override Stmt getBody() { result.getParent() = this } + + 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 } - /** Gets the body of this `while` loop. */ - Stmt getStmt() { result.getParent() = this } + /** + * DEPRECATED: Use getBody() instead. + * + * Gets the body of this `while` loop. + */ + deprecated Stmt getStmt() { result.getParent() = this } - override string pp() { result = "while (...) " + this.getStmt().pp() } + /** Gets the body of this `while` loop. */ + override Stmt getBody() { result.getParent() = this } + + 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 } - /** Gets the body of this `do` loop. */ - Stmt getStmt() { result.getParent() = this } + /** + * DEPRECATED: Use getBody() instead. + * + * Gets the body of this `do` loop. + */ + deprecated Stmt getStmt() { result.getParent() = this } - override string pp() { result = "do " + this.getStmt().pp() + " while (...)" } + /** Gets the body of this `do` loop. */ + override Stmt getBody() { result.getParent() = this } + + 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. */ diff --git a/java/ql/lib/semmle/code/java/security/internal/ArraySizing.qll b/java/ql/lib/semmle/code/java/security/internal/ArraySizing.qll index 185b1b8a46e..ab9ac70e7c4 100644 --- a/java/ql/lib/semmle/code/java/security/internal/ArraySizing.qll +++ b/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) diff --git a/java/ql/src/Likely Bugs/Termination/SpinOnField.ql b/java/ql/src/Likely Bugs/Termination/SpinOnField.ql index 7cfb2308c96..1b4185531ca 100644 --- a/java/ql/src/Likely Bugs/Termination/SpinOnField.ql +++ b/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() {