refactor(transformer/legacy-decorator): eliminate unreliable identification of metadata (#13227)

Fixes: #13215 (review)

Collect metadata and store them in a `VecDeque`, and take them when transforming the class decorators in the `exit_class` side, which can eliminate the unreliable check of `span.is_unspanned` for metadata decorators. However, it introduces a little complexity.

Author: Dunqing

crates/oxc_transformer/src/decorator/legacy/metadata.rs

@@ -87,8 +87,11 @@
 ///
 /// ## References
 /// * TypeScript's [emitDecoratorMetadata](https://www.typescriptlang.org/tsconfig#emitDecoratorMetadata)
+use std::collections::VecDeque;
+
 use oxc_allocator::{Box as ArenaBox, TakeIn};
 use oxc_ast::ast::*;
+use oxc_data_structures::stack::NonEmptyStack;
 use oxc_semantic::ReferenceFlags;
 use oxc_span::{ContentEq, SPAN};
 use oxc_traverse::{MaybeBoundIdentifier, Traverse};
@@ -100,13 +103,19 @@ use crate::{
     utils::ast_builder::create_property_access,
 };
 
+pub enum MethodMetadata<'a> {
+    Constructor(Expression<'a>),
+    Normal([Expression<'a>; 3]),
+}
+
 pub struct LegacyDecoratorMetadata<'a, 'ctx> {
     ctx: &'ctx TransformCtx<'a>,
+    metadata_stack: NonEmptyStack<VecDeque<MethodMetadata<'a>>>,
 }
 
 impl<'a, 'ctx> LegacyDecoratorMetadata<'a, 'ctx> {
     pub fn new(ctx: &'ctx TransformCtx<'a>) -> Self {
-        LegacyDecoratorMetadata { ctx }
+        LegacyDecoratorMetadata { ctx, metadata_stack: NonEmptyStack::new(VecDeque::new()) }
     }
 }
 
@@ -121,19 +130,18 @@ impl<'a> Traverse<'a, TransformState<'a>> for LegacyDecoratorMetadata<'a, '_> {
             _ => None,
         });
 
-        if let Some(constructor) = constructor {
-            if class.decorators.is_empty()
-                && constructor.value.params.items.iter().all(|param| param.decorators.is_empty())
-            {
-                return;
-            }
-
+        if let Some(constructor) = constructor
+            && !(class.decorators.is_empty()
+                && constructor.value.params.items.iter().all(|param| param.decorators.is_empty()))
+        {
             let serialized_type =
                 self.serialize_parameter_types_of_node(&constructor.value.params, ctx);
-            let metadata_decorator =
-                self.create_metadata_decorate("design:paramtypes", serialized_type, ctx);
 
-            class.decorators.push(metadata_decorator);
+            self.metadata_stack.push(VecDeque::from_iter([MethodMetadata::Constructor(
+                self.create_metadata("design:paramtypes", serialized_type, ctx),
+            )]));
+        } else {
+            self.metadata_stack.push(VecDeque::new());
         }
     }
 
@@ -152,18 +160,19 @@ impl<'a> Traverse<'a, TransformState<'a>> for LegacyDecoratorMetadata<'a, '_> {
             return;
         }
 
-        method.decorators.extend([
-            self.create_metadata_decorate("design:type", Self::global_function(ctx), ctx),
+        let metadata = [
+            self.create_metadata("design:type", Self::global_function(ctx), ctx),
             {
                 let serialized_type =
                     self.serialize_parameter_types_of_node(&method.value.params, ctx);
-                self.create_metadata_decorate("design:paramtypes", serialized_type, ctx)
+                self.create_metadata("design:paramtypes", serialized_type, ctx)
             },
             {
                 let serialized_type = self.serialize_return_type_of_node(&method.value, ctx);
-                self.create_metadata_decorate("design:returntype", serialized_type, ctx)
+                self.create_metadata("design:returntype", serialized_type, ctx)
             },
-        ]);
+        ];
+        self.metadata_stack.last_mut().push_back(MethodMetadata::Normal(metadata));
     }
 
     #[inline]
@@ -192,6 +201,10 @@ impl<'a> Traverse<'a, TransformState<'a>> for LegacyDecoratorMetadata<'a, '_> {
 }
 
 impl<'a> LegacyDecoratorMetadata<'a, '_> {
+    pub fn pop_method_metadata(&mut self) -> Option<MethodMetadata<'a>> {
+        self.metadata_stack.last_mut().pop_front()
+    }
+
     fn serialize_type_annotation(
         &mut self,
         type_annotation: Option<&ArenaBox<'a, TSTypeAnnotation<'a>>>,
@@ -607,18 +620,27 @@ impl<'a> LegacyDecoratorMetadata<'a, '_> {
     }
 
     // `_metadata(key, value)
-    fn create_metadata_decorate(
+    fn create_metadata(
         &self,
         key: &'a str,
         value: Expression<'a>,
         ctx: &mut TraverseCtx<'a>,
-    ) -> Decorator<'a> {
+    ) -> Expression<'a> {
         let arguments = ctx.ast.vec_from_array([
             Argument::from(ctx.ast.expression_string_literal(SPAN, key, None)),
             Argument::from(value),
         ]);
-        let expr = self.ctx.helper_call_expr(Helper::DecorateMetadata, SPAN, arguments, ctx);
-        ctx.ast.decorator(SPAN, expr)
+        self.ctx.helper_call_expr(Helper::DecorateMetadata, SPAN, arguments, ctx)
+    }
+
+    // `_metadata(key, value)
+    fn create_metadata_decorate(
+        &self,
+        key: &'a str,
+        value: Expression<'a>,
+        ctx: &mut TraverseCtx<'a>,
+    ) -> Decorator<'a> {
+        ctx.ast.decorator(SPAN, self.create_metadata(key, value, ctx))
     }
 
     /// `_metadata("design:type", type)`

crates/oxc_transformer/src/decorator/legacy/mod.rs

@@ -62,7 +62,7 @@ use crate::{
     state::TransformState,
     utils::ast_builder::{create_assignment, create_prototype_member},
 };
-use metadata::LegacyDecoratorMetadata;
+use metadata::{LegacyDecoratorMetadata, MethodMetadata};
 
 #[derive(Default)]
 struct ClassDecoratorInfo {
@@ -598,7 +598,7 @@ impl<'a> LegacyDecorator<'a, '_> {
     /// Transform decorators of [`ClassElement::MethodDefinition`],
     /// [`ClassElement::PropertyDefinition`] and [`ClassElement::AccessorProperty`].
     fn transform_decorators_of_class_elements(
-        &self,
+        &mut self,
         class: &mut Class<'a>,
         class_binding: &BoundIdentifier<'a>,
         ctx: &mut TraverseCtx<'a>,
@@ -680,7 +680,7 @@ impl<'a> LegacyDecorator<'a, '_> {
     /// ], Class);
     /// ```
     fn transform_decorators_of_class_and_constructor(
-        &self,
+        &mut self,
         class: &mut Class<'a>,
         class_binding: &BoundIdentifier<'a>,
         class_alias_binding: Option<&BoundIdentifier<'a>>,
@@ -821,7 +821,7 @@ impl<'a> LegacyDecorator<'a, '_> {
     /// ]
     /// ```
     fn get_all_decorators_of_class_method(
-        &self,
+        &mut self,
         method: &mut MethodDefinition<'a>,
         ctx: &mut TraverseCtx<'a>,
     ) -> Option<Expression<'a>> {
@@ -836,24 +836,11 @@ impl<'a> LegacyDecorator<'a, '_> {
 
         let mut decorations = ctx.ast.vec_with_capacity(method_decoration_count);
 
-        // Split metadata decorators from method decorators.
-        // Metadata decorators (typically used for emitting design-time type information)
-        // are identified by having an "unspanned" span. According to TypeScript's legacy
-        // decorator semantics, metadata decorators must be applied *after* all parameter
-        // decorators, so we separate them here and will insert them last.
-        let mut method_decorators = method.decorators.take_in(ctx.ast);
-        let metadata_position = method_decorators
-            .iter()
-            .position(|decorator| {
-                // All metadata decorators are unspanned
-                decorator.span.is_unspanned()
-            })
-            .unwrap_or(method_decorators.len());
-        let metadata_decorators = method_decorators.split_off(metadata_position);
-
         // Method decorators should always be injected before all other decorators
         decorations.extend(
-            method_decorators
+            method
+                .decorators
+                .take_in(ctx.ast)
                 .into_iter()
                 .map(|decorator| ArrayExpressionElement::from(decorator.expression)),
         );
@@ -864,11 +851,16 @@ impl<'a> LegacyDecorator<'a, '_> {
         }
 
         // `decorateMetadata` should always be injected after param decorators
-        decorations.extend(
-            metadata_decorators
-                .into_iter()
-                .map(|decorator| ArrayExpressionElement::from(decorator.expression)),
-        );
+        if let Some(metadata) = self.metadata.pop_method_metadata() {
+            match metadata {
+                MethodMetadata::Constructor(meta) => {
+                    decorations.push(ArrayExpressionElement::from(meta));
+                }
+                MethodMetadata::Normal(meta) => {
+                    decorations.extend(meta.map(ArrayExpressionElement::from));
+                }
+            }
+        }
 
         Some(ctx.ast.expression_array(SPAN, decorations))
     }

tasks/coverage/snapshots/semantic_typescript.snap

@@ -40311,7 +40311,7 @@ after transform: ScopeId(0): ["_C", "_decorateMetadata", "_defineProperty", "_ge
 rebuilt        : ScopeId(0): ["_C", "_decorateMetadata", "_defineProperty", "_get_x", "_method", "_set_x", "_y"]
 Symbol reference IDs mismatch for "_decorateMetadata":
 after transform: SymbolId(10): [ReferenceId(24), ReferenceId(25), ReferenceId(26), ReferenceId(28), ReferenceId(29), ReferenceId(30), ReferenceId(32), ReferenceId(34), ReferenceId(35), ReferenceId(37), ReferenceId(39), ReferenceId(41), ReferenceId(42), ReferenceId(43), ReferenceId(45), ReferenceId(46), ReferenceId(47), ReferenceId(49), ReferenceId(51), ReferenceId(52), ReferenceId(54), ReferenceId(56)]
-rebuilt        : SymbolId(1): [ReferenceId(8), ReferenceId(10), ReferenceId(11), ReferenceId(14), ReferenceId(16), ReferenceId(17), ReferenceId(20), ReferenceId(22), ReferenceId(24), ReferenceId(27), ReferenceId(31)]
+rebuilt        : SymbolId(1): [ReferenceId(14), ReferenceId(18)]
 Reference symbol mismatch for "dec":
 after transform: SymbolId(0) "dec"
 rebuilt        : <None>
@@ -40350,16 +40350,10 @@ after transform: SymbolId(0) "dec"
 rebuilt        : <None>
 Unresolved references mismatch:
 after transform: ["Function", "Number", "Object", "require"]
-rebuilt        : ["Function", "Number", "Object", "dec", "require"]
-Unresolved reference IDs mismatch for "Function":
-after transform: [ReferenceId(23), ReferenceId(27), ReferenceId(31), ReferenceId(40), ReferenceId(44), ReferenceId(48)]
-rebuilt        : [ReferenceId(9), ReferenceId(15), ReferenceId(21)]
+rebuilt        : ["Object", "dec", "require"]
 Unresolved reference IDs mismatch for "Object":
 after transform: [ReferenceId(36), ReferenceId(38), ReferenceId(53), ReferenceId(55)]
-rebuilt        : [ReferenceId(28), ReferenceId(32)]
-Unresolved reference IDs mismatch for "Number":
-after transform: [ReferenceId(33), ReferenceId(50)]
-rebuilt        : [ReferenceId(23)]
+rebuilt        : [ReferenceId(15), ReferenceId(19)]
 
 semantic Error: tasks/coverage/typescript/tests/cases/conformance/esDecorators/classExpression/namedEvaluation/esDecorators-classExpression-namedEvaluation.1.ts
 Bindings mismatch:

tasks/transform_conformance/snapshots/oxc.snap.md

@@ -592,6 +592,12 @@ rebuilt        : ScopeId(0): ["Foo"]
 Scope children mismatch:
 after transform: ScopeId(0): [ScopeId(1), ScopeId(2), ScopeId(3)]
 rebuilt        : ScopeId(0): [ScopeId(1)]
+Symbol span mismatch for "Foo":
+after transform: SymbolId(4): Span { start: 107, end: 110 }
+rebuilt        : SymbolId(0): Span { start: 0, end: 0 }
+Symbol span mismatch for "Foo":
+after transform: SymbolId(11): Span { start: 0, end: 0 }
+rebuilt        : SymbolId(1): Span { start: 107, end: 110 }
 Reference symbol mismatch for "methodDecorator":
 after transform: SymbolId(0) "methodDecorator"
 rebuilt        : <None>
@@ -616,9 +622,12 @@ rebuilt        : <None>
 Reference symbol mismatch for "paramDecorator":
 after transform: SymbolId(2) "paramDecorator"
 rebuilt        : <None>
+Reference symbol mismatch for "paramDecorator":
+after transform: SymbolId(2) "paramDecorator"
+rebuilt        : <None>
 Unresolved references mismatch:
-after transform: ["Boolean", "Function", "String", "babelHelpers"]
-rebuilt        : ["Boolean", "Function", "String", "babelHelpers", "methodDecorator", "paramDecorator"]
+after transform: ["Boolean", "Function", "Number", "String", "babelHelpers"]
+rebuilt        : ["Boolean", "Function", "Number", "String", "babelHelpers", "methodDecorator", "paramDecorator"]
 
 * oxc/metadata/this/input.ts
 Symbol span mismatch for "Example":

tasks/transform_conformance/tests/legacy-decorators/test/fixtures/oxc/metadata/params/input.ts

@@ -14,6 +14,10 @@ export class Foo {
     return !!param
   }
 
+  constructor(@paramDecorator param: number) {
+
+  }
+
   method3(@paramDecorator param: string): boolean {
     return !!param
   }

tasks/transform_conformance/tests/legacy-decorators/test/fixtures/oxc/metadata/params/output.js

@@ -1,10 +1,11 @@
-export class Foo {
+let Foo = class Foo {
 	method1(param) {
 		return !!param;
 	}
 	method2(param) {
 		return !!param;
 	}
+  constructor(param) {}
 	method3(param) {
 		return !!param;
 	}
@@ -64,3 +65,5 @@ babelHelpers.decorate(
 	"method4",
 	null,
 );
+Foo = babelHelpers.decorate([babelHelpers.decorateParam(0, paramDecorator), babelHelpers.decorateMetadata("design:paramtypes", [Number])], Foo);
+export { Foo };