Skip to content

Commit

Permalink
refactor(transformer): use ctx.var_declarations.create_var* methods (
Browse files Browse the repository at this point in the history
…#7666)

It turns out these APIs are very useful, we remove many boilerplate code.
  • Loading branch information
Dunqing committed Dec 5, 2024
1 parent 4e489bd commit 0ca10e2
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 185 deletions.
3 changes: 0 additions & 3 deletions crates/oxc_transformer/src/common/var_declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ impl<'a> VarDeclarationsStore<'a> {
/// Create a new [`BoundIdentifier`], add a var declaration to be inserted at the top of
/// the current enclosing statement block, and then return the [`BoundIdentifier`].
#[inline]
#[expect(unused)]
pub fn create_var(&self, name: &str, ctx: &mut TraverseCtx<'a>) -> BoundIdentifier<'a> {
let binding = ctx.generate_uid_in_current_hoist_scope(name);
self.insert_var(&binding, None, ctx);
Expand All @@ -112,7 +111,6 @@ impl<'a> VarDeclarationsStore<'a> {
/// to be inserted at the top of the current enclosing statement block, and then return
/// the [`BoundIdentifier`].
#[inline]
#[expect(unused)]
pub fn create_var_with_init(
&self,
name: &str,
Expand All @@ -127,7 +125,6 @@ impl<'a> VarDeclarationsStore<'a> {
/// Create a new [`BoundIdentifier`] based on node, add a var declaration to be inserted
/// at the top of the current enclosing statement block, and then return the [`BoundIdentifier`].
#[inline]
#[expect(unused)]
pub fn create_var_based_on_node<N: GatherNodeParts<'a>>(
&self,
node: &N,
Expand Down
9 changes: 2 additions & 7 deletions crates/oxc_transformer/src/es2016/exponentiation_operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
use oxc_allocator::{CloneIn, Vec as ArenaVec};
use oxc_ast::{ast::*, NONE};
use oxc_semantic::{ReferenceFlags, SymbolFlags};
use oxc_semantic::ReferenceFlags;
use oxc_span::SPAN;
use oxc_syntax::operator::{AssignmentOperator, BinaryOperator};
use oxc_traverse::{BoundIdentifier, Traverse, TraverseCtx};
Expand Down Expand Up @@ -558,13 +558,8 @@ impl<'a, 'ctx> ExponentiationOperator<'a, 'ctx> {
temp_var_inits: &mut ArenaVec<'a, Expression<'a>>,
ctx: &mut TraverseCtx<'a>,
) -> BoundIdentifier<'a> {
let binding = ctx.generate_uid_in_current_scope_based_on_node(
&expr,
SymbolFlags::FunctionScopedVariable,
);

// var _name;
self.ctx.var_declarations.insert_var(&binding, None, ctx);
let binding = self.ctx.var_declarations.create_var_based_on_node(&expr, ctx);

// Add new reference `_name = name` to `temp_var_inits`
temp_var_inits.push(ctx.ast.expression_assignment(
Expand Down
22 changes: 5 additions & 17 deletions crates/oxc_transformer/src/es2020/optional_chaining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ use std::mem;

use oxc_allocator::CloneIn;
use oxc_ast::{ast::*, NONE};
use oxc_semantic::SymbolFlags;
use oxc_span::SPAN;
use oxc_traverse::{Ancestor, BoundIdentifier, MaybeBoundIdentifier, Traverse, TraverseCtx};

Expand Down Expand Up @@ -296,18 +295,6 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
}
}

/// Generate a binding based on the node and insert the binding at the top of the current block
fn generate_binding(
&mut self,
expr: &Expression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> BoundIdentifier<'a> {
let binding = ctx
.generate_uid_in_current_scope_based_on_node(expr, SymbolFlags::FunctionScopedVariable);
self.ctx.var_declarations.insert_var(&binding, None, ctx);
binding
}

/// Return `left = right`
fn create_assignment_expression(
left: AssignmentTarget<'a>,
Expand Down Expand Up @@ -409,7 +396,7 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
.create_read_expression(ctx)
} else {
// `foo.bar` -> `_foo$bar = foo.bar`
let binding = self.generate_binding(object, ctx);
let binding = self.ctx.var_declarations.create_var_based_on_node(object, ctx);
*object = Self::create_assignment_expression(
binding.create_write_target(ctx),
ctx.ast.move_expression(object),
Expand Down Expand Up @@ -584,7 +571,7 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
}

// We should generate a temp binding for the expression first to avoid the next step changing the expression.
let temp_binding = self.generate_binding(expr, ctx);
let temp_binding = self.ctx.var_declarations.create_var_based_on_node(expr, ctx);
if is_call && !self.ctx.assumptions.pure_getters {
if let Some(member) = expr.as_member_expression_mut() {
let object = member.object_mut();
Expand All @@ -593,7 +580,8 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
if let Expression::Identifier(ident) = object {
let binding =
self.get_existing_binding_for_identifier(ident, ctx).unwrap_or_else(|| {
let binding = self.generate_binding(object, ctx);
let binding =
self.ctx.var_declarations.create_var_based_on_node(object, ctx);
// `(_foo = foo)`
*object = Self::create_assignment_expression(
binding.create_write_target(ctx),
Expand Down Expand Up @@ -650,7 +638,7 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {

let temp_binding = {
if self.temp_binding.is_none() {
let binding = self.generate_binding(expr, ctx);
let binding = self.ctx.var_declarations.create_var_based_on_node(expr, ctx);
self.set_temp_binding(binding);
}
self.temp_binding.as_ref().unwrap()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
use oxc_allocator::CloneIn;
use oxc_ast::ast::*;
use oxc_semantic::{ReferenceFlags, SymbolFlags};
use oxc_semantic::ReferenceFlags;
use oxc_span::SPAN;
use oxc_syntax::operator::{AssignmentOperator, LogicalOperator};
use oxc_traverse::{BoundIdentifier, MaybeBoundIdentifier, Traverse, TraverseCtx};
Expand Down Expand Up @@ -310,12 +310,6 @@ impl<'a, 'ctx> LogicalAssignmentOperators<'a, 'ctx> {
if ctx.is_static(expr) {
return None;
}

// var _name;
let binding = ctx
.generate_uid_in_current_scope_based_on_node(expr, SymbolFlags::FunctionScopedVariable);
self.ctx.var_declarations.insert_var(&binding, None, ctx);

Some(binding)
Some(self.ctx.var_declarations.create_var_based_on_node(expr, ctx))
}
}
37 changes: 8 additions & 29 deletions crates/oxc_transformer/src/es2022/class_properties/private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use oxc_ast::{ast::*, NONE};
use oxc_span::SPAN;
use oxc_syntax::{
reference::{ReferenceFlags, ReferenceId},
symbol::{SymbolFlags, SymbolId},
symbol::SymbolId,
};
use oxc_traverse::{
ast_operations::get_var_name_from_node, Ancestor, BoundIdentifier, MaybeBoundIdentifier,
Expand Down Expand Up @@ -676,10 +676,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let prop_ident2 = prop_binding.create_read_expression(ctx);

let temp_var_name_base = get_var_name_from_node(field_expr);
let temp_binding = ctx.generate_uid_in_current_scope(
&temp_var_name_base,
SymbolFlags::FunctionScopedVariable,
);

// TODO(improve-on-babel): Could avoid `move_expression` here and replace `update_expr.argument` instead.
// Only doing this first to match the order Babel creates temp vars.
Expand Down Expand Up @@ -739,7 +735,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
};

// `_object$prop = _assertClassBrand(Class, object, _prop)._`
self.ctx.var_declarations.insert_var(&temp_binding, None, ctx);
let temp_binding = self.ctx.var_declarations.create_var(&temp_var_name_base, ctx);
let assignment = create_assignment(&temp_binding, get_expr, ctx);

// `++_object$prop` / `_object$prop++` (reusing existing `UpdateExpression`)
Expand Down Expand Up @@ -772,11 +768,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// Source = `object.#prop++` (postfix `++`)

// `_object$prop2 = _object$prop++`
let temp_binding2 = ctx.generate_uid_in_current_scope(
&temp_var_name_base,
SymbolFlags::FunctionScopedVariable,
);
self.ctx.var_declarations.insert_var(&temp_binding2, None, ctx);
let temp_binding2 = self.ctx.var_declarations.create_var(&temp_var_name_base, ctx);
let assignment2 = create_assignment(&temp_binding2, update_expr, ctx);

// `(_object$prop = _assertClassBrand(Class, object, _prop)._, _object$prop2 = _object$prop++, _object$prop)`
Expand Down Expand Up @@ -819,7 +811,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let get_call = self.create_private_field_get(prop_ident, object2, SPAN, ctx);

// `_object$prop = _classPrivateFieldGet(_prop, object)`
self.ctx.var_declarations.insert_var(&temp_binding, None, ctx);
let temp_binding = self.ctx.var_declarations.create_var(&temp_var_name_base, ctx);
let assignment = create_assignment(&temp_binding, get_call, ctx);

// `++_object$prop` / `_object$prop++` (reusing existing `UpdateExpression`)
Expand All @@ -839,11 +831,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
} else {
// Source = `object.#prop++` (postfix `++`)
// `_object$prop2 = _object$prop++`
let temp_binding2 = ctx.generate_uid_in_current_scope(
&temp_var_name_base,
SymbolFlags::FunctionScopedVariable,
);
self.ctx.var_declarations.insert_var(&temp_binding2, None, ctx);
let temp_binding2 = self.ctx.var_declarations.create_var(&temp_var_name_base, ctx);
let assignment2 = create_assignment(&temp_binding2, update_expr, ctx);

// `(_object$prop = _classPrivateFieldGet(_prop, object), _object$prop2 = _object$prop++, _object$prop)`
Expand Down Expand Up @@ -1131,11 +1119,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// `A.B` -> `(_A$B = A.B) === null || _A$B === void 0`
// TODO: should add an API `generate_uid_in_current_hoist_scope_based_on_node` to instead this
let temp_var_binding = ctx.generate_uid_in_current_scope_based_on_node(
object,
SymbolFlags::FunctionScopedVariable,
);
self.ctx.var_declarations.insert_var(&temp_var_binding, None, ctx);
let temp_var_binding = self.ctx.var_declarations.create_var_based_on_node(object, ctx);

let object = mem::replace(object, temp_var_binding.create_read_expression(ctx));
let assignment = create_assignment(
Expand Down Expand Up @@ -1521,21 +1505,16 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// Previously `x += 1` (`x` read + write), but moving to `_x = x` (`x` read only)
*reference.flags_mut() = ReferenceFlags::Read;

ctx.generate_uid_in_current_scope(&ident.name, SymbolFlags::FunctionScopedVariable)
self.ctx.var_declarations.create_var(&ident.name, ctx)
}
Expression::ThisExpression(this) => {
// Reading `this` cannot have side effects, so no need for temp var
let object1 = ctx.ast.expression_this(this.span);
return (object1, object);
}
_ => ctx.generate_uid_in_current_scope_based_on_node(
&object,
SymbolFlags::FunctionScopedVariable,
),
_ => self.ctx.var_declarations.create_var_based_on_node(&object, ctx),
};

self.ctx.var_declarations.insert_var(&temp_var_binding, None, ctx);

let object1 = create_assignment(&temp_var_binding, object, ctx);
let object2 = temp_var_binding.create_read_expression(ctx);

Expand Down
23 changes: 9 additions & 14 deletions crates/oxc_transformer/src/jsx/refresh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,15 @@ impl<'a, 'ctx> ReactRefresh<'a, 'ctx> {
arguments.push(function);
}

let binding = ctx.generate_uid_in_current_hoist_scope("s");
// _s = refresh_sig();
let init = ctx.ast.expression_call(
SPAN,
self.refresh_sig.to_expression(ctx),
NONE,
ctx.ast.vec(),
false,
);
let binding = self.ctx.var_declarations.create_var_with_init("s", init, ctx);

// _s();
let call_expression = ctx.ast.statement_expression(
Expand All @@ -598,19 +606,6 @@ impl<'a, 'ctx> ReactRefresh<'a, 'ctx> {

body.statements.insert(0, call_expression);

// _s = refresh_sig();
self.ctx.var_declarations.insert_var(
&binding,
Some(ctx.ast.expression_call(
SPAN,
self.refresh_sig.to_expression(ctx),
NONE,
ctx.ast.vec(),
false,
)),
ctx,
);

// Following is the signature call expression, will be generated in call site.
// _s(App, signature_key, false, function() { return [] });
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ custom hooks only
Expand Down
9 changes: 0 additions & 9 deletions tasks/coverage/snapshots/semantic_misc.snap
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,6 @@ tasks/coverage/misc/pass/oxc-3948-1.ts
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["BrowserWorkingCopyBackupTracker", "CancellationToken", "DisposableStore", "EditorPart", "EditorService", "IEditorGroupsService", "IEditorService", "IFilesConfigurationService", "IInstantiationService", "ILifecycleService", "ILogService", "IUntitledTextResourceEditorInput", "IWorkingCopyBackup", "IWorkingCopyBackupService", "IWorkingCopyEditorHandler", "IWorkingCopyEditorService", "IWorkingCopyService", "InMemoryTestWorkingCopyBackupService", "LifecyclePhase", "Schemas", "TestServiceAccessor", "TestWorkingCopy", "URI", "UntitledTextEditorInput", "VSBuffer", "_asyncToGenerator", "assert", "bufferToReadable", "createEditorPart", "ensureNoDisposablesAreLeakedInTestSuite", "isWindows", "registerTestResourceEditor", "timeout", "toResource", "toTypedWorkingCopyId", "toUntypedWorkingCopyId", "workbenchInstantiationService", "workbenchTeardown"]
rebuilt : ScopeId(0): ["BrowserWorkingCopyBackupTracker", "DisposableStore", "EditorService", "IEditorGroupsService", "IEditorService", "IFilesConfigurationService", "ILifecycleService", "ILogService", "IWorkingCopyBackupService", "IWorkingCopyEditorService", "IWorkingCopyService", "InMemoryTestWorkingCopyBackupService", "LifecyclePhase", "Schemas", "TestServiceAccessor", "TestWorkingCopy", "URI", "UntitledTextEditorInput", "VSBuffer", "_asyncToGenerator", "assert", "bufferToReadable", "createEditorPart", "ensureNoDisposablesAreLeakedInTestSuite", "isWindows", "registerTestResourceEditor", "timeout", "toResource", "toTypedWorkingCopyId", "toUntypedWorkingCopyId", "workbenchInstantiationService", "workbenchTeardown"]
Bindings mismatch:
after transform: ScopeId(13): ["_await$accessor$edito", "accessor", "untitled", "untitledTextEditor", "untitledTextModel", "workingCopyBackupService"]
rebuilt : ScopeId(20): ["_await$accessor$edito", "_untitledTextModel$te", "accessor", "untitled", "untitledTextEditor", "untitledTextModel", "workingCopyBackupService"]
Bindings mismatch:
after transform: ScopeId(14): ["_untitledTextModel$te"]
rebuilt : ScopeId(21): []
Symbol reference IDs mismatch for "URI":
after transform: SymbolId(1): [ReferenceId(109), ReferenceId(117), ReferenceId(156), ReferenceId(158), ReferenceId(160), ReferenceId(162)]
rebuilt : SymbolId(1): [ReferenceId(158), ReferenceId(160), ReferenceId(162), ReferenceId(164)]
Expand Down Expand Up @@ -103,9 +97,6 @@ rebuilt : SymbolId(26): [ReferenceId(16)]
Symbol reference IDs mismatch for "TestWorkingCopyBackupTracker":
after transform: SymbolId(39): [ReferenceId(42), ReferenceId(74), ReferenceId(154), ReferenceId(215)]
rebuilt : SymbolId(34): [ReferenceId(65), ReferenceId(216)]
Symbol scope ID mismatch for "_untitledTextModel$te":
after transform: SymbolId(138): ScopeId(14)
rebuilt : SymbolId(63): ScopeId(20)
Unresolved reference IDs mismatch for "Promise":
after transform: [ReferenceId(36), ReferenceId(39), ReferenceId(82), ReferenceId(114), ReferenceId(153), ReferenceId(282)]
rebuilt : [ReferenceId(289)]
Expand Down
39 changes: 1 addition & 38 deletions tasks/coverage/snapshots/semantic_test262.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ commit: fd979d85

semantic_test262 Summary:
AST Parsed : 44026/44026 (100.00%)
Positive Passed: 43495/44026 (98.79%)
Positive Passed: 43497/44026 (98.80%)
tasks/coverage/test262/test/annexB/language/function-code/if-decl-else-decl-a-func-block-scoping.js
semantic error: Symbol scope ID mismatch for "f":
after transform: SymbolId(3): ScopeId(4294967294)
Expand Down Expand Up @@ -3740,43 +3740,6 @@ Unresolved references mismatch:
after transform: ["$DONE", "Object", "assert", "require"]
rebuilt : ["$DONE", "Object", "_superprop_getMethod", "assert", "require"]

tasks/coverage/test262/test/language/expressions/optional-chaining/iteration-statement-for-of-type-error.js
semantic error: Bindings mismatch:
after transform: ScopeId(1): []
rebuilt : ScopeId(1): ["_ref"]
Bindings mismatch:
after transform: ScopeId(2): ["_ref", "key"]
rebuilt : ScopeId(2): ["key"]
Bindings mismatch:
after transform: ScopeId(3): []
rebuilt : ScopeId(3): ["_ref2"]
Bindings mismatch:
after transform: ScopeId(4): ["_ref2", "key"]
rebuilt : ScopeId(4): ["key"]
Symbol scope ID mismatch for "_ref":
after transform: SymbolId(5): ScopeId(2)
rebuilt : SymbolId(0): ScopeId(1)
Symbol scope ID mismatch for "_ref2":
after transform: SymbolId(6): ScopeId(4)
rebuilt : SymbolId(2): ScopeId(3)

tasks/coverage/test262/test/language/expressions/optional-chaining/iteration-statement-for.js
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["count", "count2", "obj", "obj2", "obj3", "touched"]
rebuilt : ScopeId(0): ["_obj3$a", "_undefined", "count", "count2", "obj", "obj2", "obj3", "touched"]
Bindings mismatch:
after transform: ScopeId(5): ["_undefined"]
rebuilt : ScopeId(5): []
Bindings mismatch:
after transform: ScopeId(8): ["_obj3$a"]
rebuilt : ScopeId(8): []
Symbol scope ID mismatch for "_undefined":
after transform: SymbolId(6): ScopeId(5)
rebuilt : SymbolId(0): ScopeId(0)
Symbol scope ID mismatch for "_obj3$a":
after transform: SymbolId(7): ScopeId(8)
rebuilt : SymbolId(1): ScopeId(0)

tasks/coverage/test262/test/language/module-code/top-level-await/syntax/for-await-await-expr-func-expression.js
semantic error: Scope children mismatch:
after transform: ScopeId(14): [ScopeId(1)]
Expand Down
Loading

0 comments on commit 0ca10e2

Please sign in to comment.