Skip to content

Commit

Permalink
refactor(semantic): var hosting
Browse files Browse the repository at this point in the history
  • Loading branch information
Dunqing committed Jul 21, 2024
1 parent a207923 commit 79bd289
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 63 deletions.
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_import_assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ fn is_argument_of_well_known_mutation_function(node_id: AstNodeId, ctx: &LintCon

if ((ident.name == "Object" && OBJECT_MUTATION_METHODS.contains(property_name))
|| (ident.name == "Reflect" && REFLECT_MUTATION_METHODS.contains(property_name)))
&& !ctx.scopes().has_binding(current_node.scope_id(), &ident.name)
&& ident.reference_id.get().is_some_and(|id| !ctx.symbols().has_binding(id))
{
return expr
.arguments
Expand Down
16 changes: 0 additions & 16 deletions crates/oxc_mangler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,24 +104,8 @@ impl ManglerBuilder {
let mut slot = parent_max_slot;

if !bindings.is_empty() {
let mut parent_bindings = None;

// `bindings` are stored in order, traverse and increment slot
for symbol_id in bindings.values() {
// omit var hoisting because var symbols are added to every parent scope
if symbol_table.get_flag(*symbol_id).is_function_scoped_declaration()
&& parent_bindings.is_none()
{
parent_bindings = scope_tree
.get_parent_id(scope_id)
.map(|parent_scope_id| scope_tree.get_bindings(parent_scope_id));
}
if let Some(parent_bindings) = &parent_bindings {
if parent_bindings.values().contains(symbol_id) {
continue;
}
}

slots[*symbol_id] = slot;
slot += 1;
}
Expand Down
28 changes: 16 additions & 12 deletions crates/oxc_semantic/src/binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ pub trait Binder {

impl<'a> Binder for VariableDeclarator<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let current_scope_id = builder.current_scope_id;
let (includes, excludes) = match self.kind {
VariableDeclarationKind::Const => (
SymbolFlags::BlockScopedVariable | SymbolFlags::ConstVariable,
Expand All @@ -43,40 +42,45 @@ impl<'a> Binder for VariableDeclarator<'a> {

// Logic for scope hoisting `var`

let mut current_scope_id = builder.current_scope_id;
let mut var_scope_ids = vec![];
if !builder.current_scope_flags().is_var() {
var_scope_ids.push(current_scope_id);
for scope_id in builder.scope.ancestors(current_scope_id).skip(1) {
let flag = builder.scope.get_flags(scope_id);
// Skip the catch clause, the scope bindings have been cloned to the child block scope
if flag.is_catch_clause() {
continue;
}
var_scope_ids.push(scope_id);
if flag.is_var() {
current_scope_id = scope_id;
break;
}
var_scope_ids.push(scope_id);
}
}

self.id.bound_names(&mut |ident| {
let span = ident.span;
let name = &ident.name;

let mut declared_symbol_id = None;

for scope_id in &var_scope_ids {
if let Some(symbol_id) =
builder.check_redeclaration(*scope_id, span, name, excludes, true)
{
ident.symbol_id.set(Some(symbol_id));
builder.add_redeclare_variable(symbol_id, ident.span);
return;
builder.add_redeclare_variable(symbol_id, span);
let name = name.to_compact_str();
builder.scope.remove_binding(*scope_id, &name);
builder.scope.add_binding(current_scope_id, name, symbol_id);
declared_symbol_id = Some(symbol_id);
break;
}
}

let symbol_id =
builder.declare_symbol_on_scope(span, name, current_scope_id, includes, excludes);
let symbol_id = declared_symbol_id.unwrap_or_else(|| {
builder.declare_symbol_on_scope(span, name, current_scope_id, includes, excludes)
});
ident.symbol_id.set(Some(symbol_id));
for scope_id in &var_scope_ids {
builder.scope.add_binding(*scope_id, name.to_compact_str(), symbol_id);
builder.hosting_variables.entry(*scope_id).or_default().push(symbol_id);
}
});
}
Expand Down
25 changes: 18 additions & 7 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use oxc_cfg::{
use oxc_diagnostics::OxcDiagnostic;
use oxc_span::{CompactStr, SourceType, Span};
use oxc_syntax::{module_record::ModuleRecord, operator::AssignmentOperator};
use rustc_hash::FxHashMap;

use crate::{
binder::Binder,
Expand All @@ -26,7 +27,7 @@ use crate::{
module_record::ModuleRecordBuilder,
node::{AstNodeId, AstNodes, NodeFlags},
reference::{Reference, ReferenceFlag, ReferenceId},
scope::{ScopeFlags, ScopeId, ScopeTree, UnresolvedReferences},
scope::{Bindings, ScopeFlags, ScopeId, ScopeTree, UnresolvedReferences},
symbol::{SymbolFlags, SymbolId, SymbolTable},
JSDocFinder, Semantic,
};
Expand Down Expand Up @@ -67,6 +68,7 @@ pub struct SemanticBuilder<'a> {
// to value like
pub namespace_stack: Vec<SymbolId>,
current_reference_flag: ReferenceFlag,
pub hosting_variables: FxHashMap<ScopeId, Vec<SymbolId>>,

// builders
pub nodes: AstNodes<'a>,
Expand Down Expand Up @@ -127,6 +129,7 @@ impl<'a> SemanticBuilder<'a> {
function_stack: vec![],
namespace_stack: vec![],
nodes: AstNodes::default(),
hosting_variables: FxHashMap::default(),
scope,
symbols: SymbolTable::default(),
unresolved_references,
Expand Down Expand Up @@ -333,14 +336,21 @@ impl<'a> SemanticBuilder<'a> {
}

pub fn check_redeclaration(
&mut self,
&self,
scope_id: ScopeId,
span: Span,
name: &str,
excludes: SymbolFlags,
report_error: bool,
) -> Option<SymbolId> {
let symbol_id = self.scope.get_binding(scope_id, name)?;
let symbol_id = self.scope.get_binding(scope_id, name).or_else(|| {
self.hosting_variables
.get(&scope_id)
.and_then(|symbols| {
symbols.iter().find(|symbol_id| self.symbols.get_name(**symbol_id) == name)
})
.copied()
})?;
if report_error && self.symbols.get_flag(symbol_id).intersects(excludes) {
let symbol_span = self.symbols.get_span(symbol_id);
self.error(redeclaration(name, symbol_span, span));
Expand Down Expand Up @@ -480,10 +490,11 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

if let Some(parent_scope_id) = parent_scope_id {
if self.scope.get_flags(parent_scope_id).is_catch_clause() {
// Clone the `CatchClause` bindings and add them to the current scope.
// to make it easier to check redeclare errors.
let bindings = self.scope.get_bindings(parent_scope_id).clone();
self.scope.get_bindings_mut(self.current_scope_id).extend(bindings);
// Move all bindings from parent scope to current scope
// to make it easier to resole references and check redeclare errors.
let parent_bindings =
self.scope.get_bindings_mut(parent_scope_id).drain(..).collect::<Bindings>();
*self.scope.get_bindings_mut(self.current_scope_id) = parent_bindings;
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{symbol::SymbolId, AstNodeId};

type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;

type Bindings = FxIndexMap<CompactStr, SymbolId>;
pub(crate) type Bindings = FxIndexMap<CompactStr, SymbolId>;
pub(crate) type UnresolvedReference = (ReferenceId, ReferenceFlag);
pub(crate) type UnresolvedReferences = FxHashMap<CompactStr, Vec<UnresolvedReference>>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/catch/inherited
"flag": "ScopeFlags(StrictMode | CatchClause)",
"id": 2,
"node": "CatchClause",
"symbols": [
{
"flag": "SymbolFlags(FunctionScopedVariable | CatchVariable)",
"id": 1,
"name": "e",
"node": "CatchParameter",
"references": []
}
]
"symbols": []
}
],
"flag": "ScopeFlags(StrictMode | Top)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/catch/scope.ts
"flag": "ScopeFlags(StrictMode | CatchClause)",
"id": 2,
"node": "CatchClause",
"symbols": [
{
"flag": "SymbolFlags(FunctionScopedVariable | CatchVariable)",
"id": 0,
"name": "e",
"node": "CatchParameter",
"references": [
{
"flag": "ReferenceFlag(Read)",
"id": 0,
"name": "e",
"node_id": 8
}
]
}
]
"symbols": []
}
],
"flag": "ScopeFlags(StrictMode | Top)",
Expand Down
28 changes: 27 additions & 1 deletion tasks/coverage/parser_typescript.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ commit: d8086f14

parser_typescript Summary:
AST Parsed : 6444/6456 (99.81%)
Positive Passed: 6422/6456 (99.47%)
Positive Passed: 6421/6456 (99.46%)
Negative Passed: 1160/5653 (20.52%)
Expect Syntax Error: "compiler/ClassDeclaration10.ts"
Expect Syntax Error: "compiler/ClassDeclaration11.ts"
Expand Down Expand Up @@ -4639,6 +4639,19 @@ Expect to Parse: "compiler/withStatementInternalComments.ts"
2 │ /*1*/ with /*2*/ ( /*3*/ false /*4*/ ) /*5*/ {}
· ────
╰────
Expect to Parse: "conformance/async/es6/asyncWithVarShadowing_es6.ts"

× Identifier `x` has already been declared
╭─[conformance/async/es6/asyncWithVarShadowing_es6.ts:130:14]
129 │ }
130 │ catch ({ x }) {
· ┬
· ╰── `x` has already been declared here
131 │ var x;
· ┬
· ╰── It can not be redeclared here
132 │ }
╰────
Expect to Parse: "conformance/classes/propertyMemberDeclarations/staticPropertyNameConflicts.ts"

× Classes may not have a static property named prototype
Expand Down Expand Up @@ -6354,6 +6367,19 @@ Expect to Parse: "conformance/salsa/typeFromPropertyAssignmentWithExport.ts"
· ──
╰────

× Identifier `x` has already been declared
╭─[compiler/constDeclarationShadowedByVarDeclaration.ts:4:11]
3 │ {
4 │ const x = 0;
· ┬
· ╰── `x` has already been declared here
5 │
6 │ var x = 0;
· ┬
· ╰── It can not be redeclared here
7 │ }
╰────

× Identifier `y` has already been declared
╭─[compiler/constDeclarationShadowedByVarDeclaration.ts:12:11]
11 │ {
Expand Down

0 comments on commit 79bd289

Please sign in to comment.