oxc-project / oxc-project/backlog
Add separate scopes for function bodies
Nobody has claimed this yet.
- Dominant language
- No language data
- Stars
- 7
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
How it is now
Currently functions only have a single scope which contains bindings for:
- Function name (for function expressions)
- Parameters
- Declarations in body of function
In SemanticBuilder we manage to correctly resolve IdentifierReferences within function params with an extra call to resolve_references_for_current_scope after walking the function params:
i.e. We resolve references in params before any bindings from body of function are added to the scope.
The problem
In the transformer, we also need to resolve references, but we don't have the advantage that we do in SemanticBuilder of constructing ScopeTree in order. It already exists, and we have to make sense of it.
Here's an example which doesn't work at present:
import React from 'react';
function foo(elem = <Component />) {
let React;
}
In classic mode, JSX transform converts this to:
import React from 'react';
function foo(elem = React.createElement(Component, null)) {
let React;
}
We need to resolve the binding for the inserted reference React. Currently, it resolves incorrectly to let React inside the function body, as resolution goes up the scope tree and finds a binding for React in the function's scope.
Additional problem
In sloppy mode code, there's also an edge case where you can have 2 bindings for arguments in a function - oxc-project/oxc#4232.
Obvious solution
The obvious solution is to add an additional scope to all function bodies.
Advantage: This is a simple solution which has no "surprises".
Disadvantages:
- More scopes = likely perf hit when resolving references for which the binding is outside current function.
- Makes it more complicated to detect illegal redeclarations (
function foo(x) { let x; }).
Alternative solution
- Add
SymbolFlags::Parameter(or maybe existingScopeFlags::FunctionScopedVariablewill do). - Track in
Traverseif you're inside a function's parameters or not. - When resolving a reference, if inside a function's parameters, for the first scope encountered with
ScopeFlags::Functionflag, do not match bindings unless they have aSymbolFlags::Parameterflag.
Actually, it's a bit more complex than that, in order to handle cases like this:
const x = 1;
function foo(
y = function bar(z = x) {
const x = 3;
}
) {
const x = 2;
}
The x in z = x needs to ignore both the binding in bar and the binding in foo and resolve to top level const x = 1. So rather than an "in function params" yes/no flag, would need a "function parameter depth" counter (in this case depth is 2).
Advantage: Maintain lower number of scopes = probably better perf.
Disadvantage: The complexity of reference resolution could make it error-prone.
Which?
The more complicated solution should work (though it wouldn't solve the 2 x arguments bindings edge case). But it's somewhat complex.
Personally I would side with trying the simple solution first and measure how much it impacts performance. Perhaps it's negligible. If so, great, but if not then we can look at the more complicated solution.
Any thoughts?
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the linked SemanticBuilder code in crates/oxc_semantic/src/builder.rs at lines 1955-1963, then trace how the transformer resolves references against the existing ScopeTree. Compare the separate-function-body-scope and parameter-depth alternatives using the JSX and nested default-parameter examples, including the duplicate arguments case. Done means correct bindings, valid redeclaration handling, and measured scope-resolution performance.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, rust
- Domain
- compilers
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100