Repository navigation
Unified: Allow pattern-less parameters. - #22791
aschackmull wants to merge 3 commits into
Conversation
0d23722 to
5b98c6c
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The fallback makes parameter type syntax participate in control flow through generic child traversal.
1 open finding
What changed in this PR
Allows patternless Swift enum-case parameters to participate in the CFG, resolving dead-end consistency failures.
Changes:
- Falls back to the parameter itself when no pattern exists.
- Updates generated consistency expectations.
| File | Description |
|---|---|
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll |
Adds pattern fallback behavior. |
unified/ql/test/library-tests/BasicTest/CONSISTENCY/CfgConsistency.expected |
Removes resolved dead ends. |
unified/ql/test/library-tests/dataflow/CONSISTENCY/CfgConsistency.expected |
Removes resolved dead ends. |
unified/ql/test/library-tests/local-name-binding/CONSISTENCY/CfgConsistency.expected |
Removes resolved dead ends. |
unified/ql/test/library-tests/static-name-binding/CONSISTENCY/CfgConsistency.expected |
Removes resolved dead end. |
unified/ql/test/library-tests/type-inference/CONSISTENCY/CfgConsistency.expected |
Removes resolved dead ends. |
unified/ql/test/query-tests/security/CWE-022/PathInjection/CONSISTENCY/CfgConsistency.expected |
Removes resolved dead end. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| AstNode getPattern() { | ||
| result = super.getPattern() | ||
| or | ||
| not exists(super.getPattern()) and result = this | ||
| } |
asgerf
left a comment
asgerf
left a comment
There was a problem hiding this comment.
LGTM. The comment from CCR might be worth looking at, but feel free to merge otherwise
There was a problem hiding this comment.
It looks like these empty files were not deleted?
There was a problem hiding this comment.
(Here is what I normally run: find unified/ql/test -name "*Consistency.expected" -size 0 -print -delete)
There was a problem hiding this comment.
Huh, I mostly just used --learn, so I guess that's not sufficient.
Yeah, I think that comment is actually accurate and something I'll want to address. |
There are a couple of different ways to address this, but given that parameters are fully known to the shared lib, then I think it makes the most sense for the shared lib to own this concern. So I think this is best suited for a followup PR. |
Parameters of Swift enum case constructors currently don't have a pattern extracted, so the CFG breaks. In these cases the CFG library expects the parameter itself to act as pattern. This fixes a bunch of consistency errors.