Skip to content

Commit ac69ebe

Browse files
DunqingDunqing
committed
feat(formatter): correctly handle array element (#11915)
1 parent c9ccdbe commit ac69ebe

File tree

8 files changed

+49
-32
lines changed

8 files changed

+49
-32
lines changed

‎crates/oxc_formatter/src/formatter/builders.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2414,10 +2414,10 @@ where
24142414

24152415
/// Adds a new node with the specified formatted content to the output, respecting any new lines
24162416
/// that appear before the node in the input source.
2417-
pub fn entry(&mut self, span: Span, source_text: &str, content: &dyn Format<'ast>) {
2417+
pub fn entry(&mut self, span: Span, content: &dyn Format<'ast>) {
24182418
self.result = self.result.and_then(|()| {
24192419
if self.has_elements {
2420-
if self.has_lines_before(span, source_text) {
2420+
if self.has_lines_before(span) {
24212421
write!(self.fmt, empty_line())?;
24222422
} else {
24232423
self.separator.fmt(self.fmt)?;
@@ -2453,7 +2453,7 @@ where
24532453
}
24542454

24552455
/// Get the number of line breaks between two consecutive SyntaxNodes in the tree
2456-
pub fn has_lines_before(&self, span: Span, source_text: &str) -> bool {
2456+
pub fn has_lines_before(&self, span: Span) -> bool {
24572457
get_lines_before(span.start, self.fmt) > 1
24582458
}
24592459
}
@@ -2469,7 +2469,10 @@ pub fn get_lines_before(start: u32, f: &Formatter) -> usize {
24692469
};
24702470

24712471
f.source_text()[..start as usize]
2472-
.trim_end_matches(is_white_space_single_line)
2472+
// TODO: This is a workaround.
2473+
// Trim `(` because we turned off `preserveParens`, which causes us to not have a parenthesis node,
2474+
// but you will still find `(` or `)` in the source text.
2475+
.trim_end_matches(|c| is_white_space_single_line(c) || c == '(')
24732476
.chars()
24742477
.rev()
24752478
.take_while(|c| is_line_terminator(*c))

‎crates/oxc_formatter/src/write/assignment_pattern_property_list.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl<'a> Format<'a> for AssignmentTargetPropertyList<'a, '_> {
3636
.zip(self.properties.iter());
3737
let mut join = f.join_nodes_with_soft_line();
3838
for (format_entry, node) in entries {
39-
join.entry(node.span(), source_text, &format_entry);
39+
join.entry(node.span(), &format_entry);
4040
}
4141
join.finish()
4242
}

‎crates/oxc_formatter/src/write/binding_property_list.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ impl<'a> Format<'a> for BindingPropertyList<'a, '_> {
3535
.zip(self.properties.iter());
3636
let mut join = f.join_nodes_with_soft_line();
3737
for (format_entry, node) in entries {
38-
join.entry(node.span(), source_text, &format_entry);
38+
join.entry(node.span(), &format_entry);
3939
}
4040
join.finish()
4141
}

‎crates/oxc_formatter/src/write/mod.rs‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, Directive<'a>>> {
7878
let source_text = f.context().source_text();
7979
let mut join = f.join_nodes_with_hardline();
8080
for directive in self {
81-
join.entry(directive.span(), source_text, &directive);
81+
join.entry(directive.span(), &directive);
8282
}
8383
join.finish()?;
8484
// if next_sibling's first leading_trivia has more than one new_line, we should add an extra empty line at the end of
@@ -183,7 +183,7 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, ObjectPropertyKind<'a>>> {
183183
for (element, formatted) in self.iter().zip(
184184
FormatSeparatedIter::new(self.iter(), ",").with_trailing_separator(trailing_separator),
185185
) {
186-
join.entry(element.span(), source_text, &formatted);
186+
join.entry(element.span(), &formatted);
187187
}
188188
join.finish()
189189
}
@@ -371,7 +371,14 @@ impl<'a> FormatWrite<'a> for AstNode<'a, UnaryExpression<'a>> {
371371
if self.operator().is_keyword() {
372372
write!(f, space());
373373
}
374-
write!(f, self.argument())
374+
if f.comments().has_comments_in_span(self.span) {
375+
write!(
376+
f,
377+
[group(&format_args!(text("("), soft_block_indent(self.argument()), text(")")))]
378+
)
379+
} else {
380+
write!(f, self.argument())
381+
}
375382
}
376383
}
377384

@@ -443,7 +450,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ArrayAssignmentTarget<'a>> {
443450

444451
join.entry(
445452
SPAN,
446-
source_text,
447453
&format_with(|f| {
448454
if let Some(element) = element.as_ref() {
449455
write!(f, group(element))?;
@@ -569,7 +575,7 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, Statement<'a>>> {
569575
let source_text = f.context().source_text();
570576
let mut join = f.join_nodes_with_hardline();
571577
for stmt in self {
572-
join.entry(stmt.span(), source_text, stmt);
578+
join.entry(stmt.span(), stmt);
573579
}
574580
join.finish()
575581
}
@@ -873,7 +879,7 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, SwitchCase<'a>>> {
873879
let source_text = f.source_text();
874880
let mut join = f.join_nodes_with_hardline();
875881
for case in self {
876-
join.entry(case.span(), source_text, case);
882+
join.entry(case.span(), case);
877883
}
878884
join.finish()
879885
}
@@ -1178,7 +1184,7 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, ClassElement<'a>>> {
11781184
let mut join = f.join_nodes_with_hardline();
11791185
for (e1, e2) in self.iter().zip(self.iter().skip(1).map(Some).chain(std::iter::once(None)))
11801186
{
1181-
join.entry(e1.span(), source_text, &(e1, e2));
1187+
join.entry(e1.span(), &(e1, e2));
11821188
}
11831189
join.finish()
11841190
}
@@ -1858,7 +1864,7 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, TSEnumMember<'a>>> {
18581864
.with_trailing_separator(trailing_separator)
18591865
.nodes_grouped(),
18601866
) {
1861-
join.entry(element.span(), source_text, &formatted);
1867+
join.entry(element.span(), &formatted);
18621868
}
18631869
join.finish()
18641870
}
@@ -2261,7 +2267,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSInterfaceBody<'a>> {
22612267
let source_text = f.context().source_text();
22622268
let mut joiner = f.join_nodes_with_soft_line();
22632269
for (index, sig) in self.body().iter().enumerate() {
2264-
joiner.entry(sig.span(), source_text, sig);
2270+
joiner.entry(sig.span(), sig);
22652271
}
22662272
joiner.finish()
22672273
}
@@ -2293,7 +2299,7 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, TSSignature<'a>>> {
22932299
let source_text = f.source_text();
22942300
let mut join = f.join_nodes_with_soft_line();
22952301
for element in self {
2296-
join.entry(element.span(), source_text, element);
2302+
join.entry(element.span(), element);
22972303
}
22982304
join.finish()
22992305
}

‎crates/oxc_formatter/src/write/parameter_list.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ impl<'a> Format<'a> for ParameterList<'a, '_> {
134134
.with_trailing_separator(trailing_separator)
135135
.zip(FormalParametersIter::from(self.list));
136136
for (formatted, param) in entries {
137-
joiner.entry(param.span(), source_text, &formatted);
137+
joiner.entry(param.span(), &formatted);
138138
}
139139
joiner.finish()
140140
}

‎crates/oxc_formatter/src/write/utils/array.rs‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use oxc_allocator::Vec;
22
use oxc_ast::ast::*;
3-
use oxc_span::SPAN;
3+
use oxc_span::{GetSpan, SPAN};
44

55
use crate::{
66
formatter::{FormatResult, Formatter, prelude::*},
@@ -24,6 +24,7 @@ pub fn write_array_node<'a>(
2424
let mut join = f.join_nodes_with_soft_line();
2525
let last_index = node.len().saturating_sub(1);
2626

27+
let mut has_seen_elision = false;
2728
for (index, element) in node.iter().enumerate() {
2829
let separator_mode = match element.as_ref() {
2930
ArrayExpressionElement::Elision(_) => TrailingSeparatorMode::Force,
@@ -32,11 +33,21 @@ pub fn write_array_node<'a>(
3233

3334
let is_disallow = matches!(separator_mode, TrailingSeparatorMode::Disallow);
3435
let is_force = matches!(separator_mode, TrailingSeparatorMode::Force);
35-
3636
join.entry(
37-
SPAN,
38-
source_text,
39-
&format_with(|f| {
37+
// Note(different-with-Biome): this implementation isn't the same as Biome, because its output doesn't exactly match Prettier.
38+
if has_seen_elision {
39+
// Use fake span to avoid add any empty line between elision and expression element.
40+
SPAN
41+
} else {
42+
has_seen_elision = false;
43+
element.span()
44+
},
45+
&format_once(|f| {
46+
if element.is_elision() {
47+
has_seen_elision = true;
48+
return write!(f, ",");
49+
}
50+
4051
write!(f, group(&element))?;
4152

4253
if is_disallow {

‎tasks/prettier_conformance/snapshots/prettier.js.snap.md‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
js compatibility: 323/699 (46.21%)
1+
js compatibility: 326/699 (46.64%)
22

33
# Failed
44

55
| Spec path | Failed or Passed | Match ratio |
66
| :-------- | :--------------: | :---------: |
77
| js/arrays/empty.js | 💥 | 0.00% |
8-
| js/arrays/numbers-negative-comment-after-minus.js | 💥 | 94.03% |
9-
| js/arrays/numbers-with-holes.js | 💥 | 90.91% |
10-
| js/arrays/preserve_empty_lines.js | 💥 | 86.13% |
8+
| js/arrays/preserve_empty_lines.js | 💥 | 95.95% |
119
| js/arrow-call/arrow_call.js | 💥💥💥 | 62.61% |
1210
| js/arrows/arrow-chain-with-trailing-comments.js | 💥💥 | 76.67% |
1311
| js/arrows/arrow_function_expression.js | 💥💥 | 33.33% |
@@ -54,7 +52,7 @@ js compatibility: 323/699 (46.21%)
5452
| js/async/inline-await.js | 💥 | 22.22% |
5553
| js/async/nested.js | 💥 | 16.67% |
5654
| js/binary-expressions/arrow.js | 💥 | 66.67% |
57-
| js/binary-expressions/comment.js | 💥 | 76.92% |
55+
| js/binary-expressions/comment.js | 💥 | 67.16% |
5856
| js/binary-expressions/in_instanceof.js | 💥 | 97.96% |
5957
| js/binary-expressions/inline-jsx.js | 💥 | 28.57% |
6058
| js/binary-expressions/inline-object-array.js | 💥 | 67.78% |
@@ -91,7 +89,7 @@ js compatibility: 323/699 (46.21%)
9189
| js/classes/property.js | 💥 | 66.67% |
9290
| js/classes/ternary.js | 💥 | 0.00% |
9391
| js/classes-private-fields/with_comments.js | 💥💥 | 61.54% |
94-
| js/comments/15661.js | 💥💥 | 68.35% |
92+
| js/comments/15661.js | 💥💥 | 46.15% |
9593
| js/comments/16398.js | 💥💥 | 80.00% |
9694
| js/comments/binary-expressions-block-comments.js | 💥💥 | 38.64% |
9795
| js/comments/binary-expressions-single-comments.js | 💥💥 | 25.64% |
@@ -283,11 +281,10 @@ js compatibility: 323/699 (46.21%)
283281
| js/return/comment.js | 💥 | 63.89% |
284282
| js/return-outside-function/return-outside-function.js | 💥 | 0.00% |
285283
| js/sequence-break/break.js | 💥 | 31.37% |
286-
| js/sequence-expression/ignore.js | 💥 | 0.00% |
284+
| js/sequence-expression/ignore.js | 💥 | 42.86% |
287285
| js/spread/spread.js | 💥 | 80.00% |
288286
| js/strings/escaped.js | 💥💥 | 50.00% |
289287
| js/strings/multiline-literal.js | 💥💥 | 70.00% |
290-
| js/strings/strings.js | 💥💥 | 83.72% |
291288
| js/strings/template-literals.js | 💥💥 | 52.24% |
292289
| js/switch/comments.js | 💥 | 90.37% |
293290
| js/switch/comments2.js | 💥 | 84.21% |
@@ -327,7 +324,7 @@ js compatibility: 323/699 (46.21%)
327324
| js/trailing-comma/trailing_whitespace.js | 💥💥💥 | 88.64% |
328325
| js/try/catch.js | 💥 | 52.63% |
329326
| js/try/try.js | 💥 | 50.00% |
330-
| js/unary-expression/comments.js | 💥 | 17.83% |
327+
| js/unary-expression/comments.js | 💥 | 74.44% |
331328
| js/unicode/nbsp-jsx.js | 💥 | 25.00% |
332329
| js/variable_declarator/multiple.js | 💥 | 87.27% |
333330
| js/variable_declarator/string.js | 💥 | 0.00% |

‎tasks/prettier_conformance/snapshots/prettier.ts.snap.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ ts compatibility: 200/573 (34.90%)
5959
| typescript/argument-expansion/argument_expansion.ts | 💥 | 93.22% |
6060
| typescript/argument-expansion/arrow-with-return-type.ts | 💥 | 92.31% |
6161
| typescript/array/comment.ts | 💥 | 87.50% |
62-
| typescript/arrow/16067.ts | 💥💥 | 31.33% |
62+
| typescript/arrow/16067.ts | 💥💥 | 40.00% |
6363
| typescript/arrow/comments.ts | 💥✨ | 44.44% |
6464
| typescript/arrow/issue-6107-curry.ts | 💥💥 | 12.50% |
6565
| typescript/as/array-pattern.ts | 💥 | 0.00% |

0 commit comments

Comments
 (0)