Skip to content

Conversation

@mathpirate
Copy link
Contributor

@mathpirate mathpirate commented Dec 3, 2025

Two key simplifications to the data flow analysis code:

  1. Replace localNodes with expressionToNodeId Map

    • Eliminates InternalAnalysis.localNodes field entirely
    • Parent-child relationships now tracked via O(1) Map lookup
    • Reduces merge complexity (no more localNodes concatenation)
    • ~30 lines removed across return sites
  2. Eliminate DataFlowScopeInternal type

    • Merge into DataFlowScope by computing enriched parameters eagerly
    • Remove pre-computed aggregated Set in favor of on-demand computation
    • getAggregatedSymbols() walks parent chain when needed
    • Remove toDataFlowScope() conversion function

Net result: -38 lines, simpler types, clearer data flow.

A third major simplification is pending to unify logical paths for handling synthetic and non-synthetic nodes.


Summary by cubic

Simplifies the data flow analyzer by removing localNodes and DataFlowScopeInternal, switching to an expression→node map and on-demand scope aggregation. Also unifies synthetic/non-synthetic handling with safe symbol/type lookups and complete JSX handling.

  • Refactors
    • Replaced localNodes with expressionToNodeId Map for O(1) parent lookups; removed findParentNodeId and localNodes merges.
    • Merged DataFlowScopeInternal into DataFlowScope with eager parameters and on-demand aggregation; removed toDataFlowScope.
    • Unified synthetic and non-synthetic analysis paths using tryGetSymbol/tryGetType; removed the separate synthetic block.
    • Completed JSX analysis for attributes and children; added handlers for JsxSelfClosingElement and JsxFragment.

Written for commit cb1c3c3. Summary will update automatically on new commits.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

mathpirate and others added 3 commits December 2, 2025 18:33
Two key simplifications to the data flow analysis code:

1. Replace localNodes with expressionToNodeId Map
   - Eliminates InternalAnalysis.localNodes field entirely
   - Parent-child relationships now tracked via O(1) Map lookup
   - Reduces merge complexity (no more localNodes concatenation)
   - ~30 lines removed across return sites

2. Eliminate DataFlowScopeInternal type
   - Merge into DataFlowScope by computing enriched parameters eagerly
   - Remove pre-computed `aggregated` Set in favor of on-demand computation
   - getAggregatedSymbols() walks parent chain when needed
   - Remove toDataFlowScope() conversion function

Net result: -38 lines, simpler types, clearer data flow.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Remove the separate ~300 line synthetic node handling block by integrating
fallback logic into the unified expression handlers. This refactoring:

- Adds tryGetSymbol/tryGetType helpers that gracefully handle synthetic nodes
- Updates each expression handler to try checker methods first, then fall back
  to heuristics when dealing with synthetic nodes
- Removes duplicate logic that was maintained in two places
- Adds proper handlers for JsxSelfClosingElement and JsxFragment

Net result: 214 lines removed (432 deletions, 218 insertions)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The dataflow analyzer now provides complete, self-contained analysis
for JSX elements including both attributes and children.

Previously, attribute analysis used `ts.isExpression(attr)` which
always returned false for JsxAttribute nodes (dead code). The system
worked because the OpaqueRefJSXTransformer handled JSX at the visitor
level, but this created unclear contracts and hidden coupling.

Now the analyzer properly handles:
- JsxAttribute with expression initializers: `value={expr}`
- JsxSpreadAttribute: `{...expr}`
- JsxElement children (JsxExpression, nested elements)
- JsxSelfClosingElement
- JsxFragment

This makes the analyzer's contract clear: callers get correct results
regardless of how they traverse the AST.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@mathpirate mathpirate force-pushed the refactor/dataflow-cleanup branch from 0ae57d3 to cb1c3c3 Compare December 3, 2025 02:33
@mathpirate mathpirate merged commit 819fa28 into main Dec 3, 2025
9 checks passed
@mathpirate mathpirate deleted the refactor/dataflow-cleanup branch December 3, 2025 21:03
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.

2 participants