mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(semantic): keep block type bindings separate from hoisted vars (#28601)
Fixes #28600. `var I; interface I {}` in a nested block merged the interface into the hoisted `var`, so the type moved out of the block. TypeScript keeps the interface in the block. #27936 fixed the opposite order, `interface I {} var I;`. ## Cause Hoisting a `var` records it in `hoisting_variables` for each intermediate scope. `check_redeclaration` falls back to that map, and `declare_symbol_on_scope` then reused the returned symbol, even for a type declaration. ## Changes - `declare_symbol_on_scope`: a declaration with no value flags only reuses a symbol bound in the target scope. Otherwise it creates a new symbol there. - `check_redeclaration`: a binding in the scope and a hoisted `var` are now checked separately. Without this, the block's type binding would hide the conflict and the first change would lose the error in `{ var X; interface X {} let X; }`. - `check_redeclaration`: only the value flags of a hoisted symbol are compared. The symbol can also carry type flags from a declaration in the outer scope, and those do not conflict with a declaration in the block. The second change also fixes a missed error in `{ interface X {} var X; let X; }`. tsc rejects both forms with TS2481. The value-flag restriction also fixes a false error on main: `type X = number; { var X = 0; interface X {} }` was reported as a redeclaration, but tsc accepts it. An uninstantiated `namespace` in a block also has no value flags, so it now gets its own block symbol too. tsc already rejects namespaces in blocks (TS1235). ## Tests - `tests/integration/symbols.rs`: symbol separation and reference resolution, including the switch case, plus the redeclaration errors and the valid programs where an outer type shares a symbol with the hoisted `var`. - `tests/fixtures/oxc/ts/interfaces/hoisted-var-then-interface.ts`: a new snapshot of the scope placement. No existing semantic snapshots changed. --- AI assistance was used for this change. I reviewed and tested it. --------- Co-authored-by: qing deng <dengqing0821@gmail.com>
This commit is contained in:
1 parent
3fd78d7451
commit
ec327aa1ec
4 files changed
+166
-14
No files matched your search
@@ -544,7 +544,12 @@ impl<'a> SemanticBuilder<'a> {
|
||||
includes: SymbolFlags,
|
||||
excludes: SymbolFlags,
|
||||
) -> SymbolId {
|
||||
if let Some(symbol_id) = self.check_redeclaration(scope_id, span, name, excludes) {
|
||||
// A `var` hoisted through this scope is bound in an outer scope. Interfaces and type aliases
|
||||
// live in a separate declaration space, so they get their own symbol in this scope.
|
||||
if let Some(symbol_id) = self.check_redeclaration(scope_id, span, name, excludes)
|
||||
&& (includes.intersects(SymbolFlags::Value)
|
||||
|| self.scoping.scope_has_binding(scope_id, name))
|
||||
{
|
||||
self.add_redeclare_variable(symbol_id, includes, span);
|
||||
self.scoping.union_symbol_flag(symbol_id, includes);
|
||||
return symbol_id;
|
||||
@@ -580,25 +585,37 @@ impl<'a> SemanticBuilder<'a> {
|
||||
name: Ident<'a>,
|
||||
excludes: SymbolFlags,
|
||||
) -> Option<SymbolId> {
|
||||
let symbol_id = self
|
||||
.scoping
|
||||
.get_binding(scope_id, name)
|
||||
.or_else(|| self.hoisting_variables.get(&(scope_id, name)).copied())?;
|
||||
let binding_id = self.scoping.get_binding(scope_id, name);
|
||||
|
||||
let flags = self.scoping.symbol_flags(symbol_id);
|
||||
if let Some(symbol_id) = binding_id {
|
||||
let flags = self.scoping.symbol_flags(symbol_id);
|
||||
|
||||
// A named function expression binds its own name in the function's scope, so a same-named
|
||||
// parameter is not a redeclaration: `(function n(n) {})` - param `n` is fine.
|
||||
if flags.contains(SymbolFlags::FunctionExpression) {
|
||||
return None;
|
||||
// A named function expression binds its own name in the function's scope, so a same-named
|
||||
// parameter is not a redeclaration: `(function n(n) {})` - param `n` is fine.
|
||||
if flags.contains(SymbolFlags::FunctionExpression) {
|
||||
return None;
|
||||
}
|
||||
|
||||
if flags.intersects(excludes) {
|
||||
let symbol_span = self.scoping.symbol_span(symbol_id);
|
||||
self.error(redeclaration(&name, symbol_span, span));
|
||||
return Some(symbol_id);
|
||||
}
|
||||
}
|
||||
|
||||
if flags.intersects(excludes) {
|
||||
// A type-only binding in this scope must not hide a conflicting hoisted `var`:
|
||||
// `{ var x; interface x {} let x; }`
|
||||
// Only the value meaning conflicts. A type sharing the symbol belongs to the outer scope:
|
||||
// `type x = 0; { var x; interface x {} }`
|
||||
let hoisted_id = self.hoisting_variables.get(&(scope_id, name)).copied();
|
||||
if let Some(symbol_id) = hoisted_id
|
||||
&& self.scoping.symbol_flags(symbol_id).intersects(excludes & SymbolFlags::Value)
|
||||
{
|
||||
let symbol_span = self.scoping.symbol_span(symbol_id);
|
||||
self.error(redeclaration(&name, symbol_span, span));
|
||||
}
|
||||
|
||||
Some(symbol_id)
|
||||
binding_id.or(hoisted_id)
|
||||
}
|
||||
|
||||
/// Declare an unresolved reference in the current scope.
|
||||
|
||||
+81
@@ -0,0 +1,81 @@
|
||||
---
|
||||
source: crates/oxc_semantic/tests/main.rs
|
||||
input_file: crates/oxc_semantic/tests/fixtures/oxc/ts/interfaces/hoisted-var-then-interface.ts
|
||||
---
|
||||
[
|
||||
{
|
||||
"children": [
|
||||
{
|
||||
"children": [
|
||||
{
|
||||
"children": [],
|
||||
"flags": "ScopeFlags(StrictMode)",
|
||||
"id": 2,
|
||||
"node": "TSInterfaceDeclaration",
|
||||
"symbols": []
|
||||
},
|
||||
{
|
||||
"children": [],
|
||||
"flags": "ScopeFlags(StrictMode)",
|
||||
"id": 3,
|
||||
"node": "TSInterfaceDeclaration",
|
||||
"symbols": []
|
||||
}
|
||||
],
|
||||
"flags": "ScopeFlags(StrictMode)",
|
||||
"id": 1,
|
||||
"node": "SwitchStatement",
|
||||
"symbols": [
|
||||
{
|
||||
"flags": "SymbolFlags(Interface)",
|
||||
"id": 1,
|
||||
"name": "I",
|
||||
"node": "TSInterfaceDeclaration",
|
||||
"references": [
|
||||
{
|
||||
"flags": "ReferenceFlags(Type)",
|
||||
"id": 1,
|
||||
"name": "I",
|
||||
"node_id": 33,
|
||||
"scope_id": 1
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"flags": "SymbolFlags(BlockScopedVariable | ConstVariable)",
|
||||
"id": 2,
|
||||
"name": "value",
|
||||
"node": "VariableDeclarator(value)",
|
||||
"references": []
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"children": [],
|
||||
"flags": "ScopeFlags(StrictMode)",
|
||||
"id": 4,
|
||||
"node": "TSTypeAliasDeclaration",
|
||||
"symbols": []
|
||||
}
|
||||
],
|
||||
"flags": "ScopeFlags(StrictMode | Top)",
|
||||
"id": 0,
|
||||
"node": "Program",
|
||||
"symbols": [
|
||||
{
|
||||
"flags": "SymbolFlags(FunctionScopedVariable)",
|
||||
"id": 0,
|
||||
"name": "I",
|
||||
"node": "VariableDeclarator(I)",
|
||||
"references": []
|
||||
},
|
||||
{
|
||||
"flags": "SymbolFlags(TypeAlias)",
|
||||
"id": 3,
|
||||
"name": "Outer",
|
||||
"node": "TSTypeAliasDeclaration",
|
||||
"references": []
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
+12
@@ -0,0 +1,12 @@
|
||||
export {};
|
||||
|
||||
switch (Number(1)) {
|
||||
case 1:
|
||||
var I = 0;
|
||||
interface I { a: number }
|
||||
case 2:
|
||||
interface I { b: number }
|
||||
const value: I = { a: 1, b: 2 };
|
||||
}
|
||||
|
||||
type Outer = I;
|
||||
@@ -180,6 +180,11 @@ fn test_hoisted_var_does_not_reuse_block_type_binding() {
|
||||
"function f() { { type X = string; var X = 1; type Y = X; X; } X; }",
|
||||
"function f() { { interface X {} var X = 1; type Y = X; X; } X; }",
|
||||
"function f() { { type X = string; { var X = 1; } type Y = X; X; } X; }",
|
||||
"function f() { { var X = 1; interface X {} type Y = X; X; } X; }",
|
||||
"function f() { { var X = 1; type X = string; type Y = X; X; } X; }",
|
||||
"function f() { { { var X = 1; } interface X {} type Y = X; X; } X; }",
|
||||
"function f() { { var X = 1; interface X {} var X = 2; type Y = X; X; } X; }",
|
||||
"function f() { switch (0) { case 0: var X = 1; interface X {} case 1: interface X {} type Y = X; X; } X; }",
|
||||
] {
|
||||
let tester = SemanticTester::ts(source);
|
||||
let semantic = tester.build();
|
||||
@@ -196,14 +201,51 @@ fn test_hoisted_var_does_not_reuse_block_type_binding() {
|
||||
.unwrap();
|
||||
let type_symbol = symbols.into_iter().find(|&symbol| symbol != value_symbol).unwrap();
|
||||
assert_ne!(scoping.symbol_scope_id(value_symbol), scoping.symbol_scope_id(type_symbol));
|
||||
assert_eq!(scoping.get_resolved_references(value_symbol).count(), 2);
|
||||
assert_eq!(scoping.get_resolved_references(type_symbol).count(), 1);
|
||||
assert!(!scoping.symbol_flags(value_symbol).is_type(), "{source}");
|
||||
assert_eq!(scoping.get_resolved_references(value_symbol).count(), 2, "{source}");
|
||||
assert_eq!(scoping.get_resolved_references(type_symbol).count(), 1, "{source}");
|
||||
assert!(scoping.get_resolved_references(value_symbol).all(Reference::is_read));
|
||||
assert!(scoping.get_resolved_references(type_symbol).all(Reference::is_type));
|
||||
assert!(!scoping.root_unresolved_references().contains_key("X"));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_block_interface_is_not_visible_outside_of_hoisted_var() {
|
||||
let tester = SemanticTester::ts(
|
||||
"switch (0) { case 0: var I = 0; interface I {} case 1: interface I {} } type Outer = I;",
|
||||
);
|
||||
let semantic = tester.build();
|
||||
let scoping = semantic.scoping();
|
||||
// `Outer` cannot use the hoisted value `I` as a type, and the interface is out of scope.
|
||||
assert!(scoping.root_unresolved_references().contains_key("I"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_lexical_redeclaration_of_hoisted_var_next_to_block_type_binding() {
|
||||
for source in [
|
||||
"function f() { { var X; interface X {} let X; } }",
|
||||
"function f() { { var X; type X = string; const X = 1; } }",
|
||||
"function f() { { var X; interface X {} class X {} } }",
|
||||
"function f() { { interface X {} var X; let X; } }",
|
||||
"function f() { type X = number; { var X; interface X {} let X; } }",
|
||||
] {
|
||||
SemanticTester::ts(source).has_error("Identifier `X` has already been declared");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_block_type_binding_ignores_outer_type_merged_with_hoisted_var() {
|
||||
for source in [
|
||||
"function f() { type X = number; { interface X { a: number } var X = 0; interface X { b: number } const value: X = { a: 1, b: 2 }; } }",
|
||||
"function f() { type X = number; { var X = 0; interface X {} } }",
|
||||
"function f() { type X = number; { var X = 0; type X = string; } }",
|
||||
"function f() { interface X {} { var X = 0; type X = string; } }",
|
||||
] {
|
||||
SemanticTester::ts(source).build();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_var_read_write() {
|
||||
SemanticTester::js("let x; x += 1")
|
||||
|
||||
Reference in new issue
Block a user