Skip to content

fix(linter): rewrite constructor-super to use iterative dataflow analysis#16706

Merged
graphite-app[bot] merged 1 commit intomainfrom
c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis
Dec 13, 2025
Merged

fix(linter): rewrite constructor-super to use iterative dataflow analysis#16706
graphite-app[bot] merged 1 commit intomainfrom
c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis

Conversation

@camc314
Copy link
Contributor

@camc314 camc314 commented Dec 10, 2025

The constructor-super rule's DFS path analysis had exponential O(2^n)
complexity, causing oxlint to hang indefinitely on files with complex
control flow graphs (e.g., next.js compiled bundles with 59+ classes).

Root Cause

The previous algorithm explored all possible paths through the CFG,
removing blocks from the visited set after each path to allow
re-exploration. With k branch points, this creates 2^k paths.

Solution

Replaced the DFS path enumeration with iterative dataflow analysis:

  • New SuperCallState enum tracks abstract states: Unreached, Never,
    Once, Multiple, Mixed
  • Merge operation at CFG join points combines states from different paths
  • Transfer function computes state after executing super() calls
  • Worklist algorithm propagates states until fixpoint

Complexity

  • Before: O(2^n) where n = number of branch points
  • After: O(n × m) where n = blocks, m = edges

The state lattice has finite height (5 states), guaranteeing termination.

Performance

File Before After
fetch.js (796KB, 59 classes) hung indefinitely 16ms
load.js (800KB, 59 classes) hung indefinitely 50ms
tar/index.js (98KB, 30 classes) hung indefinitely 6ms
Full next.js (19,535 files) hung indefinitely 1.2s

🤖 generated with help from Claude Opus 4.5

@github-actions github-actions bot added A-linter Area - Linter C-bug Category - Bug labels Dec 10, 2025
Copy link
Contributor Author

camc314 commented Dec 10, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 10, 2025

CodSpeed Performance Report

Merging #16706 will not alter performance

Comparing c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis (38a129b) with main (73da317)1

Summary

✅ 4 untouched
⏩ 41 skipped2

Footnotes

  1. No successful run was found on main (a3b9eff) during the generation of this report, so 73da317 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 41 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@camc314 camc314 self-assigned this Dec 11, 2025
@camc314 camc314 force-pushed the c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis branch 3 times, most recently from d511468 to 5986993 Compare December 13, 2025 13:36
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Dec 13, 2025
@camc314 camc314 marked this pull request as ready for review December 13, 2025 13:36
Copilot AI review requested due to automatic review settings December 13, 2025 13:36
Copy link
Contributor Author

camc314 commented Dec 13, 2025

Merge activity

  • Dec 13, 1:36 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Dec 13, 1:41 PM UTC: camc314 added this pull request to the Graphite merge queue.
  • Dec 13, 1:44 PM UTC: The Graphite merge queue couldn't merge this PR because it was in draft mode.
  • Dec 13, 3:39 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Dec 13, 3:43 PM UTC: camc314 added this pull request to the Graphite merge queue.
  • Dec 13, 3:48 PM UTC: Merged by the Graphite merge queue.

graphite-app bot pushed a commit that referenced this pull request Dec 13, 2025
…ysis (#16706)

The constructor-super rule's DFS path analysis had exponential O(2^n)
complexity, causing oxlint to hang indefinitely on files with complex
control flow graphs (e.g., next.js compiled bundles with 59+ classes).

## Root Cause

The previous algorithm explored all possible paths through the CFG,
removing blocks from the visited set after each path to allow
re-exploration. With k branch points, this creates 2^k paths.

## Solution

Replaced the DFS path enumeration with iterative dataflow analysis:

- New `SuperCallState` enum tracks abstract states: Unreached, Never,
  Once, Multiple, Mixed
- Merge operation at CFG join points combines states from different paths
- Transfer function computes state after executing super() calls
- Worklist algorithm propagates states until fixpoint

## Complexity

- Before: O(2^n) where n = number of branch points
- After: O(n × m) where n = blocks, m = edges

The state lattice has finite height (5 states), guaranteeing termination.

## Performance

| File | Before | After |
|------|--------|-------|
| fetch.js (796KB, 59 classes) | hung indefinitely | 16ms |
| load.js (800KB, 59 classes) | hung indefinitely | 50ms |
| tar/index.js (98KB, 30 classes) | hung indefinitely | 6ms |
| Full next.js (19,535 files) | hung indefinitely | 1.2s |

🤖 generated with help from Claude Opus 4.5
@graphite-app graphite-app bot force-pushed the c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis branch from 5986993 to e08b1c3 Compare December 13, 2025 13:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a critical performance issue in the constructor-super rule by replacing exponential-complexity DFS path enumeration with linear-complexity iterative dataflow analysis. The previous algorithm had O(2^n) time complexity that caused oxlint to hang indefinitely on files with complex control flow (59+ classes), while the new algorithm achieves O(n × m) complexity, processing previously problematic files in milliseconds.

Key Changes:

  • Introduced SuperCallState enum to represent abstract states in dataflow analysis (Unreached, Never, Once, Multiple, Mixed)
  • Replaced recursive DFS traversal with worklist-based iterative algorithm using state propagation and merging
  • Refactored several match expressions to use more idiomatic Rust patterns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@camc314 camc314 marked this pull request as draft December 13, 2025 13:44
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 13, 2025
Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

The shift to a worklist dataflow analysis is a strong improvement, but a few semantic edge cases look off. In particular, exceptional control-flow (EdgeType::Error(Explicit)) currently propagates a pre-transfer state, which can undercount super() in try blocks, and exit blocks may still propagate to successors, diverging from prior behavior. The loop handling via loop_with_super => NoSuper is too coarse and likely produces incorrect results on common CFG shapes; consider a more conservative Multiple/SCC-based approach.

Summary of changes

What changed

Performance fix: replace path-enumeration DFS with dataflow

  • Replaced the previous DFS path exploration (exponential O(2^k) over branch points) with an iterative worklist dataflow analysis over CFG blocks.
  • Introduced a 5-state lattice SuperCallState (Unreached, Never, Once, Multiple, Mixed) with:
    • merge() for join points
    • add_super_calls() as a transfer function based on per-block super() counts
  • Added a VecDeque-based worklist to propagate states to fixpoint.

CFG scanning improvements

  • Simplified classify_super_class pattern matching.
  • Refactored ExpressionStatement handling and added special-case detection for:
    • ternary cond ? super() : ...
    • super() || super() (counts as duplicate because RHS always runs)

Path result construction

  • Collected exit states ((SuperCallState, is_acceptable_exit)) and converted them into PathResult values after fixpoint.
  • Added a heuristic flag loop_with_super to account for loops involving super() via backedges.

@charliecreates charliecreates bot removed the request for review from CharlieHelps December 13, 2025 13:46
@camc314 camc314 force-pushed the c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis branch from e08b1c3 to acc93da Compare December 13, 2025 15:01
@camc314 camc314 marked this pull request as ready for review December 13, 2025 15:05
@camc314 camc314 requested a review from Copilot December 13, 2025 15:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

The new worklist dataflow approach addresses the performance problem, but there are still correctness risks in the abstract interpretation. In particular, SuperCallState::add_super_calls collapses Mixed too aggressively, and propagating pre-transfer state across EdgeType::Error(Explicit) can undercount super() within a basic block. Loop handling remains heuristic-driven and may be both over- and under-conservative compared to SCC-based reasoning. Finally, diagnostics behavior changed to emit “missing super on some paths” even when duplicates exist, which may increase noise.

Additional notes (1)
  • Maintainability | crates/oxc_linter/src/rules/eslint/constructor_super.rs:270-276
    some_missing diagnostics behavior changed: previously missing-super-on-some-paths was suppressed when there was also a duplicate (!has_duplicate). Now it always emits missing-super diagnostics even when duplicates exist.

This can create noisy or contradictory reports on the same constructor (e.g., one path calls super() twice while another calls it zero times). If the rule’s intent is to prioritize duplicate-super as the primary issue (as the prior code suggests), this is a behavioral change that should be justified or restored.

Summary of changes

Summary of changes

constructor-super rule: switch from path-enumeration DFS to dataflow

  • Replaced the previous DFS path exploration (with cycle re-visitation) with a worklist-based iterative dataflow analysis over CFG blocks.
  • Added a 5-element abstract domain SuperCallState (Unreached, Never, Once, Multiple, Mixed) with:
    • merge() join operator
    • add_super_calls() transfer based on per-block super() counts
  • Introduced VecDeque worklist and block_states: FxHashMap<BlockNodeId, SuperCallState> to compute a fixpoint.

CFG/AST scanning tweaks

  • Simplified classify_super_class to a more idiomatic match.
  • Improved super() discovery in ExpressionStatement by special-casing:
    • ternary cond ? super() : ... (counts once)
    • super() || super() (counts both; RHS always executes)

Path result construction

  • Collected exit states into exit_results and converted them into Vec<PathResult> after the fixpoint.
  • Added conservative loop handling via a loop_with_super flag that injects both NoSuper and CalledMultiple.

Tests

  • Added passing cases covering super() before a potential throw and super() inside finally.

@charliecreates charliecreates bot removed the request for review from CharlieHelps December 13, 2025 15:17
Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

The new dataflow approach fixes the exponential blowup, but several semantics look incorrect or overly heuristic. In particular, SuperCallState::add_super_calls collapses Mixed too aggressively, explicit error-edge propagation likely undercounts super() within try blocks, and the loop heuristic can both miss loop-contained super() and incorrectly force NoSuper. Additionally, the super() || super() special-case relies on an incorrect JS assumption (that super() is falsy), which can directly cause false duplicate reports.

Summary of changes

Summary

This change rewrites the constructor-super rule’s control-flow analysis to avoid exponential path enumeration.

Key updates

  • Replaced DFS path exploration with a worklist-based iterative dataflow analysis using a new SuperCallState lattice (Unreached | Never | Once | Multiple | Mixed).
  • Added state operations:
    • merge() for join points
    • add_super_calls() as the transfer function per basic block
  • Updated CFG traversal:
    • Uses VecDeque worklist and block_states map.
    • Treats blocks with exit instructions (return/throw/implicit return) as terminal.
    • Detects loops via EdgeType::Backedge and emits conservative results.
  • Improved AST scanning in find_super_calls_in_cfg to count super() in:
    • ternaries (cond ? super() : ...)
    • super() || super() (counts as duplicate because RHS always runs)
  • Simplified superclass classification logic and expanded documentation/comments.
  • Added new test cases around try/catch/finally and throwing behavior.

Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

The dataflow rewrite fixes the exponential path explosion, but there are correctness regressions: SuperCallState::add_super_calls collapses Mixed into Multiple in cases where it should stay Mixed, which can change which diagnostics fire. Explicit error-edge propagation uses a pre-transfer state based on a questionable CFG invariant and can undercount super() executed before a throw in the same block. Loop handling via a loop_with_super flag remains too heuristic, and the super() || super() duplicate rule is based on incorrect JavaScript semantics and can produce false positives.

Summary of changes

Overview

This change rewrites the constructor-super rule’s path analysis to avoid exponential CFG path enumeration.

Key changes

  • Replaced recursive DFS path exploration with an iterative worklist dataflow analysis using VecDeque.
  • Introduced a 5-state lattice SuperCallState (Unreached, Never, Once, Multiple, Mixed) with merge() and add_super_calls() transfer.
  • Updated CFG propagation logic:
    • Terminal blocks (return/throw/implicit return) are collected as exits and no longer propagate to successors.
    • EdgeType::Error(Explicit) is handled specially (propagates pre-transfer state).
    • Backedges are not followed; loops set a flag that contributes both NoSuper and CalledMultiple results.
  • Simplified classify_super_class matching.
  • Extended super-call detection in expression statements for conditional expressions and the special case super() || super().
  • Added tests covering super() around throw/try/finally patterns.

Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

The new worklist-based dataflow is a big performance improvement, but there are likely semantic regressions in the core abstract interpretation. SuperCallState::add_super_calls() is too lossy and can incorrectly jump to Multiple, the super() || super() special-case is based on incorrect JS semantics, and the remaining loop heuristic can both over- and under-report. Additionally, the diagnostic gating change may introduce noisy double-reporting when duplicates and missing paths co-occur.

Additional notes (1)
  • Maintainability | crates/oxc_linter/src/rules/eslint/constructor_super.rs:270-276
    The rule now reports missing_super_some whenever some_missing is true, even if the same analysis also detects duplicates (has_duplicate). Previously it suppressed the missing-path diagnostic when duplicates were present (some_missing && !has_duplicate).

That behavior change is likely noisy: in a constructor with both a missing path and a duplicate path, you’ll now emit both “missing on some paths” and “duplicate super” diagnostics. In ESLint’s constructor-super, the duplicate is usually the more actionable primary error; piling on can be confusing and may be considered a regression in output quality.

Summary of changes

Summary

This diff refactors the constructor-super lint rule to avoid exponential CFG path exploration by switching from recursive DFS path enumeration to an iterative worklist dataflow analysis.

Key updates

  • Replaced FxHashSet-based DFS cycle handling with a VecDeque worklist and per-block abstract states stored in FxHashMap<BlockNodeId, SuperCallState>.
  • Added a 5-element lattice SuperCallState (Unreached, Never, Once, Multiple, Mixed) with merge() (join) and add_super_calls() (transfer).
  • Adjusted path-result construction to derive PathResult from exit states and loop heuristics rather than enumerating all paths.
  • Improved superclass classification with more idiomatic match guards.
  • Expanded super-call detection in ExpressionStatement for ternaries and super() || super().
  • Added tests covering super() around try/catch/finally and potential throws.

@camc314 camc314 force-pushed the c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis branch from acc93da to 38a129b Compare December 13, 2025 15:37
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Dec 13, 2025
…ysis (#16706)

The constructor-super rule's DFS path analysis had exponential O(2^n)
complexity, causing oxlint to hang indefinitely on files with complex
control flow graphs (e.g., next.js compiled bundles with 59+ classes).

## Root Cause

The previous algorithm explored all possible paths through the CFG,
removing blocks from the visited set after each path to allow
re-exploration. With k branch points, this creates 2^k paths.

## Solution

Replaced the DFS path enumeration with iterative dataflow analysis:

- New `SuperCallState` enum tracks abstract states: Unreached, Never,
  Once, Multiple, Mixed
- Merge operation at CFG join points combines states from different paths
- Transfer function computes state after executing super() calls
- Worklist algorithm propagates states until fixpoint

## Complexity

- Before: O(2^n) where n = number of branch points
- After: O(n × m) where n = blocks, m = edges

The state lattice has finite height (5 states), guaranteeing termination.

## Performance

| File | Before | After |
|------|--------|-------|
| fetch.js (796KB, 59 classes) | hung indefinitely | 16ms |
| load.js (800KB, 59 classes) | hung indefinitely | 50ms |
| tar/index.js (98KB, 30 classes) | hung indefinitely | 6ms |
| Full next.js (19,535 files) | hung indefinitely | 1.2s |

🤖 generated with help from Claude Opus 4.5
@graphite-app graphite-app bot force-pushed the c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis branch from 38a129b to 50e0a23 Compare December 13, 2025 15:43
@graphite-app graphite-app bot merged commit 50e0a23 into main Dec 13, 2025
20 checks passed
@graphite-app graphite-app bot deleted the c/12-10-fix_linter_rewrite_constructor-super_to_use_iterative_dataflow_analysis branch December 13, 2025 15:48
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants