refactor: replace CFG processStatement switch with handler-table dispatch (TS + Rust) - #1590
Conversation
Sorry, something went wrong.
…atch (TS) docs check acknowledged
…complexity docs check acknowledged — pure internal Rust refactor, no API/language/CLI changes
Greptile SummaryThis PR is a pure complexity-decomposition refactor across the TypeScript CFG visitor and three Rust modules — no new features or behavioral changes. The cyclomatic reduction targets (processStatement 23→5, allNativeDataComplete 24→12, fileNeedsWasmTree 23→11, match_csharp_type_map cog 82→helpers) are all achieved cleanly.
Confidence Score: 5/5Safe to merge — all changes are mechanical decompositions of existing logic with no behavioral differences. Every extracted helper is a direct lift of the original code path: the Rust helpers preserve identical edge-wiring, the TS dispatch table builds once per visitor and correctly handles the processLabeled forward-reference through a closure, and the engine predicates are shared faithfully between fileNeedsWasmTree and allNativeDataComplete. No logic has been dropped or reordered in a way that could alter CFG construction or dataflow output. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["createCfgVisitor(cfgRules)"] --> B["buildStatementProcessors(cfgRules)"]
B --> C["buildStatementDispatch(cfgRules, processLabeled)"]
C --> D["map: Map<string, StatementHandler>\n(labeled, return, throw, break,\ncontinue, do, infinite, try)"]
C --> E["fallback: StatementEntry[4]\n(isIfNode/unless, isForNode,\nisWhileNode/until, isSwitchNode)"]
B --> F["processStatement closure"]
F --> H{"dispatch.map.get(type)?"}
H -- "hit O(1)" --> I["mapHandler(...)"]
H -- "miss" --> J{"fallback.find(...)?"}
J -- "hit O(4)" --> K["fallbackEntry.handle(...)"]
J -- "miss" --> L["update block lines, return currentBlock"]
I --> M["return CfgBlockInternal | null"]
K --> M
L --> M
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["createCfgVisitor(cfgRules)"] --> B["buildStatementProcessors(cfgRules)"]
B --> C["buildStatementDispatch(cfgRules, processLabeled)"]
C --> D["map: Map<string, StatementHandler>\n(labeled, return, throw, break,\ncontinue, do, infinite, try)"]
C --> E["fallback: StatementEntry[4]\n(isIfNode/unless, isForNode,\nisWhileNode/until, isSwitchNode)"]
B --> F["processStatement closure"]
F --> H{"dispatch.map.get(type)?"}
H -- "hit O(1)" --> I["mapHandler(...)"]
H -- "miss" --> J{"fallback.find(...)?"}
J -- "hit O(4)" --> K["fallbackEntry.handle(...)"]
J -- "miss" --> L["update block lines, return currentBlock"]
I --> M["return CfgBlockInternal | null"]
K --> M
L --> M
Reviews (3): Last reviewed commit: "docs: restore clarifying comment about d..." | Re-trigger Greptile |
Sorry, something went wrong.
| if (doComplexity && fileNeedsWasmComplexity(symbols, ext, langId)) { | ||
| debug(`allNativeDataComplete: ${relPath} missing complexity`); | ||
| return false; | ||
| } | ||
| if (doCfg && fileNeedsWasmCfg(symbols, ext, langId)) { | ||
| debug(`allNativeDataComplete: ${relPath} missing cfg blocks`); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Debug logs lose per-function granularity
Before this refactor, when allNativeDataComplete found a missing complexity or CFG entry it emitted ${relPath}:${def.name} missing complexity — including the offending function name. The new code delegates to fileNeedsWasmComplexity / fileNeedsWasmCfg, which only know about the file, so the debug lines now say ${relPath} missing complexity without specifying which function triggered the return. When chasing down why a particular file keeps being re-analysed by WASM the function name was the most actionable piece of context.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed — restored per-function name in WASM fallback debug logs by using Array.find on the definitions array to surface the offending function: ${relPath}:${offender?.name ?? '?'} missing complexity / missing cfg blocks.
Sorry, something went wrong.
| finally_handler = Some(child); | ||
| } | ||
| if matches_opt(child.kind(), self.rules.else_node) { | ||
| // Only treat as try-else if it's a direct child of the try statement | ||
| // (not the else_clause of an if inside the try body) | ||
| else_handler = Some(child); |
There was a problem hiding this comment.
Clarifying comment removed from collect_try_handlers
The deleted line read: "Only treat as try-else if it's a direct child of the try statement (not the else_clause of an if inside the try body)". That comment explained why iterating only over direct children of try_stmt is both necessary and sufficient — without it a reader has to re-derive that the loop is not recursive and cannot accidentally pick up else_clause nodes from nested if statements. Worth preserving as a doc-comment on the extracted function.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed — restored as a doc-comment on collect_try_handlers explaining that direct-child iteration is both necessary and sufficient, and that the loop cannot accidentally pick up else_clause nodes from nested if statements.
Sorry, something went wrong.
Codegraph Impact Analysis43 functions changed → 57 callers affected across 13 files
|
Sorry, something went wrong.
Impact: 5 functions changed, 24 affected
Impact: 1 functions changed, 3 affected
…lect_try_handlers
|
Addressed all 3 Greptile findings:
|
Sorry, something went wrong.
Sorry, something went wrong.
Summary
Titan Audit Context
Changes
Metrics Impact
Test plan