fix(ast)!: ImportExpression only allows one option argument (#10432)

Boshen · web-flow · commit c538efa1e219 · 2025-04-16T13:13:28.000+08:00
Co-authored-by: Boshen &lt;1430279+Boshen@users.noreply.github.com&gt;
diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs
@@ -2351,8 +2351,7 @@ pub struct AccessorProperty<'a> {
 pub struct ImportExpression<'a> {
     pub span: Span,
     pub source: Expression<'a>,
-    #[estree(via = ImportExpressionOptions)]
-    pub options: Vec<'a, Expression<'a>>,
+    pub options: Option<Expression<'a>>,
     #[estree(skip)]
     pub phase: Option<ImportPhase>,
 }
diff --git a/crates/oxc_ast/src/generated/assert_layouts.rs b/crates/oxc_ast/src/generated/assert_layouts.rs
@@ -658,12 +658,12 @@ const _: () = {
     assert!(offset_of!(AccessorProperty, type_annotation) == 88);
     assert!(offset_of!(AccessorProperty, accessibility) == 96);
 
-    assert!(size_of::<ImportExpression>() == 64);
+    assert!(size_of::<ImportExpression>() == 48);
     assert!(align_of::<ImportExpression>() == 8);
     assert!(offset_of!(ImportExpression, span) == 0);
     assert!(offset_of!(ImportExpression, source) == 8);
     assert!(offset_of!(ImportExpression, options) == 24);
-    assert!(offset_of!(ImportExpression, phase) == 56);
+    assert!(offset_of!(ImportExpression, phase) == 40);
 
     assert!(size_of::<ImportDeclaration>() == 112);
     assert!(align_of::<ImportDeclaration>() == 8);
@@ -2051,12 +2051,12 @@ const _: () = {
     assert!(offset_of!(AccessorProperty, type_annotation) == 48);
     assert!(offset_of!(AccessorProperty, accessibility) == 52);
 
-    assert!(size_of::<ImportExpression>() == 36);
+    assert!(size_of::<ImportExpression>() == 28);
     assert!(align_of::<ImportExpression>() == 4);
     assert!(offset_of!(ImportExpression, span) == 0);
     assert!(offset_of!(ImportExpression, source) == 8);
     assert!(offset_of!(ImportExpression, options) == 16);
-    assert!(offset_of!(ImportExpression, phase) == 32);
+    assert!(offset_of!(ImportExpression, phase) == 24);
 
     assert!(size_of::<ImportDeclaration>() == 64);
     assert!(align_of::<ImportDeclaration>() == 4);
diff --git a/crates/oxc_ast/src/generated/ast_builder.rs b/crates/oxc_ast/src/generated/ast_builder.rs
@@ -800,7 +800,7 @@ impl<'a> AstBuilder<'a> {
         self,
         span: Span,
         source: Expression<'a>,
-        options: Vec<'a, Expression<'a>>,
+        options: Option<Expression<'a>>,
         phase: Option<ImportPhase>,
     ) -> Expression<'a> {
         Expression::ImportExpression(self.alloc_import_expression(span, source, options, phase))
@@ -7334,7 +7334,7 @@ impl<'a> AstBuilder<'a> {
         self,
         span: Span,
         source: Expression<'a>,
-        options: Vec<'a, Expression<'a>>,
+        options: Option<Expression<'a>>,
         phase: Option<ImportPhase>,
     ) -> ImportExpression<'a> {
         ImportExpression { span, source, options, phase }
@@ -7355,7 +7355,7 @@ impl<'a> AstBuilder<'a> {
         self,
         span: Span,
         source: Expression<'a>,
-        options: Vec<'a, Expression<'a>>,
+        options: Option<Expression<'a>>,
         phase: Option<ImportPhase>,
     ) -> Box<'a, ImportExpression<'a>> {
         Box::new_in(self.import_expression(span, source, options, phase), self.allocator)
diff --git a/crates/oxc_ast/src/generated/derive_estree.rs b/crates/oxc_ast/src/generated/derive_estree.rs
@@ -1651,7 +1651,7 @@ impl ESTree for ImportExpression<'_> {
         state.serialize_field("start", &self.span.start);
         state.serialize_field("end", &self.span.end);
         state.serialize_field("source", &self.source);
-        state.serialize_field("options", &crate::serialize::ImportExpressionOptions(self));
+        state.serialize_field("options", &self.options);
         state.end();
     }
 }
diff --git a/crates/oxc_ast/src/serialize.rs b/crates/oxc_ast/src/serialize.rs
@@ -730,29 +730,6 @@ impl ESTree for AssignmentTargetPropertyIdentifierValue<'_> {
     }
 }
 
-/// Serializer for `options` field of `ImportExpression`.
-///
-/// Serialize only the first expression in `options`, or `null` if `options` is empty.
-#[ast_meta]
-#[estree(
-    ts_type = "Expression | null",
-    raw_deser = "
-        const options = DESER[Vec<Expression>](POS_OFFSET.options);
-        options.length === 0 ? null : options[0]
-    "
-)]
-pub struct ImportExpressionOptions<'a>(pub &'a ImportExpression<'a>);
-
-impl ESTree for ImportExpressionOptions<'_> {
-    fn serialize<S: Serializer>(&self, serializer: S) {
-        if let Some(expression) = self.0.options.first() {
-            expression.serialize(serializer);
-        } else {
-            ().serialize(serializer);
-        }
-    }
-}
-
 // Serializers for `with_clause` field of `ImportDeclaration`, `ExportNamedDeclaration`,
 // and `ExportAllDeclaration` (which are renamed to `attributes` in ESTree AST).
 //
diff --git a/crates/oxc_ast_visit/src/generated/visit.rs b/crates/oxc_ast_visit/src/generated/visit.rs
@@ -2666,7 +2666,9 @@ pub mod walk {
         visitor.enter_node(kind);
         visitor.visit_span(&it.span);
         visitor.visit_expression(&it.source);
-        visitor.visit_expressions(&it.options);
+        if let Some(options) = &it.options {
+            visitor.visit_expression(options);
+        }
         visitor.leave_node(kind);
     }
 
diff --git a/crates/oxc_ast_visit/src/generated/visit_mut.rs b/crates/oxc_ast_visit/src/generated/visit_mut.rs
@@ -2767,7 +2767,9 @@ pub mod walk_mut {
         visitor.enter_node(kind);
         visitor.visit_span(&mut it.span);
         visitor.visit_expression(&mut it.source);
-        visitor.visit_expressions(&mut it.options);
+        if let Some(options) = &mut it.options {
+            visitor.visit_expression(options);
+        }
         visitor.leave_node(kind);
     }
 
diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs
@@ -2087,8 +2087,8 @@ impl GenExpr for ImportExpression<'_> {
                 || p.has_comment(self.source.span().start)
                 || self
                     .options
-                    .first()
-                    .is_some_and(|argument| p.has_comment(argument.span().start)));
+                    .as_ref()
+                    .is_some_and(|options| p.has_comment(options.span().start)));
 
         p.wrap(wrap, |p| {
             p.print_space_before_identifier();
@@ -2108,15 +2108,15 @@ impl GenExpr for ImportExpression<'_> {
                 p.print_indent();
             }
             self.source.print_expr(p, Precedence::Comma, Context::empty());
-            if !self.options.is_empty() {
+            if let Some(options) = &self.options {
                 p.print_comma();
                 if has_comment {
                     p.print_soft_newline();
                     p.print_indent();
                 } else {
                     p.print_soft_space();
                 }
-                p.print_expressions(&self.options, Precedence::Comma, Context::empty());
+                options.gen_expr(p, Precedence::Comma, Context::empty());
             }
             if has_comment {
                 // Handle `/* comment */);`
diff --git a/crates/oxc_formatter/src/write/mod.rs b/crates/oxc_formatter/src/write/mod.rs
@@ -1068,8 +1068,8 @@ impl<'a> FormatWrite<'a> for AccessorProperty<'a> {
 impl<'a> FormatWrite<'a> for ImportExpression<'a> {
     fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
         write!(f, ["import(", self.source])?;
-        for option in &self.options {
-            write!(f, [",", space(), option])?;
+        if let Some(options) = &self.options {
+            write!(f, [",", space(), options])?;
         }
         write!(f, ")")
     }
diff --git a/crates/oxc_parser/src/diagnostics.rs b/crates/oxc_parser/src/diagnostics.rs
@@ -514,6 +514,11 @@ pub fn private_in_private(span: Span) -> OxcDiagnostic {
     OxcDiagnostic::error("Unexpected right-hand side of private-in expression").with_label(span)
 }
 
+#[cold]
+pub fn import_arguments(span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::error("Dynamic imports can only accept a module specifier and an optional set of attributes as arguments").with_label(span)
+}
+
 // ================================= MODIFIERS =================================
 
 #[cold]
diff --git a/crates/oxc_parser/src/js/module.rs b/crates/oxc_parser/src/js/module.rs
@@ -16,24 +16,24 @@ impl<'a> ParserImpl<'a> {
         phase: Option<ImportPhase>,
     ) -> Result<Expression<'a>> {
         self.expect(Kind::LParen)?;
-
         if self.eat(Kind::RParen) {
             return Err(oxc_diagnostics::OxcDiagnostic::error("import() requires a specifier.")
                 .with_label(self.end_span(span)));
         }
-
         let has_in = self.ctx.has_in();
         self.ctx = self.ctx.and_in(true);
-
         let expression = self.parse_assignment_expression_or_higher()?;
-        let mut arguments = self.ast.vec();
-        if self.eat(Kind::Comma) && !self.at(Kind::RParen) {
-            arguments.push(self.parse_assignment_expression_or_higher()?);
+        let arguments = if self.eat(Kind::Comma) && !self.at(Kind::RParen) {
+            Some(self.parse_assignment_expression_or_higher()?)
+        } else {
+            None
+        };
+        // Allow trailing comma
+        self.bump(Kind::Comma);
+        if !self.eat(Kind::RParen) {
+            return Err(diagnostics::import_arguments(self.end_span(span)));
         }
-
         self.ctx = self.ctx.and_in(has_in);
-        self.bump(Kind::Comma);
-        self.expect(Kind::RParen)?;
         let expr =
             self.ast.alloc_import_expression(self.end_span(span), expression, arguments, phase);
         self.module_record_builder.visit_import_expression(&expr);
diff --git a/crates/oxc_traverse/src/generated/ancestor.rs b/crates/oxc_traverse/src/generated/ancestor.rs
@@ -9352,10 +9352,10 @@ impl<'a, 't> ImportExpressionWithoutSource<'a, 't> {
     }
 
     #[inline]
-    pub fn options(self) -> &'t Vec<'a, Expression<'a>> {
+    pub fn options(self) -> &'t Option<Expression<'a>> {
         unsafe {
             &*((self.0 as *const u8).add(OFFSET_IMPORT_EXPRESSION_OPTIONS)
-                as *const Vec<'a, Expression<'a>>)
+                as *const Option<Expression<'a>>)
         }
     }
 
diff --git a/crates/oxc_traverse/src/generated/scopes_collector.rs b/crates/oxc_traverse/src/generated/scopes_collector.rs
@@ -1058,7 +1058,9 @@ impl<'a> Visit<'a> for ChildScopeCollector {
     #[inline]
     fn visit_import_expression(&mut self, it: &ImportExpression<'a>) {
         self.visit_expression(&it.source);
-        self.visit_expressions(&it.options);
+        if let Some(options) = &it.options {
+            self.visit_expression(options);
+        }
     }
 
     #[inline(always)]
diff --git a/crates/oxc_traverse/src/generated/walk.rs b/crates/oxc_traverse/src/generated/walk.rs
@@ -2757,11 +2757,11 @@ unsafe fn walk_import_expression<'a, Tr: Traverse<'a>>(
         (node as *mut u8).add(ancestor::OFFSET_IMPORT_EXPRESSION_SOURCE) as *mut Expression,
         ctx,
     );
-    ctx.retag_stack(AncestorType::ImportExpressionOptions);
-    for item in &mut *((node as *mut u8).add(ancestor::OFFSET_IMPORT_EXPRESSION_OPTIONS)
-        as *mut Vec<Expression>)
+    if let Some(field) = &mut *((node as *mut u8).add(ancestor::OFFSET_IMPORT_EXPRESSION_OPTIONS)
+        as *mut Option<Expression>)
     {
-        walk_expression(traverser, item as *mut _, ctx);
+        ctx.retag_stack(AncestorType::ImportExpressionOptions);
+        walk_expression(traverser, field as *mut _, ctx);
     }
     ctx.pop_stack(pop_token);
     traverser.exit_import_expression(&mut *node, ctx);
diff --git a/napi/parser/deserialize-js.js b/napi/parser/deserialize-js.js
@@ -903,13 +903,12 @@ function deserializeAccessorProperty(pos) {
 }
 
 function deserializeImportExpression(pos) {
-  const options = deserializeVecExpression(pos + 24);
   return {
     type: 'ImportExpression',
     start: deserializeU32(pos),
     end: deserializeU32(pos + 4),
     source: deserializeExpression(pos + 8),
-    options: options.length === 0 ? null : options[0],
+    options: deserializeOptionExpression(pos + 24),
   };
 }
 
diff --git a/napi/parser/deserialize-ts.js b/napi/parser/deserialize-ts.js
@@ -985,13 +985,12 @@ function deserializeAccessorProperty(pos) {
 }
 
 function deserializeImportExpression(pos) {
-  const options = deserializeVecExpression(pos + 24);
   return {
     type: 'ImportExpression',
     start: deserializeU32(pos),
     end: deserializeU32(pos + 4),
     source: deserializeExpression(pos + 8),
-    options: options.length === 0 ? null : options[0],
+    options: deserializeOptionExpression(pos + 24),
   };
 }
 
diff --git a/tasks/coverage/snapshots/parser_babel.snap b/tasks/coverage/snapshots/parser_babel.snap
@@ -5908,11 +5908,10 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
    ·     ─────────────
    ╰────
 
-  × Expected `)` but found `string`
-   ╭─[babel/packages/babel-parser/test/fixtures/es2020/dynamic-import/multiple-args/input.js:1:26]
+  × Dynamic imports can only accept a module specifier and an optional set of attributes as arguments
+   ╭─[babel/packages/babel-parser/test/fixtures/es2020/dynamic-import/multiple-args/input.js:1:1]
  1 │ import('hello', 'world', '!');
-   ·                          ─┬─
-   ·                           ╰── `)` expected
+   · ────────────────────────
    ╰────
 
   × import() requires a specifier.
@@ -5947,11 +5946,10 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
    ·     ─────────────
    ╰────
 
-  × Expected `)` but found `string`
-   ╭─[babel/packages/babel-parser/test/fixtures/es2020/dynamic-import-createImportExpression-false/multiple-args/input.js:1:26]
+  × Dynamic imports can only accept a module specifier and an optional set of attributes as arguments
+   ╭─[babel/packages/babel-parser/test/fixtures/es2020/dynamic-import-createImportExpression-false/multiple-args/input.js:1:1]
  1 │ import('hello', 'world', '!');
-   ·                          ─┬─
-   ·                           ╰── `)` expected
+   · ────────────────────────
    ╰────
 
   × import() requires a specifier.
diff --git a/tasks/coverage/snapshots/parser_test262.snap b/tasks/coverage/snapshots/parser_test262.snap

Original file line number	Diff line number	Diff line change
`@@ -1651,7 +1651,7 @@ impl ESTree for ImportExpression<'_> {`
`1651`	`1651`	`state.serialize_field("start", &self.span.start);`
`1652`	`1652`	`state.serialize_field("end", &self.span.end);`
`1653`	`1653`	`state.serialize_field("source", &self.source);`
`1654`		`- state.serialize_field("options", &crate::serialize::ImportExpressionOptions(self));`
	`1654`	`+ state.serialize_field("options", &self.options);`
`1655`	`1655`	`state.end();`
`1656`	`1656`	`}`
`1657`	`1657`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1068,8 +1068,8 @@ impl<'a> FormatWrite<'a> for AccessorProperty<'a> {`
`1068`	`1068`	`impl<'a> FormatWrite<'a> for ImportExpression<'a> {`
`1069`	`1069`	`fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {`
`1070`	`1070`	`write!(f, ["import(", self.source])?;`
`1071`		`- for option in &self.options {`
`1072`		`- write!(f, [",", space(), option])?;`
	`1071`	`+ if let Some(options) = &self.options {`
	`1072`	`+ write!(f, [",", space(), options])?;`
`1073`	`1073`	`}`
`1074`	`1074`	`write!(f, ")")`
`1075`	`1075`	`}`