Cfg: Add consistency check for multiple default cases. - #22510
Cfg: Add consistency check for multiple default cases.#22510aschackmull wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new check incorrectly reports valid guarded Python wildcard cases.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll — This flags valid Python match statements as inconsistent. Python maps every wildcard case to… |
What changed in this PR
Adds CFG consistency detection for switches with multiple default cases and completes the consistency overview.
Changes:
- Adds missing consistency overview entries.
- Fixes a consistency-query name.
- Detects multiple default cases.
| File | Description |
|---|---|
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll |
Extends and corrects CFG consistency checks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * A well-formed switch statement should have at most one default case. | ||
| */ | ||
| query predicate multipleDefaultCases(Switch s, int defaultCases) { | ||
| defaultCases = strictcount(DefaultCase c | s.getCase(_) = c) and |
There was a problem hiding this comment.
@yoff I think this indicates a mistake in the python CFG - a case _ if cond: should not be a DefaultCase, instead it should be included in predicate matchAll. The reason is that the shared CFG puts a default case last, and multiple default cases can't all be put "last". I.e. in several languages (e.g. in Java) it's valid to have the default case occur before another case, e.g. for the purpose of fallthrough, so it's only the non-default cases that are executed in-order.
I encountered this in Unified when certain case patterns weren't extracted properly.
(Also, the overview query was missing a few cases, which I drive-by-fixed).