From df772411eadcdddfaefcf6a896f55a43d38b5cd9 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Wed, 6 Nov 2024 03:02:36 +0000 Subject: [PATCH] feat(transformer): enable `ArrowFunctionConverter` in `async-to-generator` and `async-generator-functions` plugins (#7113) Part of https://github.com/oxc-project/oxc/pull/7074 In async-to-generator and async-generator-functions plugins, we may need to transform the async arrow function to a regular generator function, now we can reuse the ability of the ArrowFunction plugin by enabling `ArrowFunctionConverter`. I will fix semantic errors in the follow-up PR --- .../src/common/arrow_function_converter.rs | 62 ++++++++++++++----- crates/oxc_transformer/src/common/mod.rs | 17 +---- .../src/es2017/async_to_generator.rs | 9 ++- .../es2018/async_generator_functions/mod.rs | 6 +- .../snapshots/babel.snap.md | 30 ++++++++- .../snapshots/babel_exec.snap.md | 30 --------- 6 files changed, 90 insertions(+), 64 deletions(-) diff --git a/crates/oxc_transformer/src/common/arrow_function_converter.rs b/crates/oxc_transformer/src/common/arrow_function_converter.rs index 4c99899cb..993c44d5a 100644 --- a/crates/oxc_transformer/src/common/arrow_function_converter.rs +++ b/crates/oxc_transformer/src/common/arrow_function_converter.rs @@ -97,6 +97,8 @@ use oxc_syntax::{ }; use oxc_traverse::{Ancestor, BoundIdentifier, Traverse, TraverseCtx}; +use crate::TransformOptions; + /// Mode for arrow function conversion #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ArrowFunctionConverterMode { @@ -107,23 +109,27 @@ pub enum ArrowFunctionConverterMode { Enabled, /// Only convert async arrow functions - #[expect(unused)] AsyncOnly, } -pub struct ArrowFunctionConverterOptions { - pub mode: ArrowFunctionConverterMode, -} - pub struct ArrowFunctionConverter<'a> { mode: ArrowFunctionConverterMode, this_var_stack: SparseStack>, } impl<'a> ArrowFunctionConverter<'a> { - pub fn new(options: &ArrowFunctionConverterOptions) -> Self { + pub fn new(options: &TransformOptions) -> Self { + let mode = if options.env.es2015.arrow_function.is_some() { + ArrowFunctionConverterMode::Enabled + } else if options.env.es2017.async_to_generator + || options.env.es2018.async_generator_functions + { + ArrowFunctionConverterMode::AsyncOnly + } else { + ArrowFunctionConverterMode::Disabled + }; // `SparseStack` is created with 1 empty entry, for `Program` - Self { mode: options.mode, this_var_stack: SparseStack::new() } + Self { mode, this_var_stack: SparseStack::new() } } } @@ -254,7 +260,11 @@ impl<'a> Traverse<'a> for ArrowFunctionConverter<'a> { return; } - if let Expression::ArrowFunctionExpression(_) = expr { + if let Expression::ArrowFunctionExpression(arrow_function_expr) = expr { + if self.is_async_only() && !arrow_function_expr.r#async { + return; + } + let Expression::ArrowFunctionExpression(arrow_function_expr) = ctx.ast.move_expression(expr) else { @@ -272,13 +282,18 @@ impl<'a> ArrowFunctionConverter<'a> { self.mode == ArrowFunctionConverterMode::Disabled } + /// Check if arrow function conversion has enabled for transform async arrow functions + fn is_async_only(&self) -> bool { + self.mode == ArrowFunctionConverterMode::AsyncOnly + } + fn get_this_identifier( &mut self, span: Span, ctx: &mut TraverseCtx<'a>, ) -> Option>> { // Find arrow function we are currently in (if we are) - let arrow_scope_id = Self::get_arrow_function_scope(ctx)?; + let arrow_scope_id = self.get_arrow_function_scope(ctx)?; // TODO(improve-on-babel): We create a new UID for every scope. This is pointless, as only one // `this` can be in scope at a time. We could create a single `_this` UID and reuse it in each @@ -304,7 +319,7 @@ impl<'a> ArrowFunctionConverter<'a> { /// Find arrow function we are currently in, if it's between current node, and where `this` is bound. /// Return its `ScopeId`. - fn get_arrow_function_scope(ctx: &mut TraverseCtx<'a>) -> Option { + fn get_arrow_function_scope(&self, ctx: &mut TraverseCtx<'a>) -> Option { // `this` inside a class resolves to `this` *outside* the class in: // * `extends` clause // * Computed method key @@ -346,13 +361,13 @@ impl<'a> ArrowFunctionConverter<'a> { // ``` // // So in this loop, we only exit when we encounter one of the above. - for ancestor in ctx.ancestors() { + let mut ancestors = ctx.ancestors(); + while let Some(ancestor) = ancestors.next() { match ancestor { // Top level Ancestor::ProgramBody(_) // Function (includes class method body) | Ancestor::FunctionParams(_) - | Ancestor::FunctionBody(_) // Class property body | Ancestor::PropertyDefinitionValue(_) // Class accessor property body @@ -361,10 +376,29 @@ impl<'a> ArrowFunctionConverter<'a> { | Ancestor::StaticBlockBody(_) => return None, // Arrow function Ancestor::ArrowFunctionExpressionParams(func) => { - return Some(func.scope_id().get().unwrap()) + return if self.is_async_only() && !*func.r#async() { + None + } else { + Some(func.scope_id().get().unwrap()) + } } Ancestor::ArrowFunctionExpressionBody(func) => { - return Some(func.scope_id().get().unwrap()) + return if self.is_async_only() && !*func.r#async() { + None + } else { + Some(func.scope_id().get().unwrap()) + } + } + Ancestor::FunctionBody(func) => { + return if self.is_async_only() && *func.r#async() + && matches!( + ancestors.next().unwrap(), + Ancestor::MethodDefinitionValue(_) | Ancestor::ObjectPropertyValue(_) + ) { + Some(func.scope_id().get().unwrap()) + } else { + None + } } _ => {} } diff --git a/crates/oxc_transformer/src/common/mod.rs b/crates/oxc_transformer/src/common/mod.rs index cb8d99554..8c6ba957e 100644 --- a/crates/oxc_transformer/src/common/mod.rs +++ b/crates/oxc_transformer/src/common/mod.rs @@ -1,8 +1,6 @@ //! Utility transforms which are in common between other transforms. -use arrow_function_converter::{ - ArrowFunctionConverter, ArrowFunctionConverterMode, ArrowFunctionConverterOptions, -}; +use arrow_function_converter::ArrowFunctionConverter; use oxc_allocator::Vec as ArenaVec; use oxc_ast::ast::*; use oxc_traverse::{Traverse, TraverseCtx}; @@ -31,23 +29,12 @@ pub struct Common<'a, 'ctx> { impl<'a, 'ctx> Common<'a, 'ctx> { pub fn new(options: &TransformOptions, ctx: &'ctx TransformCtx<'a>) -> Self { - let arrow_function_converter_options = { - let mode = if options.env.es2015.arrow_function.is_some() { - ArrowFunctionConverterMode::Enabled - } else { - ArrowFunctionConverterMode::Disabled - }; - ArrowFunctionConverterOptions { mode } - }; - Self { module_imports: ModuleImports::new(ctx), var_declarations: VarDeclarations::new(ctx), statement_injector: StatementInjector::new(ctx), top_level_statements: TopLevelStatements::new(ctx), - arrow_function_converter: ArrowFunctionConverter::new( - &arrow_function_converter_options, - ), + arrow_function_converter: ArrowFunctionConverter::new(options), } } } diff --git a/crates/oxc_transformer/src/es2017/async_to_generator.rs b/crates/oxc_transformer/src/es2017/async_to_generator.rs index 7b52e56f3..81d6159a6 100644 --- a/crates/oxc_transformer/src/es2017/async_to_generator.rs +++ b/crates/oxc_transformer/src/es2017/async_to_generator.rs @@ -131,7 +131,14 @@ impl<'a, 'ctx> Traverse<'a> for AsyncToGenerator<'a, 'ctx> { } fn exit_function(&mut self, func: &mut Function<'a>, ctx: &mut TraverseCtx<'a>) { - if func.r#async && matches!(ctx.parent(), Ancestor::MethodDefinitionValue(_)) { + if func.r#async + && !func.is_typescript_syntax() + && matches!( + ctx.parent(), + // `class A { async foo() {} }` | `({ async foo() {} })` + Ancestor::MethodDefinitionValue(_) | Ancestor::PropertyDefinitionValue(_) + ) + { self.executor.transform_function_for_method_definition(func, ctx); } } diff --git a/crates/oxc_transformer/src/es2018/async_generator_functions/mod.rs b/crates/oxc_transformer/src/es2018/async_generator_functions/mod.rs index 22fdf5de8..d048d9206 100644 --- a/crates/oxc_transformer/src/es2018/async_generator_functions/mod.rs +++ b/crates/oxc_transformer/src/es2018/async_generator_functions/mod.rs @@ -172,7 +172,11 @@ impl<'a, 'ctx> Traverse<'a> for AsyncGeneratorFunctions<'a, 'ctx> { if func.r#async && func.generator && !func.is_typescript_syntax() - && matches!(ctx.parent(), Ancestor::MethodDefinitionValue(_)) + && matches!( + ctx.parent(), + // `class A { async foo() {} }` | `({ async foo() {} })` + Ancestor::MethodDefinitionValue(_) | Ancestor::ObjectPropertyValue(_) + ) { self.executor.transform_function_for_method_definition(func, ctx); } diff --git a/tasks/transform_conformance/snapshots/babel.snap.md b/tasks/transform_conformance/snapshots/babel.snap.md index 22a15bcaf..577ab810d 100644 --- a/tasks/transform_conformance/snapshots/babel.snap.md +++ b/tasks/transform_conformance/snapshots/babel.snap.md @@ -1452,13 +1452,37 @@ x Output mismatch # babel-plugin-transform-async-generator-functions (15/19) * async-generators/class-method/input.js -x Output mismatch +Bindings mismatch: +after transform: ScopeId(0): ["C", "_this"] +rebuilt : ScopeId(0): ["C"] +Bindings mismatch: +after transform: ScopeId(3): [] +rebuilt : ScopeId(2): ["_this"] +Symbol scope ID mismatch for "_this": +after transform: SymbolId(1): ScopeId(0) +rebuilt : SymbolId(1): ScopeId(2) * async-generators/object-method/input.js -x Output mismatch +Bindings mismatch: +after transform: ScopeId(0): ["_this"] +rebuilt : ScopeId(0): [] +Bindings mismatch: +after transform: ScopeId(2): [] +rebuilt : ScopeId(1): ["_this"] +Symbol scope ID mismatch for "_this": +after transform: SymbolId(0): ScopeId(0) +rebuilt : SymbolId(0): ScopeId(1) * async-generators/static-method/input.js -x Output mismatch +Bindings mismatch: +after transform: ScopeId(0): ["C", "_this"] +rebuilt : ScopeId(0): ["C"] +Bindings mismatch: +after transform: ScopeId(3): [] +rebuilt : ScopeId(2): ["_this"] +Symbol scope ID mismatch for "_this": +after transform: SymbolId(1): ScopeId(0) +rebuilt : SymbolId(1): ScopeId(2) * nested/arrows-in-declaration/input.js x Output mismatch diff --git a/tasks/transform_conformance/snapshots/babel_exec.snap.md b/tasks/transform_conformance/snapshots/babel_exec.snap.md index 5f39b203e..865eacd21 100644 --- a/tasks/transform_conformance/snapshots/babel_exec.snap.md +++ b/tasks/transform_conformance/snapshots/babel_exec.snap.md @@ -234,33 +234,3 @@ AssertionError: expected false to be true // Object.is equality ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[14/14]⎯ - -⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯ -AssertionError: expected [Function Object] to be [Function Bar] // Object.is equality - -- Expected -+ Received - -- [Function Bar] -+ [Function Object] - - ❯ fixtures/babel-preset-env-test-fixtures-plugins-integration-regression-7064-exec.test.js:13:30 - 11| }).call(this); - 12| _asyncToGenerator(function* () { - 13| expect(this.constructor).toBe(Bar); - | ^ - 14| })(); - 15| _asyncToGenerator(function* () { - ❯ asyncGeneratorStep ../../node_modules/.pnpm/@babel+runtime@7.26.0/node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:17 - ❯ _next ../../node_modules/.pnpm/@babel+runtime@7.26.0/node_modules/@babel/runtime/helpers/asyncToGenerator.js:17:9 - ❯ ../../node_modules/.pnpm/@babel+runtime@7.26.0/node_modules/@babel/runtime/helpers/asyncToGenerator.js:22:7 - ❯ ../../node_modules/.pnpm/@babel+runtime@7.26.0/node_modules/@babel/runtime/helpers/asyncToGenerator.js:14:12 - ❯ Bar.test fixtures/babel-preset-env-test-fixtures-plugins-integration-regression-7064-exec.test.js:14:6 - ❯ fixtures/babel-preset-env-test-fixtures-plugins-integration-regression-7064-exec.test.js:20:12 - ❯ ../../node_modules/.pnpm/@vitest+runner@2.1.2/node_modules/@vitest/runner/dist/index.js:146:14 - -This error originated in "fixtures/babel-preset-env-test-fixtures-plugins-integration-regression-7064-exec.test.js" test file. It doesn't mean the error was thrown inside the file itself, but while it was running. -The latest test that might've caused the error is "fixtures/babel-preset-env-test-fixtures-plugins-integration-regression-7064-exec.test.js". It might mean one of the following: -- The error was thrown, while Vitest was running this test. -- If the error occurred after the test had been completed, this was the last documented test before it was thrown. -