Skip to content

Commit 4a663c3

Browse files
committed
feat(formatter): complete printing ForStatement, ForInStatement and ForOfStatement
1 parent 8a7a9f3 commit 4a663c3

File tree

11 files changed

+314
-95
lines changed

11 files changed

+314
-95
lines changed

crates/oxc_formatter/src/formatter/comments/mod.rs

Lines changed: 102 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,25 @@ impl<'a> Comments<'a> {
6060
&comments[..index]
6161
}
6262

63+
/// Returns own line comments that are before the given `pos`.
64+
pub fn own_line_comments_before(&self, pos: u32) -> &'a [Comment] {
65+
let mut index = 0;
66+
67+
let comments = self.unprinted_comments();
68+
for comment in comments {
69+
if !is_own_line_comment(comment, self.source_text) {
70+
break;
71+
}
72+
73+
if comment.span.end > pos {
74+
break;
75+
}
76+
index += 1;
77+
}
78+
79+
&comments[..index]
80+
}
81+
6382
/// Returns the comments that after the given `start` position, even if they were already printed.
6483
pub fn comments_after(&self, pos: u32) -> &'a [Comment] {
6584
let mut index = self.printed_count;
@@ -279,15 +298,39 @@ impl<'a> Comments<'a> {
279298
&self,
280299
enclosing_node: &SiblingNode<'a>,
281300
preceding_node: &SiblingNode<'a>,
282-
following_node: Option<&SiblingNode<'a>>,
301+
mut following_node: Option<&SiblingNode<'a>>,
283302
) -> &'a [Comment] {
303+
if !matches!(
304+
enclosing_node,
305+
SiblingNode::Program(_)
306+
| SiblingNode::BlockStatement(_)
307+
| SiblingNode::FunctionBody(_)
308+
| SiblingNode::TSModuleBlock(_)
309+
| SiblingNode::SwitchStatement(_)
310+
| SiblingNode::StaticBlock(_)
311+
) && matches!(following_node, Some(SiblingNode::EmptyStatement(_)))
312+
{
313+
let enclosing_span = enclosing_node.span();
314+
return self.comments_before(enclosing_span.end);
315+
}
316+
284317
// The preceding_node is the callee of the call expression or new expression, let following node to print it.
285318
// Based on https://github.com/prettier/prettier/blob/7584432401a47a26943dd7a9ca9a8e032ead7285/src/language-js/comments/handle-comments.js#L726-L741
286319
if matches!(enclosing_node, SiblingNode::CallExpression(CallExpression { callee, ..}) | SiblingNode::NewExpression(NewExpression { callee, ..}) if callee.span().contains_inclusive(preceding_node.span()))
287320
{
288321
return &[];
289322
}
290323

324+
// No need to print trailing comments for the most right side for `BinaryExpression` and `LogicalExpression`,
325+
// instead, print trailing comments for expression itself.
326+
if matches!(
327+
enclosing_node,
328+
SiblingNode::BinaryExpression(_) | SiblingNode::LogicalExpression(_)
329+
) && matches!(following_node, Some(SiblingNode::ExpressionStatement(_)))
330+
{
331+
return &[];
332+
}
333+
291334
let comments = self.unprinted_comments();
292335
if comments.is_empty() {
293336
return &[];
@@ -303,15 +346,11 @@ impl<'a> Comments<'a> {
303346

304347
let Some(following_node) = following_node else {
305348
let enclosing_span = enclosing_node.span();
306-
return comments
307-
.iter()
308-
.enumerate()
309-
.take_while(|(_, comment)| comment.span.end <= enclosing_span.end)
310-
.last()
311-
.map_or(&[], |(index, _)| &comments[..=index]);
349+
return self.comments_before(enclosing_span.end);
312350
};
313351

314352
let following_span = following_node.span();
353+
315354
let mut comment_index = 0;
316355
while let Some(comment) = comments.get(comment_index) {
317356
// Check if the comment is before the following node's span
@@ -322,21 +361,42 @@ impl<'a> Comments<'a> {
322361
if is_own_line_comment(comment, source_text) {
323362
// TODO: describe the logic here
324363
// Reached an own line comment, which means it is the leading comment for the next node.
364+
365+
if matches!(enclosing_node, SiblingNode::IfStatement(stmt) if stmt.test.span() == preceding_span)
366+
|| matches!(enclosing_node, SiblingNode::WhileStatement(stmt) if stmt.test.span() == preceding_span)
367+
{
368+
return handle_if_and_while_statement_comments(
369+
following_span.start,
370+
comment_index,
371+
comments,
372+
source_text,
373+
);
374+
}
375+
325376
break;
326377
} else if is_end_of_line_comment(comment, source_text) {
327-
let first_comment_start = comments[0].span.start;
328-
// TODO: Maybe we can remove this check once we complete the logic of handling comments.
329-
if preceding_span.end < first_comment_start {
330-
let is_break = source_text.as_bytes()
331-
[(preceding_span.end as usize)..(first_comment_start as usize)]
332-
.iter()
333-
.any(|&b| matches!(b, b'\n' | b'\r' | b')'));
334-
335-
if is_break {
336-
return &[];
378+
if let SiblingNode::IfStatement(if_stmt) = enclosing_node {
379+
if if_stmt.consequent.span() == preceding_span {
380+
// If comment is after the `else` keyword, it is not a trailing comment of consequent.
381+
if source_text[preceding_span.end as usize..comment.span.start as usize]
382+
.contains("else")
383+
{
384+
return &[];
385+
}
337386
}
338387
}
339388

389+
if matches!(enclosing_node, SiblingNode::IfStatement(stmt) if stmt.test.span() == preceding_span)
390+
|| matches!(enclosing_node, SiblingNode::WhileStatement(stmt) if stmt.test.span() == preceding_span)
391+
{
392+
return handle_if_and_while_statement_comments(
393+
following_span.start,
394+
comment_index,
395+
comments,
396+
source_text,
397+
);
398+
}
399+
340400
// Should be a leading comment of following node.
341401
// Based on https://github.com/prettier/prettier/blob/7584432401a47a26943dd7a9ca9a8e032ead7285/src/language-js/comments/handle-comments.js#L852-L883
342402
if matches!(
@@ -383,6 +443,31 @@ impl<'a> Comments<'a> {
383443
}
384444
}
385445

446+
fn handle_if_and_while_statement_comments<'a>(
447+
mut end: u32,
448+
mut comment_index: usize,
449+
comments: &'a [Comment],
450+
source_text: &'a str,
451+
) -> &'a [Comment] {
452+
// `if (a /* comment before paren */) // comment after paren`
453+
loop {
454+
let cur_comment_span = comments[comment_index].span;
455+
if source_text.as_bytes()[cur_comment_span.end as usize..end as usize].contains(&b')') {
456+
return &comments[..=comment_index];
457+
}
458+
459+
end = cur_comment_span.start;
460+
461+
if comment_index == 0 {
462+
return &[];
463+
}
464+
465+
comment_index -= 1;
466+
}
467+
468+
unreachable!()
469+
}
470+
386471
#[inline]
387472
pub fn is_new_line(char: char) -> ControlFlow<bool> {
388473
if char == ' ' || char == '\t' {

crates/oxc_formatter/src/formatter/trivia.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -208,14 +208,13 @@ impl<'a> Format<'a> for FormatTrailingComments<'a, '_> {
208208

209209
match self {
210210
Self::Node((enclosing_node, preceding_node, following_node)) => {
211-
format_trailing_comments_impl(
212-
f.context().comments().get_trailing_comments(
213-
enclosing_node,
214-
preceding_node,
215-
*following_node,
216-
),
217-
f,
218-
)
211+
let comments = f.context().comments().get_trailing_comments(
212+
enclosing_node,
213+
preceding_node,
214+
*following_node,
215+
);
216+
217+
format_trailing_comments_impl(comments, f)
219218
}
220219
Self::Comments(comments) => format_trailing_comments_impl(*comments, f),
221220
}

crates/oxc_formatter/src/generated/ast_nodes.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5842,7 +5842,7 @@ impl<'a> GetSpan for AstNode<'a, Hashbang<'a>> {
58425842
impl<'a> AstNode<'a, BlockStatement<'a>> {
58435843
#[inline]
58445844
pub fn body(&self) -> &AstNode<'a, Vec<'a, Statement<'a>>> {
5845-
let following_node = self.following_node;
5845+
let following_node = None;
58465846
self.allocator.alloc(AstNode {
58475847
inner: &self.inner.body,
58485848
allocator: self.allocator,
@@ -6178,7 +6178,7 @@ impl<'a> AstNode<'a, DoWhileStatement<'a>> {
61786178

61796179
#[inline]
61806180
pub fn test(&self) -> &AstNode<'a, Expression<'a>> {
6181-
let following_node = self.following_node;
6181+
let following_node = None;
61826182
self.allocator.alloc(AstNode {
61836183
inner: &self.inner.test,
61846184
allocator: self.allocator,
@@ -6222,7 +6222,7 @@ impl<'a> AstNode<'a, WhileStatement<'a>> {
62226222

62236223
#[inline]
62246224
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
6225-
let following_node = self.following_node;
6225+
let following_node = None;
62266226
self.allocator.alloc(AstNode {
62276227
inner: &self.inner.body,
62286228
allocator: self.allocator,
@@ -6305,7 +6305,7 @@ impl<'a> AstNode<'a, ForStatement<'a>> {
63056305

63066306
#[inline]
63076307
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
6308-
let following_node = self.following_node;
6308+
let following_node = None;
63096309
self.allocator.alloc(AstNode {
63106310
inner: &self.inner.body,
63116311
allocator: self.allocator,
@@ -6396,7 +6396,7 @@ impl<'a> AstNode<'a, ForInStatement<'a>> {
63966396

63976397
#[inline]
63986398
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
6399-
let following_node = self.following_node;
6399+
let following_node = None;
64006400
self.allocator.alloc(AstNode {
64016401
inner: &self.inner.body,
64026402
allocator: self.allocator,
@@ -6492,7 +6492,7 @@ impl<'a> AstNode<'a, ForOfStatement<'a>> {
64926492

64936493
#[inline]
64946494
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
6495-
let following_node = self.following_node;
6495+
let following_node = None;
64966496
self.allocator.alloc(AstNode {
64976497
inner: &self.inner.body,
64986498
allocator: self.allocator,
@@ -6525,7 +6525,7 @@ impl<'a> GetSpan for AstNode<'a, ForOfStatement<'a>> {
65256525
impl<'a> AstNode<'a, ContinueStatement<'a>> {
65266526
#[inline]
65276527
pub fn label(&self) -> Option<&AstNode<'a, LabelIdentifier<'a>>> {
6528-
let following_node = self.following_node;
6528+
let following_node = None;
65296529
self.allocator
65306530
.alloc(self.inner.label.as_ref().map(|inner| AstNode {
65316531
inner,
@@ -6560,7 +6560,7 @@ impl<'a> GetSpan for AstNode<'a, ContinueStatement<'a>> {
65606560
impl<'a> AstNode<'a, BreakStatement<'a>> {
65616561
#[inline]
65626562
pub fn label(&self) -> Option<&AstNode<'a, LabelIdentifier<'a>>> {
6563-
let following_node = self.following_node;
6563+
let following_node = None;
65646564
self.allocator
65656565
.alloc(self.inner.label.as_ref().map(|inner| AstNode {
65666566
inner,
@@ -6641,7 +6641,7 @@ impl<'a> AstNode<'a, WithStatement<'a>> {
66416641

66426642
#[inline]
66436643
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
6644-
let following_node = self.following_node;
6644+
let following_node = None;
66456645
self.allocator.alloc(AstNode {
66466646
inner: &self.inner.body,
66476647
allocator: self.allocator,
@@ -6692,7 +6692,7 @@ impl<'a> AstNode<'a, SwitchStatement<'a>> {
66926692

66936693
#[inline]
66946694
pub fn cases(&self) -> &AstNode<'a, Vec<'a, SwitchCase<'a>>> {
6695-
let following_node = self.following_node;
6695+
let following_node = None;
66966696
self.allocator.alloc(AstNode {
66976697
inner: &self.inner.cases,
66986698
allocator: self.allocator,
@@ -6789,7 +6789,7 @@ impl<'a> AstNode<'a, LabeledStatement<'a>> {
67896789

67906790
#[inline]
67916791
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
6792-
let following_node = self.following_node;
6792+
let following_node = None;
67936793
self.allocator.alloc(AstNode {
67946794
inner: &self.inner.body,
67956795
allocator: self.allocator,
@@ -6886,7 +6886,7 @@ impl<'a> AstNode<'a, TryStatement<'a>> {
68866886

68876887
#[inline]
68886888
pub fn finalizer(&self) -> Option<&AstNode<'a, BlockStatement<'a>>> {
6889-
let following_node = self.following_node;
6889+
let following_node = None;
68906890
self.allocator
68916891
.alloc(self.inner.finalizer.as_ref().map(|inner| AstNode {
68926892
inner: inner.as_ref(),

crates/oxc_formatter/src/generated/format_write.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentTarget<'a>> {
523523
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
524524
let allocator = self.allocator;
525525
let parent = self.parent;
526-
match self.inner {
526+
let needs_parentheses = self.needs_parentheses(f);
527+
if needs_parentheses {
528+
"(".fmt(f)?;
529+
}
530+
let result = match self.inner {
527531
it @ match_simple_assignment_target!(AssignmentTarget) => {
528532
let inner = it.to_simple_assignment_target();
529533
allocator
@@ -546,7 +550,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentTarget<'a>> {
546550
})
547551
.fmt(f)
548552
}
553+
};
554+
if needs_parentheses {
555+
")".fmt(f)?;
549556
}
557+
result
550558
}
551559
}
552560

@@ -555,7 +563,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, SimpleAssignmentTarget<'a>> {
555563
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
556564
let allocator = self.allocator;
557565
let parent = self.parent;
558-
match self.inner {
566+
let needs_parentheses = self.needs_parentheses(f);
567+
if needs_parentheses {
568+
"(".fmt(f)?;
569+
}
570+
let result = match self.inner {
559571
SimpleAssignmentTarget::AssignmentTargetIdentifier(inner) => allocator
560572
.alloc(AstNode::<IdentifierReference> {
561573
inner,
@@ -607,7 +619,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, SimpleAssignmentTarget<'a>> {
607619
})
608620
.fmt(f)
609621
}
622+
};
623+
if needs_parentheses {
624+
")".fmt(f)?;
610625
}
626+
result
611627
}
612628
}
613629

@@ -616,7 +632,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentTargetPattern<'a>> {
616632
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
617633
let allocator = self.allocator;
618634
let parent = self.parent;
619-
match self.inner {
635+
let needs_parentheses = self.needs_parentheses(f);
636+
if needs_parentheses {
637+
"(".fmt(f)?;
638+
}
639+
let result = match self.inner {
620640
AssignmentTargetPattern::ArrayAssignmentTarget(inner) => allocator
621641
.alloc(AstNode::<ArrayAssignmentTarget> {
622642
inner,
@@ -633,7 +653,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentTargetPattern<'a>> {
633653
following_node: self.following_node,
634654
})
635655
.fmt(f),
656+
};
657+
if needs_parentheses {
658+
")".fmt(f)?;
636659
}
660+
result
637661
}
638662
}
639663

0 commit comments

Comments
 (0)