Skip to content

Commit 6bf8bac

Browse files
committed
feat(formatter): reimplement formatting for ImportExpression (#14712)
The formatting logic of `arguments` in `ImportExpression` and `CallExpression` is the same, but our AST differs between these two nodes. Previously, I tried to pick up some logic from `CallExpression` to `ImportExpression` to cover most cases. However, it turns out that it isn't covered very well. In this PR, I tried to convert `ImportExpression`'s `source` and `options` to `AstNode<Vec<Argument>` with a tricky unsafe transmute so that it matches the Prettier behavior.
1 parent df48416 commit 6bf8bac

File tree

7 files changed

+137
-65
lines changed

7 files changed

+137
-65
lines changed

crates/oxc_formatter/src/ast_nodes/node.rs

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
use core::fmt;
2-
use std::ops::Deref;
2+
use std::{
3+
mem::{transmute, transmute_copy},
4+
ops::Deref,
5+
};
36

4-
use oxc_allocator::Allocator;
5-
use oxc_ast::ast::{ExpressionStatement, Program};
7+
use oxc_allocator::{Allocator, Vec};
8+
use oxc_ast::ast::*;
69
use oxc_span::{GetSpan, Span};
710

811
use super::AstNodes;
@@ -124,3 +127,46 @@ impl<'a> AstNode<'a, ExpressionStatement<'a>> {
124127
matches!(self.parent.parent(), AstNodes::ArrowFunctionExpression(arrow) if arrow.expression)
125128
}
126129
}
130+
131+
impl<'a> AstNode<'a, ImportExpression<'a>> {
132+
/// Converts the arguments of the ImportExpression into an `AstNode` representing a `Vec` of `Argument`.
133+
#[inline]
134+
pub fn to_arguments(&self) -> &AstNode<'a, Vec<'a, Argument<'a>>> {
135+
// Convert ImportExpression's source and options to Vec<'a, Argument<'a>>.
136+
// This allows us to reuse CallExpression's argument formatting logic when printing
137+
// import expressions, since import(source, options) has the same structure as
138+
// a function call with arguments.
139+
let mut arguments = Vec::new_in(self.allocator);
140+
141+
// SAFETY: Argument inherits all Expression variants through the inherit_variants! macro,
142+
// so Expression and Argument have identical memory layout for shared variants.
143+
// Both are discriminated unions where each Expression variant (e.g., Expression::Identifier)
144+
// has a corresponding Argument variant (e.g., Argument::Identifier) with the same discriminant
145+
// and the same inner type (Box<'a, T>). Transmuting Expression to Argument via transmute_copy
146+
// is safe because we're just copying the bits (discriminant + pointer).
147+
unsafe {
148+
arguments.push(transmute_copy(&self.inner.source));
149+
if let Some(ref options) = self.inner.options {
150+
arguments.push(transmute_copy(options));
151+
}
152+
}
153+
154+
let arguments_ref = self.allocator.alloc(arguments);
155+
let following_span = self.following_span;
156+
157+
self.allocator.alloc(AstNode {
158+
inner: arguments_ref,
159+
allocator: self.allocator,
160+
parent: self.allocator.alloc(AstNodes::ImportExpression({
161+
/// * SAFETY: `self` is already allocated in Arena, so transmute from `&` to `&'a` is safe.
162+
unsafe {
163+
transmute::<
164+
&AstNode<'_, ImportExpression<'_>>,
165+
&'a AstNode<'a, ImportExpression<'a>>,
166+
>(self)
167+
}
168+
})),
169+
following_span,
170+
})
171+
}
172+
}

crates/oxc_formatter/src/write/call_arguments.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ pub fn is_function_composition_args(args: &[Argument<'_>]) -> bool {
193193
}
194194

195195
fn format_all_elements_broken_out<'a, 'b>(
196+
node: &'b AstNode<'a, ArenaVec<'a, Argument<'a>>>,
196197
elements: impl Iterator<Item = (FormatResult<Option<FormatElement<'a>>>, usize)>,
197198
expand: bool,
198199
mut buffer: impl Buffer<'a>,
@@ -215,7 +216,11 @@ fn format_all_elements_broken_out<'a, 'b>(
215216
}
216217
}
217218

218-
write!(f, FormatTrailingCommas::All)
219+
write!(
220+
f,
221+
[(!matches!(node.parent, AstNodes::ImportExpression(_)))
222+
.then_some(FormatTrailingCommas::All)]
223+
)
219224
})),
220225
")",
221226
))
@@ -245,7 +250,11 @@ fn format_all_args_broken_out<'a, 'b>(
245250
write!(f, [argument, (index != last_index).then_some(",")])?;
246251
}
247252

248-
write!(f, FormatTrailingCommas::All)
253+
write!(
254+
f,
255+
[(!matches!(node.parent, AstNodes::ImportExpression(_)))
256+
.then_some(FormatTrailingCommas::All)]
257+
)
249258
})),
250259
")",
251260
))
@@ -641,7 +650,7 @@ fn write_grouped_arguments<'a>(
641650
// If any of the not grouped elements break, then fall back to the variant where
642651
// all arguments are printed in expanded mode.
643652
if non_grouped_breaks {
644-
return format_all_elements_broken_out(elements.into_iter(), true, f);
653+
return format_all_elements_broken_out(node, elements.into_iter(), true, f);
645654
}
646655

647656
// We now cache the delimiter tokens. This is needed because `[crate::best_fitting]` will try to
@@ -654,7 +663,7 @@ fn write_grouped_arguments<'a>(
654663
let mut buffer = VecBuffer::new(f.state_mut());
655664
buffer.write_element(FormatElement::Tag(Tag::StartEntry))?;
656665

657-
format_all_elements_broken_out(elements.iter().cloned(), true, &mut buffer);
666+
format_all_elements_broken_out(node, elements.iter().cloned(), true, &mut buffer);
658667

659668
buffer.write_element(FormatElement::Tag(Tag::EndEntry))?;
660669

Lines changed: 3 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use oxc_allocator::Vec;
12
use oxc_ast::ast::*;
23

34
use crate::{
@@ -17,63 +18,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ImportExpression<'a>> {
1718
write!(f, [".", phase.as_str()])?;
1819
}
1920

20-
// The formatting implementation of `source` and `options` picks from `call_arguments`.
21-
if self.options.is_none()
22-
&& (!matches!(
23-
self.source,
24-
Expression::StringLiteral(_)
25-
| Expression::TemplateLiteral(_)
26-
// Theoretically dynamic import shouldn't have this.
27-
| Expression::TaggedTemplateExpression(_)
28-
) || f.comments().has_comment_before(self.span.end))
29-
{
30-
return write!(
31-
f,
32-
[
33-
"(",
34-
group(&soft_block_indent(&format_once(|f| {
35-
write!(f, [self.source()])?;
36-
if let Some(options) = self.options() {
37-
write!(
38-
f,
39-
[
40-
",",
41-
soft_line_break_or_space(),
42-
group(&options).should_expand(true)
43-
]
44-
)?;
45-
}
46-
Ok(())
47-
}))),
48-
")"
49-
]
50-
);
51-
}
52-
53-
let source = self.source().memoized();
54-
let options = self.options().memoized();
55-
56-
best_fitting![
57-
group(&format_once(|f| {
58-
write!(f, ["(", source])?;
59-
if self.options().is_some() {
60-
write!(f, [",", space(), group(&options).should_expand(true)])?;
61-
}
62-
write!(f, ")")
63-
})),
64-
group(&format_args!(
65-
"(",
66-
&soft_block_indent(&format_once(|f| {
67-
write!(f, [source])?;
68-
if self.options.is_some() {
69-
write!(f, [",", soft_line_break_or_space(), options])?;
70-
}
71-
Ok(())
72-
})),
73-
")"
74-
))
75-
.should_expand(true),
76-
]
77-
.fmt(f)
21+
// Use the same logic as CallExpression arguments formatting
22+
write!(f, self.to_arguments())
7823
}
7924
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import(
2+
// comment1
3+
`../alias/${base}.js`
4+
// comment2
5+
);
6+
7+
import(
8+
// comment1
9+
`../alias/${base}.js`,
10+
// comment2
11+
{ with: { type: "json" }}
12+
// comment2
13+
);
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
source: crates/oxc_formatter/tests/fixtures/mod.rs
3+
---
4+
==================== Input ====================
5+
import(
6+
// comment1
7+
`../alias/${base}.js`
8+
// comment2
9+
);
10+
11+
import(
12+
// comment1
13+
`../alias/${base}.js`,
14+
// comment2
15+
{ with: { type: "json" }}
16+
// comment2
17+
);
18+
19+
==================== Output ====================
20+
import(
21+
// comment1
22+
`../alias/${base}.js`
23+
// comment2
24+
);
25+
26+
import(
27+
// comment1
28+
`../alias/${base}.js`,
29+
// comment2
30+
{ with: { type: "json" } }
31+
// comment2
32+
);
33+
34+
===================== End =====================
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
const fixturePackageJson = (
2+
await import(
3+
pathToFileURL(path.join(fixtureDir, 'package.json')).href,
4+
{ with: { type: 'json' } }
5+
)
6+
).default;
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
source: crates/oxc_formatter/tests/fixtures/mod.rs
3+
---
4+
==================== Input ====================
5+
const fixturePackageJson = (
6+
await import(
7+
pathToFileURL(path.join(fixtureDir, 'package.json')).href,
8+
{ with: { type: 'json' } }
9+
)
10+
).default;
11+
12+
==================== Output ====================
13+
const fixturePackageJson = (
14+
await import(pathToFileURL(path.join(fixtureDir, "package.json")).href, {
15+
with: { type: "json" },
16+
})
17+
).default;
18+
19+
===================== End =====================

0 commit comments

Comments
 (0)