Skip to content

C#: Rename ForeachStmt to ForEachStmt. - #12

Open
anurag6569201 wants to merge 1 commit into
qa/agent-github-codeql/pr-12-22492/basefrom
qa/agent-github-codeql/pr-12-22492/head
Open

anurag6569201 wants to merge 1 commit into
qa/agent-github-codeql/pr-12-22492/basefrom
qa/agent-github-codeql/pr-12-22492/head

Conversation

@anurag6569201

Copy link
Copy Markdown

Source merge-base: 738c91f1784934668eb1fa499fd27161d85d9aca
Source head: a5e733cf72e7fb3996e098e059b83fe2b62386ab

@shipwright-agent

Copy link
Copy Markdown

⛔ Shipwright · Blocked

Recommendation: do not merge PR #12 · Tier T1
Checks: 0 total · 0 needing attention

Next step: resolve the blocking findings before merge.

Findings (5)

  • CRITICAL The deprecated alias 'ForeachStmt' is defined as 'deprecated class ForeachStmt = ForEachStmt;' in Stmt.qll, but the new class 'ForEachStmt' overrides 'getAPrimaryQlClass()' to retu · csharp/ql/lib/semmle/code/csharp/Stmt.qll:425
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL The deprecated aliases in Linq/Helpers.qll are declared as 'deprecated class ForeachStmtGenericEnumerable = ForEachStmtGenericEnumerable;' and 'deprecated class ForeachStmtEnumerab · csharp/ql/lib/Linq/Helpers.qll:36
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun 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' · csharp/ql/test/library-tests/statements/Foreach1.ql:7
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun 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. · csharp/ql/lib/semmle/code/csharp/Stmt.qll:425
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH (unverified) The rename from 'ForeachStmt' to 'ForEachStmt' is applied inconsistently across the codebase. · Abuse/ForeachCapture.ql:26
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

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 /shipwright rerun to verify again.

override string getAPrimaryQlClass() { result = "ForStmt" }
}

/** DEPRECATED: Use `ForEachStmt` instead. */

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 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. */

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 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()))

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 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. */

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 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant