Skip to content

CFG: Rename ForeachStmt to ForEachStmt. #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anurag6569201 wants to merge 1 commit into
base: qa/agent-github-codeql/pr-17-22489/base
Choose a base branch
from
Open

CFG: Rename ForeachStmt to ForEachStmt. #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraph.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ module Ast implements AstSig<Location> {

final private class FinalForeachStmt = CS::ForeachStmt;

class ForeachStmt extends FinalForeachStmt {
class ForEachStmt extends FinalForeachStmt {

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

The rename is applied inconsistently: the shared signature and Make0 module are updated, but the diff does not show updates to all internal references or to the C#/Java/Python/Ruby

Impact: The rename is applied inconsistently: the shared signature and Make0 module are updated, but the diff does not show updates to all internal references or to the C#/Java/Python/Ruby implementations' callers. If any reference to the old name remains outside the diff, the library will fail to compile. The empty check evidence index provides no proof that compilation or tests were run.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Expr getVariable() {
result = this.getVariableDeclExpr() or result = this.getVariableDeclTuple()
}
Expand Down
2 changes: 1 addition & 1 deletion java/ql/lib/semmle/code/java/ControlFlowGraph.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ private module Ast implements AstSig<Location> {

final private class FinalEnhancedForStmt = J::EnhancedForStmt;

class ForeachStmt extends FinalEnhancedForStmt {
class ForEachStmt extends FinalEnhancedForStmt {

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The rename from ForeachStmt to ForEachStmt is a cosmetic API change with no functional benefit shown.

Impact: The rename from ForeachStmt to ForEachStmt is a cosmetic API change with no functional benefit shown. It forces every consumer to update references and creates churn across six language libraries without a migration guide, deprecation notice, or changelog entry in the diff.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Expr getVariable() { result = super.getVariable() }

Expr getCollection() { result = super.getExpr() }
Expand Down
6 changes: 3 additions & 3 deletions python/ql/lib/semmle/python/controlflow/internal/AstNodeImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,10 +657,10 @@ module Ast implements AstSig<Py::Location> {
}

/** A for-each loop (`for x in iterable:`). */
class ForeachStmt extends LoopStmtImpl {
class ForEachStmt extends LoopStmtImpl {
private Py::For forStmt;

ForeachStmt() { this = TPyStmt(forStmt) }
ForEachStmt() { this = TPyStmt(forStmt) }

/** Gets the loop variable. */
Expr getVariable() { result.asExpr() = forStmt.getTarget() }
Expand Down Expand Up @@ -898,7 +898,7 @@ module Ast implements AstSig<Py::Location> {
AstNode getLoopElse(LoopStmt loop) {
result = loop.(WhileStmt).getElse()
or
result = loop.(ForeachStmt).getElse()
result = loop.(ForEachStmt).getElse()
}

/** An exception handler (`except` or `except*`). */
Expand Down
6 changes: 3 additions & 3 deletions ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,9 @@ private module Ast implements AstSig<Location> {
AstNode getUpdate(int index) { none() }
}

// `ForExpr` would be a match for `ForeachStmt`, but it is desugared.
class ForeachStmt extends LoopStmt {
ForeachStmt() { none() }
// `ForExpr` would be a match for `ForEachStmt`, but it is desugared.
class ForEachStmt extends LoopStmt {
ForEachStmt() { none() }

Expr getVariable() { none() }

Expand Down
8 changes: 4 additions & 4 deletions shared/controlflow/codeql/controlflow/ControlFlowGraph.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ signature module AstSig<LocationSig Location> {
}

/** A for-loop that iterates over the elements of a collection. */
class ForeachStmt extends LoopStmt {
class ForEachStmt extends LoopStmt {

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

This is a cross-language public API rename (ForeachStmt -> ForEachStmt) across C#, Java, Python, Ruby, shared, and unified CFG libraries with zero evidence of query migration, comp

Impact: This is a cross-language public API rename (ForeachStmt -> ForEachStmt) across C#, Java, Python, Ruby, shared, and unified CFG libraries with zero evidence of query migration, compatibility aliases, or deprecation shims. Any existing query or downstream library referencing ForeachStmt will fail to compile or resolve, breaking the entire CodeQL query suite for all affected languages.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

/** Gets the variable declaration of this `foreach` loop. */
Expr getVariable();

Expand Down Expand Up @@ -650,7 +650,7 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
any(Case case).getGuard() = n
)
or
any(ForeachStmt foreachstmt).getCollection() = n and kind.isEmptiness()
any(ForEachStmt foreachstmt).getCollection() = n and kind.isEmptiness()
or
kind.isMatching() and
(
Expand Down Expand Up @@ -1656,7 +1656,7 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
n2.isAfter(loopstmt)
)
or
exists(ForeachStmt foreachstmt |
exists(ForEachStmt foreachstmt |
n1.isBefore(foreachstmt) and
n2.isBefore(foreachstmt.getCollection())
or
Expand Down Expand Up @@ -2401,7 +2401,7 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
// allow for loop headers in foreach loops (they're checking emptiness on the iterator, not the collection)
not (
t instanceof DirectSuccessor and
node.isAdditional(any(ForeachStmt foreach), loopHeaderTag())
node.isAdditional(any(ForEachStmt foreach), loopHeaderTag())
) and
// allow for functions with multiple bodies
not exists(Callable c |
Expand Down
2 changes: 1 addition & 1 deletion unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private module Ast implements AstSig<Location> {
AstNode getUpdate(int index) { none() }
}

class ForeachStmt extends LoopStmt instanceof U::ForEachStmt {
class ForEachStmt extends LoopStmt instanceof U::ForEachStmt {

@shipwright-agent shipwright-agent Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The unified CFG implementation now extends U::ForEachStmt but the diff does not show whether the unified AST's ForEachStmt exposes the same getVariable/getCollection contract requi

Impact: The unified CFG implementation now extends U::ForEachStmt but the diff does not show whether the unified AST's ForEachStmt exposes the same getVariable/getCollection contract required by the shared signature. A mismatch could cause silent empty results in CFG-based security queries (e.g., taint tracking through foreach loops), leading to false negatives in vulnerability detection.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

override Stmt getBody() { result = U::ForEachStmt.super.getBody() }

// TODO support foreach guard
Expand Down