C#: Rename ForeachStmt to ForEachStmt. - #12
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: github#22492 Source head: a5e733c
⛔ Shipwright · BlockedRecommendation: do not merge PR #12 · Tier
Findings (5)
Fireworks usage: 23,197 input · 911 output · 24,108 total tokens · $0.0057 · 17s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
| override string getAPrimaryQlClass() { result = "ForStmt" } | ||
| } | ||
|
|
||
| /** DEPRECATED: Use `ForEachStmt` instead. */ |
There was a problem hiding this comment.
Shipwright · CRITICAL
The deprecated alias 'ForeachStmt' is defined as 'deprecated class ForeachStmt = ForEachStmt;' in Stmt.qll, but the new class 'ForEachStmt' overrides 'getAPrimaryQlClass()' to retu
Impact: The deprecated alias 'ForeachStmt' is defined as 'deprecated class ForeachStmt = ForEachStmt;' in Stmt.qll, but the new class 'ForEachStmt' overrides 'getAPrimaryQlClass()' to return "ForEachStmt". Any existing query or library that still references the deprecated 'ForeachStmt' name and relies on the primary QL class string "ForeachStmt" (e.g., for AST printing, dbscheme mapping, or query result classification) will…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| ) | ||
| } | ||
|
|
||
| /** DEPRECATED: Use `ForEachStmtGenericEnumerable` instead. */ |
There was a problem hiding this comment.
Shipwright · CRITICAL
The deprecated aliases in Linq/Helpers.qll are declared as 'deprecated class ForeachStmtGenericEnumerable = ForEachStmtGenericEnumerable;' and 'deprecated class ForeachStmtEnumerab
Impact: The deprecated aliases in Linq/Helpers.qll are declared as 'deprecated class ForeachStmtGenericEnumerable = ForEachStmtGenericEnumerable;' and 'deprecated class ForeachStmtEnumerable = ForEachStmtEnumerable;'. These aliases reference the new class names, but the new classes themselves are defined immediately after the aliases. In QL, a class alias to a class that is not yet declared at that point may cause a compila…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| import csharp | ||
|
|
||
| where forall(ForeachStmt s | exists(s.getBody()) and exists(s.getIterableExpr())) | ||
| where forall(ForEachStmt s | exists(s.getBody()) and exists(s.getIterableExpr())) |
There was a problem hiding this comment.
Shipwright · HIGH
The rename is incomplete: the test file 'csharp/ql/test/library-tests/statements/Foreach1.ql' still has the old class name 'ForeachStmt' in its 'where' clause, while 'Foreach3.ql'
Impact: The rename is incomplete: the test file 'csharp/ql/test/library-tests/statements/Foreach1.ql' still has the old class name 'ForeachStmt' in its 'where' clause, while 'Foreach3.ql' was updated to 'ForEachStmt'. This inconsistency means the test suite will not compile or will test the deprecated alias instead of the new class, masking regressions in the renamed class and leaving the codebase in a half-migrated state.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| override string getAPrimaryQlClass() { result = "ForStmt" } | ||
| } | ||
|
|
||
| /** DEPRECATED: Use `ForEachStmt` instead. */ |
There was a problem hiding this comment.
Shipwright · HIGH
The deprecated alias 'ForeachStmt' is declared in 'Stmt.qll' but the new class 'ForEachStmt' is defined in the same file with the same '@foreach_stmt' database type.
Impact: The deprecated alias 'ForeachStmt' is declared in 'Stmt.qll' but the new class 'ForEachStmt' is defined in the same file with the same '@foreach_stmt' database type. The alias does not include any deprecation message or migration guidance, and the comment '/** DEPRECATED: Use ForEachStmt instead. */' is the only documentation. A new hire or downstream maintainer encountering the alias will not know whether the alias…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
Source merge-base:
738c91f1784934668eb1fa499fd27161d85d9acaSource head:
a5e733cf72e7fb3996e098e059b83fe2b62386ab