Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(transformer/typescript): correctly resolve references to non-constant enum members #8543

Merged
merged 7 commits into from
Jan 18, 2025
Merged

fix(transformer/typescript): correctly resolve references to non-constant enum members #8543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
88f1689
fix(transformer/typescript): correctly resolve references to non-cons…
branchseer Jan 16, 2025
554801a
update snapshots
branchseer Jan 16, 2025
52c1fce
fix incorrect initial value
branchseer Jan 16, 2025
613a519
simplify control flow
branchseer Jan 16, 2025
244d1db
Re-order imports
overlookmotel Jan 16, 2025
aec2efc
remove unnecessary scope ancestor traverse
branchseer Jan 17, 2025
e68f8e9
Merge branch 'main' into non-constant-enum-member-ref
Dunqing Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 59 additions & 72 deletions crates/oxc_transformer/src/typescript/enum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use rustc_hash::FxHashMap;
use oxc_allocator::Vec as ArenaVec;
use oxc_ast::{ast::*, visit::walk_mut, VisitMut, NONE};
use oxc_ecmascript::ToInt32;
use oxc_semantic::ScopeId;
use oxc_span::{Atom, Span, SPAN};
use oxc_syntax::{
number::{NumberBase, ToJsString},
Expand All @@ -12,8 +13,11 @@ use oxc_syntax::{
};
use oxc_traverse::{BoundIdentifier, Traverse, TraverseCtx};

/// enum member values (or None if it can't be evaluated at build time) keyed by names
type PrevMembers<'a> = FxHashMap<Atom<'a>, Option<ConstantValue>>;

pub struct TypeScriptEnum<'a> {
enums: FxHashMap<Atom<'a>, FxHashMap<Atom<'a>, ConstantValue>>,
enums: FxHashMap<Atom<'a>, PrevMembers<'a>>,
}

impl TypeScriptEnum<'_> {
Expand Down Expand Up @@ -97,7 +101,8 @@ impl<'a> TypeScriptEnum<'a> {
// Foo[Foo["X"] = 0] = "X";
let is_already_declared = self.enums.contains_key(&enum_name);

let statements = self.transform_ts_enum_members(&mut decl.members, &param_binding, ctx);
let statements =
self.transform_ts_enum_members(decl.scope_id(), &mut decl.members, &param_binding, ctx);
let body = ast.alloc_function_body(decl.span, ast.vec(), statements);
let callee = Expression::FunctionExpression(ctx.ast.alloc_function_with_scope_id(
SPAN,
Expand Down Expand Up @@ -176,6 +181,7 @@ impl<'a> TypeScriptEnum<'a> {

fn transform_ts_enum_members(
&mut self,
enum_scope_id: ScopeId,
members: &mut ArenaVec<'a, TSEnumMember<'a>>,
param_binding: &BoundIdentifier<'a>,
ctx: &mut TraverseCtx<'a>,
Expand All @@ -199,44 +205,34 @@ impl<'a> TypeScriptEnum<'a> {
let constant_value =
self.computed_constant_value(initializer, &previous_enum_members);

previous_enum_members.insert(member_name.clone(), constant_value.clone());

// prev_constant_value = constant_value
let init = match constant_value {
None => {
prev_constant_value = None;
let mut new_initializer = ast.move_expression(initializer);

// If the initializer is a binding identifier,
// and it is not a binding in the current scope and parent scopes,
// we need to rename it to the enum name. e.g. `d = c` to `d = A.c`
// same behavior in https://github.com/babel/babel/blob/610897a9a96c5e344e77ca9665df7613d2f88358/packages/babel-plugin-transform-typescript/src/enum.ts#L145-L150
let has_binding = matches!(
&new_initializer,
Expression::Identifier(ident) if ctx.scopes().has_binding(ctx.current_scope_id(), &ident.name)
);
if !has_binding {
IdentifierReferenceRename::new(
param_binding.name.clone(),
previous_enum_members.clone(),
ctx,
)
.visit_expression(&mut new_initializer);
}
IdentifierReferenceRename::new(
param_binding.name.clone(),
overlookmotel marked this conversation as resolved.
Show resolved Hide resolved
enum_scope_id,
previous_enum_members.clone(),
ctx,
)
.visit_expression(&mut new_initializer);

new_initializer
}
Some(constant_value) => {
previous_enum_members.insert(member_name.clone(), constant_value.clone());
match constant_value {
ConstantValue::Number(v) => {
prev_constant_value = Some(ConstantValue::Number(v));
Self::get_initializer_expr(v, ctx)
}
ConstantValue::String(str) => {
prev_constant_value = None;
ast.expression_string_literal(SPAN, str, None)
}
Some(constant_value) => match constant_value {
ConstantValue::Number(v) => {
prev_constant_value = Some(ConstantValue::Number(v));
Self::get_initializer_expr(v, ctx)
}
}
ConstantValue::String(str) => {
prev_constant_value = None;
ast.expression_string_literal(SPAN, str, None)
}
},
};

init
Expand All @@ -246,12 +242,13 @@ impl<'a> TypeScriptEnum<'a> {
let value = value + 1.0;
let constant_value = ConstantValue::Number(value);
prev_constant_value = Some(constant_value.clone());
previous_enum_members.insert(member_name.clone(), constant_value);
previous_enum_members.insert(member_name.clone(), Some(constant_value));
Self::get_initializer_expr(value, ctx)
}
ConstantValue::String(_) => unreachable!(),
}
} else if let Some(prev_member_name) = prev_member_name {
previous_enum_members.insert(member_name.clone(), None);
let self_ref = {
let obj = param_binding.create_read_expression(ctx);
let expr = ctx.ast.expression_string_literal(SPAN, prev_member_name, None);
Expand All @@ -262,6 +259,7 @@ impl<'a> TypeScriptEnum<'a> {
let one = Self::get_number_literal_expression(1.0, ctx);
ast.expression_binary(SPAN, one, BinaryOperator::Addition, self_ref)
} else {
previous_enum_members.insert(member_name.clone(), Some(ConstantValue::Number(0.0)));
Self::get_number_literal_expression(0.0, ctx)
};

Expand Down Expand Up @@ -345,23 +343,23 @@ impl<'a> TypeScriptEnum<'a> {
fn computed_constant_value(
&self,
expr: &Expression<'a>,
prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
prev_members: &PrevMembers<'a>,
) -> Option<ConstantValue> {
self.evaluate(expr, prev_members)
}

fn evaluate_ref(
&self,
expr: &Expression<'a>,
prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
prev_members: &PrevMembers<'a>,
) -> Option<ConstantValue> {
match expr {
match_member_expression!(Expression) => {
let expr = expr.to_member_expression();
let Expression::Identifier(ident) = expr.object() else { return None };
let members = self.enums.get(&ident.name)?;
let property = expr.static_property_name()?;
members.get(property).cloned()
members.get(property).cloned()?
}
Expression::Identifier(ident) => {
if ident.name == "Infinity" {
Expand All @@ -371,7 +369,7 @@ impl<'a> TypeScriptEnum<'a> {
}

if let Some(value) = prev_members.get(&ident.name) {
return Some(value.clone());
return value.clone();
}

// TODO:
Expand All @@ -388,7 +386,7 @@ impl<'a> TypeScriptEnum<'a> {
fn evaluate(
&self,
expr: &Expression<'a>,
prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
prev_members: &PrevMembers<'a>,
) -> Option<ConstantValue> {
match expr {
Expression::Identifier(_)
Expand Down Expand Up @@ -417,7 +415,7 @@ impl<'a> TypeScriptEnum<'a> {
fn eval_binary_expression(
&self,
expr: &BinaryExpression<'a>,
prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
prev_members: &PrevMembers<'a>,
) -> Option<ConstantValue> {
let left = self.evaluate(&expr.left, prev_members)?;
let right = self.evaluate(&expr.right, prev_members)?;
Expand Down Expand Up @@ -482,7 +480,7 @@ impl<'a> TypeScriptEnum<'a> {
fn eval_unary_expression(
&self,
expr: &UnaryExpression<'a>,
prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
prev_members: &PrevMembers<'a>,
) -> Option<ConstantValue> {
let value = self.evaluate(&expr.argument, prev_members)?;

Expand Down Expand Up @@ -527,56 +525,45 @@ impl<'a> TypeScriptEnum<'a> {
/// ```
struct IdentifierReferenceRename<'a, 'ctx> {
enum_name: Atom<'a>,
enum_scope_id: ScopeId,
ctx: &'ctx TraverseCtx<'a>,
previous_enum_members: FxHashMap<Atom<'a>, ConstantValue>,
previous_enum_members: PrevMembers<'a>,
}

impl<'a, 'ctx> IdentifierReferenceRename<'a, 'ctx> {
fn new(
enum_name: Atom<'a>,
previous_enum_members: FxHashMap<Atom<'a>, ConstantValue>,
enum_scope_id: ScopeId,
previous_enum_members: PrevMembers<'a>,
ctx: &'ctx TraverseCtx<'a>,
) -> Self {
IdentifierReferenceRename { enum_name, ctx, previous_enum_members }
IdentifierReferenceRename { enum_name, enum_scope_id, ctx, previous_enum_members }
}
}

impl IdentifierReferenceRename<'_, '_> {
fn should_reference_enum_member(&self, ident: &IdentifierReference<'_>) -> bool {
let symbol_table = self.ctx.scoping.symbols();
let Some(symbol_id) = symbol_table.get_reference(ident.reference_id()).symbol_id() else {
// No symbol found. If the name is found in previous_enum_members,
// it must be referencing a member declared in a previous enum block: `enum Foo { A }; enum Foo { B = A }`
return self.previous_enum_members.contains_key(&ident.name);
};
symbol_table.get_scope_id(symbol_id) == self.enum_scope_id
}
}

impl<'a> VisitMut<'a> for IdentifierReferenceRename<'a, '_> {
fn visit_expression(&mut self, expr: &mut Expression<'a>) {
let new_expr = match expr {
match_member_expression!(Expression) => {
// handle a.toString() -> A.a.toString()
let expr = expr.to_member_expression();
if let Expression::Identifier(ident) = expr.object() {
if !self.previous_enum_members.contains_key(&ident.name) {
return;
}
};
None
}
Expression::Identifier(ident) => {
// If the identifier is binding in current/parent scopes,
// and it is not a member of the enum,
// we don't need to rename it.
// `var c = 1; enum A { a = c }` -> `var c = 1; enum A { a = c }
if !self.previous_enum_members.contains_key(&ident.name)
&& self.ctx.scopes().has_binding(self.ctx.current_scope_id(), &ident.name)
{
return;
}

// TODO: shadowed case, e.g. let ident = 1; ident; // ident is not an enum
// enum_name.identifier
match expr {
Expression::Identifier(ident) if self.should_reference_enum_member(ident) => {
let object = self.ctx.ast.expression_identifier_reference(SPAN, &self.enum_name);
let property = self.ctx.ast.identifier_name(SPAN, &ident.name);
Some(self.ctx.ast.member_expression_static(SPAN, object, property, false).into())
*expr = self.ctx.ast.member_expression_static(SPAN, object, property, false).into();
}
_ => {
walk_mut::walk_expression(self, expr);
}
_ => None,
};
if let Some(new_expr) = new_expr {
*expr = new_expr;
} else {
walk_mut::walk_expression(self, expr);
}
}
}
42 changes: 7 additions & 35 deletions tasks/coverage/snapshots/semantic_typescript.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3014,11 +3014,7 @@ after transform: SymbolId(14): [ReferenceId(12), ReferenceId(13), ReferenceId(14
rebuilt : SymbolId(3): [ReferenceId(0), ReferenceId(1), ReferenceId(2), ReferenceId(4), ReferenceId(5), ReferenceId(6), ReferenceId(7), ReferenceId(8), ReferenceId(9), ReferenceId(10), ReferenceId(11), ReferenceId(13), ReferenceId(14), ReferenceId(16), ReferenceId(17), ReferenceId(19), ReferenceId(20), ReferenceId(22), ReferenceId(23), ReferenceId(24), ReferenceId(25), ReferenceId(27), ReferenceId(28)]

tasks/coverage/typescript/tests/cases/compiler/computedEnumTypeWidening.ts
semantic error: Missing ReferenceId: "E"
Missing ReferenceId: "E"
Missing ReferenceId: "E"
Missing ReferenceId: "E"
Bindings mismatch:
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["C", "E", "E2", "MyDeclaredEnum", "MyEnum", "_defineProperty", "c1", "c2", "f1", "f2", "f3", "f4", "v1", "v2", "val1", "val2"]
rebuilt : ScopeId(0): ["C", "E", "MyEnum", "_defineProperty", "c1", "c2", "f1", "f2", "f3", "f4", "v1", "v2", "val1", "val2"]
Scope children mismatch:
Expand All @@ -3042,9 +3038,6 @@ rebuilt : SymbolId(1): SymbolFlags(FunctionScopedVariable)
Symbol reference IDs mismatch for "E":
after transform: SymbolId(1): [ReferenceId(4), ReferenceId(8), ReferenceId(9), ReferenceId(11), ReferenceId(15), ReferenceId(16), ReferenceId(17), ReferenceId(18), ReferenceId(25), ReferenceId(26), ReferenceId(27), ReferenceId(28), ReferenceId(35), ReferenceId(37), ReferenceId(38), ReferenceId(40), ReferenceId(41), ReferenceId(43), ReferenceId(44), ReferenceId(46), ReferenceId(50), ReferenceId(51), ReferenceId(52), ReferenceId(54), ReferenceId(55), ReferenceId(57), ReferenceId(58), ReferenceId(60), ReferenceId(61), ReferenceId(78)]
rebuilt : SymbolId(1): [ReferenceId(14), ReferenceId(15), ReferenceId(19), ReferenceId(24), ReferenceId(25), ReferenceId(32), ReferenceId(39), ReferenceId(41), ReferenceId(43), ReferenceId(45), ReferenceId(47), ReferenceId(50), ReferenceId(51), ReferenceId(52), ReferenceId(53), ReferenceId(54), ReferenceId(56), ReferenceId(58), ReferenceId(60), ReferenceId(62)]
Symbol reference IDs mismatch for "E":
after transform: SymbolId(61): [ReferenceId(69), ReferenceId(70), ReferenceId(71), ReferenceId(72), ReferenceId(73), ReferenceId(74), ReferenceId(75), ReferenceId(76), ReferenceId(77)]
rebuilt : SymbolId(2): [ReferenceId(1), ReferenceId(2), ReferenceId(3), ReferenceId(4), ReferenceId(5), ReferenceId(6), ReferenceId(7), ReferenceId(8), ReferenceId(9), ReferenceId(10), ReferenceId(11), ReferenceId(12), ReferenceId(13)]
Symbol flags mismatch for "MyEnum":
after transform: SymbolId(51): SymbolFlags(RegularEnum)
rebuilt : SymbolId(43): SymbolFlags(FunctionScopedVariable)
Expand All @@ -3059,7 +3052,7 @@ after transform: SymbolId(56) "MyDeclaredEnum"
rebuilt : <None>
Unresolved references mismatch:
after transform: ["computed", "const", "require"]
rebuilt : ["E2", "MyDeclaredEnum", "require"]
rebuilt : ["E2", "MyDeclaredEnum", "computed", "require"]

tasks/coverage/typescript/tests/cases/compiler/computedPropertiesTransformedInOtherwiseNonTSClasses.ts
semantic error: Scope flags mismatch:
Expand Down Expand Up @@ -16842,8 +16835,7 @@ after transform: ScopeId(0): ["Class1", "Class2", "decorate"]
rebuilt : ScopeId(0): ["Class2", "decorate"]

tasks/coverage/typescript/tests/cases/compiler/methodContainingLocalFunction.ts
semantic error: Missing ReferenceId: "E"
Scope flags mismatch:
semantic error: Scope flags mismatch:
after transform: ScopeId(13): ScopeFlags(StrictMode | Function)
rebuilt : ScopeId(13): ScopeFlags(Function)
Bindings mismatch:
Expand All @@ -16861,12 +16853,6 @@ rebuilt : SymbolId(14): Span { start: 0, end: 0 }
Symbol flags mismatch for "E":
after transform: SymbolId(23): SymbolFlags(RegularEnum)
rebuilt : SymbolId(19): SymbolFlags(FunctionScopedVariable)
Symbol reference IDs mismatch for "E":
after transform: SymbolId(28): [ReferenceId(18), ReferenceId(19), ReferenceId(20)]
rebuilt : SymbolId(20): [ReferenceId(14), ReferenceId(15), ReferenceId(17), ReferenceId(18)]
Symbol reference IDs mismatch for "localFunction":
after transform: SymbolId(25): [ReferenceId(13)]
rebuilt : SymbolId(21): []

tasks/coverage/typescript/tests/cases/compiler/methodSignatureDeclarationEmit1.ts
semantic error: Scope children mismatch:
Expand Down Expand Up @@ -16903,8 +16889,7 @@ after transform: ScopeId(0): [ScopeId(1), ScopeId(5), ScopeId(8)]
rebuilt : ScopeId(0): []

tasks/coverage/typescript/tests/cases/compiler/mixedTypeEnumComparison.ts
semantic error: Missing ReferenceId: "E2"
Bindings mismatch:
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["E", "E2", "someNumber", "someString", "unionOfEnum"]
rebuilt : ScopeId(0): ["E", "E2"]
Scope children mismatch:
Expand All @@ -16931,9 +16916,6 @@ rebuilt : SymbolId(0): [ReferenceId(7), ReferenceId(9), ReferenceId(11),
Symbol flags mismatch for "E2":
after transform: SymbolId(8): SymbolFlags(RegularEnum)
rebuilt : SymbolId(2): SymbolFlags(FunctionScopedVariable)
Symbol reference IDs mismatch for "E2":
after transform: SymbolId(13): [ReferenceId(29), ReferenceId(30), ReferenceId(31), ReferenceId(32), ReferenceId(33), ReferenceId(34)]
rebuilt : SymbolId(3): [ReferenceId(20), ReferenceId(21), ReferenceId(22), ReferenceId(23), ReferenceId(24), ReferenceId(25), ReferenceId(26)]
Reference symbol mismatch for "someNumber":
after transform: SymbolId(5) "someNumber"
rebuilt : <None>
Expand Down Expand Up @@ -16966,7 +16948,7 @@ after transform: SymbolId(5) "someNumber"
rebuilt : <None>
Unresolved references mismatch:
after transform: ["someValue"]
rebuilt : ["someNumber", "someString", "unionOfEnum"]
rebuilt : ["someNumber", "someString", "someValue", "unionOfEnum"]

tasks/coverage/typescript/tests/cases/compiler/mixinIntersectionIsValidbaseType.ts
semantic error: Scope children mismatch:
Expand Down Expand Up @@ -19069,11 +19051,7 @@ after transform: SymbolId(0): [ReferenceId(0), ReferenceId(7)]
rebuilt : SymbolId(0): [ReferenceId(5)]

tasks/coverage/typescript/tests/cases/compiler/numericEnumMappedType.ts
semantic error: Missing ReferenceId: "N1"
Missing ReferenceId: "N1"
Missing ReferenceId: "N2"
Missing ReferenceId: "N2"
Bindings mismatch:
semantic error: Bindings mismatch:
after transform: ScopeId(0): ["E", "E1", "E2", "N1", "N2", "b1", "b2", "e", "e1", "e2", "x"]
rebuilt : ScopeId(0): ["E1", "N1", "N2", "b1", "b2", "e", "e1", "e2", "x"]
Scope children mismatch:
Expand Down Expand Up @@ -19109,18 +19087,12 @@ rebuilt : SymbolId(6): SymbolFlags(FunctionScopedVariable)
Symbol reference IDs mismatch for "N1":
after transform: SymbolId(16): [ReferenceId(18), ReferenceId(38)]
rebuilt : SymbolId(6): [ReferenceId(23)]
Symbol reference IDs mismatch for "N1":
after transform: SymbolId(31): [ReferenceId(33), ReferenceId(34), ReferenceId(35), ReferenceId(36), ReferenceId(37)]
rebuilt : SymbolId(7): [ReferenceId(16), ReferenceId(17), ReferenceId(18), ReferenceId(19), ReferenceId(20), ReferenceId(21), ReferenceId(22)]
Symbol flags mismatch for "N2":
after transform: SymbolId(19): SymbolFlags(RegularEnum)
rebuilt : SymbolId(8): SymbolFlags(FunctionScopedVariable)
Symbol reference IDs mismatch for "N2":
after transform: SymbolId(19): [ReferenceId(19), ReferenceId(44)]
rebuilt : SymbolId(8): [ReferenceId(31)]
Symbol reference IDs mismatch for "N2":
after transform: SymbolId(32): [ReferenceId(39), ReferenceId(40), ReferenceId(41), ReferenceId(42), ReferenceId(43)]
rebuilt : SymbolId(9): [ReferenceId(24), ReferenceId(25), ReferenceId(26), ReferenceId(27), ReferenceId(28), ReferenceId(29), ReferenceId(30)]
Reference symbol mismatch for "E2":
after transform: SymbolId(4) "E2"
rebuilt : <None>
Expand All @@ -19129,7 +19101,7 @@ after transform: SymbolId(24) "E"
rebuilt : <None>
Unresolved references mismatch:
after transform: ["val"]
rebuilt : ["E", "E2"]
rebuilt : ["E", "E2", "val"]

tasks/coverage/typescript/tests/cases/compiler/numericIndexerConstraint3.ts
semantic error: Symbol reference IDs mismatch for "A":
Expand Down
Loading
Loading