CFG: Rename ForeachStmt to ForEachStmt. - #17
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: github#22489 Source head: 53f30a0
⛔ Shipwright · BlockedRecommendation: do not merge PR #17 · Tier
Findings (4)
Fireworks usage: 8,299 input · 498 output · 8,797 total tokens · $0.0022 · 8s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
|
|
||
| /** A for-loop that iterates over the elements of a collection. */ | ||
| class ForeachStmt extends LoopStmt { | ||
| class ForEachStmt extends LoopStmt { |
There was a problem hiding this comment.
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.
| final private class FinalForeachStmt = CS::ForeachStmt; | ||
|
|
||
| class ForeachStmt extends FinalForeachStmt { | ||
| class ForEachStmt extends FinalForeachStmt { |
There was a problem hiding this comment.
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.
| final private class FinalEnhancedForStmt = J::EnhancedForStmt; | ||
|
|
||
| class ForeachStmt extends FinalEnhancedForStmt { | ||
| class ForEachStmt extends FinalEnhancedForStmt { |
There was a problem hiding this comment.
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.
| } | ||
|
|
||
| class ForeachStmt extends LoopStmt instanceof U::ForEachStmt { | ||
| class ForEachStmt extends LoopStmt instanceof U::ForEachStmt { |
There was a problem hiding this comment.
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.
1 participant
This renames all the instances that are effectively private (everything except C#).
Source merge-base:
71cf2f1aec4a16d8581607265a8b32a15abeaa2eSource head:
53f30a0796ae179132d3a3601c5409eee4dce60d