dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-24 12:21:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(transformer): use UIDs in TS namespace transforms (#3395)

Browse source 
Use the `TraverseCtx::generate_uid` method introduced in #3395 to fix
some of the TS namespace test cases.

But... I honestly have no idea what I'm doing here!

I am running up against a combination of 3 different areas where I know
very little:

1. I am unfamiliar with `Semantic`.
2. I am unfamiliar with Oxc's conventions for writing transforms.
3. I don't "speak" Typescript!

This PR should not be merged as is. I'm pretty sure it's wrong. I've
done it mostly just to test out `generate_uid`.

In particular:

1. Is `SymbolFlags::FunctionScopedVariable` the right flags to use in
all cases here?
2. `generate_uid` creates a symbol, and registers a binding for it in
the scope tree. So if there are any branches in this logic where `name`
doesn't actually get used after `generate_uid` is called, then it's not
right.
3. Identifiers which are added to AST should also have corresponding
`ReferenceId`s created for them (but I imagine this is also missing in
many other places in the transformer).

On point 2: We could split up `generate_uid` into 2 functions. One
function would find a valid UID name *but not register a binding for
it*. If you want to actually use that name, you'd call a 2nd function to
generate the binding. Would that be a better API?

Could someone help me out to progress this please?

(Sorry my lack of knowledge is a bit useless here. I will learn all
these things in time, but just trying to be honest about where I'm at
right now. I'm sure I could figure it out myself, but it would take me
hours, whereas others will probably look at it and know what to do in
about 5 mins.)
This commit is contained in:
overlookmotel 2024-05-27 01:53:13 +01:00 committed by GitHub
parent 7fd4deb7e2
commit d4371e8f95
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 36 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
crates/oxc_semantic/src/scope.rs
View file

@ -151,6 +151,10 @@ impl ScopeTree {
self.bindings[scope_id].insert(name, symbol_id);
}
pub fn remove_binding(&mut self, scope_id: ScopeId, name: &CompactStr) {
self.bindings[scope_id].shift_remove(name);
}
pub(crate) fn add_unresolved_reference(
&mut self,
scope_id: ScopeId,

4
crates/oxc_transformer/src/lib.rs
View file

@ -92,8 +92,8 @@ impl<'a> Transformer<'a> {
}
impl<'a> Traverse<'a> for Transformer<'a> {
fn enter_program(&mut self, program: &mut Program<'a>, _ctx: &mut TraverseCtx<'a>) {
self.x0_typescript.transform_program(program);
fn enter_program(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
self.x0_typescript.transform_program(program, ctx);
}
fn exit_program(&mut self, program: &mut Program<'a>, _ctx: &mut TraverseCtx<'a>) {

4
crates/oxc_transformer/src/typescript/mod.rs
View file

@ -68,8 +68,8 @@ impl<'a> TypeScript<'a> {
// Transforms
impl<'a> TypeScript<'a> {
pub fn transform_program(&self, program: &mut Program<'a>) {
self.transform_program_for_namespace(program);
pub fn transform_program(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx) {
self.transform_program_for_namespace(program, ctx);
}
pub fn transform_program_on_exit(&self, program: &mut Program<'a>) {

33
crates/oxc_transformer/src/typescript/namespace.rs
View file

@ -4,14 +4,22 @@ use super::{diagnostics::ambient_module_nested, TypeScript};
use oxc_allocator::{Box, Vec};
use oxc_ast::{ast::*, syntax_directed_operations::BoundNames};
use oxc_span::{Atom, SPAN};
use oxc_syntax::operator::{AssignmentOperator, LogicalOperator};
use oxc_span::{Atom, CompactStr, SPAN};
use oxc_syntax::{
operator::{AssignmentOperator, LogicalOperator},
symbol::SymbolFlags,
};
use oxc_traverse::TraverseCtx;
// TODO:
// 1. register scope for the newly created function: <https://github.com/babel/babel/blob/08b0472069cd207f043dd40a4d157addfdd36011/packages/babel-plugin-transform-typescript/src/namespace.ts#L38>
impl<'a> TypeScript<'a> {
// `namespace Foo { }` -> `let Foo; (function (_Foo) { })(Foo || (Foo = {}));`
pub(super) fn transform_program_for_namespace(&self, program: &mut Program<'a>) {
pub(super) fn transform_program_for_namespace(
&self,
program: &mut Program<'a>,
ctx: &mut TraverseCtx,
) {
// namespace declaration is only allowed at the top level
if !has_namespace(program.body.as_slice()) {
@ -31,7 +39,7 @@ impl<'a> TypeScript<'a> {
Statement::TSModuleDeclaration(decl) => {
if !decl.modifiers.is_contains_declare() {
if let Some(transformed_stmt) =
self.handle_nested(self.ctx.ast.copy(&decl).unbox(), None)
self.handle_nested(self.ctx.ast.copy(&decl).unbox(), None, ctx)
{
let name = decl.id.name();
if names.insert(name.clone()) {
@ -51,7 +59,7 @@ impl<'a> TypeScript<'a> {
{
if !decl.modifiers.is_contains_declare() {
if let Some(transformed_stmt) =
self.handle_nested(self.ctx.ast.copy(decl), None)
self.handle_nested(self.ctx.ast.copy(decl), None, ctx)
{
let name = decl.id.name();
if names.insert(name.clone()) {
@ -119,12 +127,17 @@ impl<'a> TypeScript<'a> {
&self,
decl: TSModuleDeclaration<'a>,
parent_export: Option<Expression<'a>>,
ctx: &mut TraverseCtx,
) -> Option<Statement<'a>> {
let mut names: FxHashSet<Atom<'a>> = FxHashSet::default();
let real_name = decl.id.name();
let name = self.ctx.ast.new_atom(&format!("_{}", real_name.clone())); // path.scope.generateUid(realName.name);
// TODO: This binding is created in wrong scope.
// Needs to be created in scope of function which `transform_namespace` creates below.
let name = self.ctx.ast.new_atom(
&ctx.generate_uid_in_current_scope(real_name, SymbolFlags::FunctionScopedVariable),
);
let namespace_top_level = if let Some(body) = decl.body {
match body {
@ -161,7 +174,7 @@ impl<'a> TypeScript<'a> {
}
let module_name = decl.id.name().clone();
if let Some(transformed) = self.handle_nested(decl.unbox(), None) {
if let Some(transformed) = self.handle_nested(decl.unbox(), None, ctx) {
is_empty = false;
if names.insert(module_name.clone()) {
new_stmts.push(Statement::from(
@ -233,6 +246,7 @@ impl<'a> TypeScript<'a> {
Some(self.ctx.ast.identifier_reference_expression(
IdentifierReference::new(SPAN, name.clone()),
)),
ctx,
) {
is_empty = false;
if names.insert(module_name.clone()) {
@ -267,6 +281,11 @@ impl<'a> TypeScript<'a> {
}
if is_empty {
// Delete the scope binding that `ctx.generate_uid_in_current_scope` created above,
// as no binding is actually being created
let current_scope_id = ctx.current_scope_id();
ctx.scopes_mut().remove_binding(current_scope_id, &CompactStr::from(name.as_str()));
return None;
}

8
tasks/transform_conformance/babel.snap.md
View file

@ -1,6 +1,6 @@
commit: 4bd1b2c2
Passed: 309/351
Passed: 313/351
# All Passed:
* babel-preset-react
@ -21,18 +21,14 @@ Passed: 309/351
* opts/optimizeConstEnums/input.ts
* opts/rewriteImportExtensions/input.ts
# babel-plugin-transform-typescript (123/154)
# babel-plugin-transform-typescript (127/154)
* enum/mix-references/input.ts
* enum/scoped/input.ts
* enum/ts5.0-const-foldable/input.ts
* exports/declared-types/input.ts
* imports/enum-value/input.ts
* imports/type-only-export-specifier-2/input.ts
* namespace/contentious-names/input.ts
* namespace/empty-removed/input.ts
* namespace/module-nested/input.ts
* namespace/module-nested-export/input.ts
* namespace/multiple/input.ts
* namespace/mutable-fail/input.ts
* namespace/namespace-flag/input.ts
* namespace/namespace-nested-module/input.ts