Skip to content

Commit b43db57

Browse files
committed
refactor(formatter): improve printing trailing comments
1 parent 7df4b92 commit b43db57

File tree

7 files changed

+57
-50
lines changed

7 files changed

+57
-50
lines changed

crates/oxc_formatter/src/ast_nodes/generated/format.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4202,13 +4202,11 @@ impl<'a> Format<'a> for AstNode<'a, TSTypeAliasDeclaration<'a>> {
42024202
impl<'a> Format<'a> for AstNode<'a, TSClassImplements<'a>> {
42034203
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
42044204
let is_suppressed = f.comments().is_suppressed(self.span().start);
4205-
if is_suppressed {
4206-
self.format_leading_comments(f)?;
4207-
FormatSuppressedNode(self.span()).fmt(f)?;
4208-
self.format_trailing_comments(f)
4209-
} else {
4210-
self.write(f)
4211-
}
4205+
self.format_leading_comments(f)?;
4206+
let result =
4207+
if is_suppressed { FormatSuppressedNode(self.span()).fmt(f) } else { self.write(f) };
4208+
self.format_trailing_comments(f)?;
4209+
result
42124210
}
42134211
}
42144212

crates/oxc_formatter/src/formatter/comments.rs

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,6 @@ impl<'a> Comments<'a> {
312312
let Some(following_span) = following_span else {
313313
// Find dangling comments at the end of the enclosing node
314314
let comments = self.comments_before(enclosing_span.end);
315-
316315
let mut start = preceding_span.end;
317316
for (idx, comment) in comments.iter().enumerate() {
318317
// Comments inside the preceding node, which should be printed without checking
@@ -333,30 +332,41 @@ impl<'a> Comments<'a> {
333332
};
334333

335334
let mut comment_index = 0;
336-
while let Some(comment) = comments.get(comment_index) {
337-
// Check if the comment is before the following node's span
338-
if comment.span.end > following_span.start {
339-
break;
340-
}
335+
let mut type_cast_comment = None;
341336

342-
if matches!(comment.content, CommentContent::Jsdoc)
343-
&& self.is_type_cast_comment(comment)
337+
while let Some(comment) = comments.get(comment_index) {
338+
// Stop if the comment:
339+
// 1. is over the following node
340+
// 2. is after the enclosing node, which means the comment should be printed in the parent node.
341+
if comment.span.end > following_span.start
342+
|| (comment.span.end > enclosing_span.end && enclosing_span != preceding_span)
344343
{
345344
break;
346345
}
347346

348-
if self.is_own_line_comment(comment) {
349-
// Own line comments are typically leading comments for the next node
347+
if following_span.start > enclosing_span.end && comment.span.end <= enclosing_span.end {
348+
// Do nothing; this comment is inside the enclosing node, and the following node is outside the enclosing node.
349+
// So it must be a trailing comment, continue checking the next comment.
350+
} else if self.is_type_cast_comment(comment) {
351+
// `A || /* @type {Number} */ (B)`:
352+
// ^^^^^^^^^^^^^^^^^^^^^^^^
353+
// Type cast comments should always be treated as leading comment to the following node
354+
type_cast_comment = Some(comment);
355+
break;
356+
} else if self.is_own_line_comment(comment) {
357+
// Own-line comments should be treated as leading comments to the following node
350358
break;
351359
} else if self.is_end_of_line_comment(comment) {
360+
//End-of-line comments are always trailing comments to the preceding node.
352361
return &comments[..=comment_index];
353362
}
354363

355364
comment_index += 1;
356365
}
357366

358367
// Find the first comment (from the end) that has non-whitespace/non-paren content after it
359-
let mut gap_end = following_span.start;
368+
let mut gap_end = type_cast_comment.map_or(following_span.start, |c| c.span.start);
369+
360370
for (idx, comment) in comments[..comment_index].iter().enumerate().rev() {
361371
if source_text.all_bytes_match(comment.span.end, gap_end, |b| {
362372
b.is_ascii_whitespace() || b == b'('

crates/oxc_formatter/src/write/binary_like_expression.rs

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use crate::{
1313
ast_nodes::{AstNode, AstNodes},
1414
formatter::{FormatResult, Formatter, trivia::FormatTrailingComments},
1515
parentheses::NeedsParentheses,
16-
utils::format_node_without_trailing_comments::FormatNodeWithoutTrailingComments,
1716
};
1817

1918
use crate::{format_args, formatter::prelude::*, write};
@@ -294,8 +293,6 @@ enum BinaryLeftOrRightSide<'a, 'b> {
294293
parent: BinaryLikeExpression<'a, 'b>,
295294
/// Is the parent the condition of a `if` / `while` / `do-while` / `for` statement?
296295
inside_condition: bool,
297-
/// It is the root of the expression.
298-
root: bool,
299296
},
300297
}
301298

@@ -306,7 +303,6 @@ impl<'a> Format<'a> for BinaryLeftOrRightSide<'a, '_> {
306303
Self::Right {
307304
parent: binary_like_expression,
308305
inside_condition: inside_parenthesis,
309-
root,
310306
} => {
311307
let mut binary_like_expression = *binary_like_expression;
312308
// // It's only possible to suppress the formatting of the whole binary expression formatting OR
@@ -401,16 +397,7 @@ impl<'a> Format<'a> for BinaryLeftOrRightSide<'a, '_> {
401397
write!(f, [soft_line_break_or_space()])?;
402398
}
403399

404-
if *root {
405-
write!(f, FormatNodeWithoutTrailingComments(right))?;
406-
let comments = f
407-
.context()
408-
.comments()
409-
.comments_before(binary_like_expression.span().end);
410-
write!(f, FormatTrailingComments::Comments(comments))
411-
} else {
412-
write!(f, right)
413-
}
400+
write!(f, right)
414401
});
415402

416403
// Doesn't match prettier that only distinguishes between logical and binary
@@ -425,6 +412,16 @@ impl<'a> Format<'a> for BinaryLeftOrRightSide<'a, '_> {
425412
right.as_ast_nodes(),
426413
) || (*inside_parenthesis && logical_operator.is_some()));
427414

415+
match binary_like_expression.left().as_ast_nodes() {
416+
AstNodes::LogicalExpression(logical) => {
417+
logical.format_trailing_comments(f)?;
418+
}
419+
AstNodes::BinaryExpression(binary) => {
420+
binary.format_trailing_comments(f)?;
421+
}
422+
_ => {}
423+
}
424+
428425
if should_group {
429426
// `left` side has printed before `right` side, so that trailing comments of `left` side has been printed,
430427
// so we need to find if there are any printed comments that are after the `left` side and it is line comment.
@@ -489,7 +486,6 @@ fn split_into_left_and_right_sides<'a, 'b>(
489486
inside_condition: bool,
490487
) -> Vec<BinaryLeftOrRightSide<'a, 'b>> {
491488
fn split_into_left_and_right_sides_inner<'a, 'b>(
492-
is_root: bool,
493489
binary: BinaryLikeExpression<'a, 'b>,
494490
inside_condition: bool,
495491
items: &mut Vec<BinaryLeftOrRightSide<'a, 'b>>,
@@ -500,7 +496,6 @@ fn split_into_left_and_right_sides<'a, 'b>(
500496
// We can flatten the left hand side, so we need to check if we have a nested binary expression
501497
// that we can flatten.
502498
split_into_left_and_right_sides_inner(
503-
false,
504499
// SAFETY: `left` is guaranteed to be a valid binary like expression in `can_flatten()`.
505500
BinaryLikeExpression::try_from(left).unwrap(),
506501
inside_condition,
@@ -510,19 +505,15 @@ fn split_into_left_and_right_sides<'a, 'b>(
510505
items.push(BinaryLeftOrRightSide::Left { parent: binary });
511506
}
512507

513-
items.push(BinaryLeftOrRightSide::Right {
514-
parent: binary,
515-
inside_condition,
516-
root: is_root,
517-
});
508+
items.push(BinaryLeftOrRightSide::Right { parent: binary, inside_condition });
518509
}
519510

520511
// Stores the left and right parts of the binary expression in sequence (rather than nested as they
521512
// appear in the tree).
522513
// `with_capacity(2)` because we expect at least 2 items (left and right).
523514
let mut items = Vec::with_capacity(2);
524515

525-
split_into_left_and_right_sides_inner(true, binary, inside_condition, &mut items);
516+
split_into_left_and_right_sides_inner(binary, inside_condition, &mut items);
526517

527518
items
528519
}

crates/oxc_formatter/src/write/class.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -235,14 +235,22 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, TSClassImplements<'a>>> {
235235
group(&indent(&format_args!(
236236
soft_line_break_or_space(),
237237
format_once(|f| {
238-
// the grouping will be applied by the parent
239-
f.join_with(&soft_line_break_or_space())
240-
.entries_with_trailing_separator(
241-
self.iter(),
242-
",",
243-
TrailingSeparator::Disallowed,
244-
)
245-
.finish()
238+
let last_index = self.len().saturating_sub(1);
239+
let mut joiner = f.join_with(soft_line_break_or_space());
240+
241+
for (i, heritage) in FormatSeparatedIter::new(self.into_iter(), ",")
242+
.with_trailing_separator(TrailingSeparator::Disallowed)
243+
.enumerate()
244+
{
245+
if i == last_index {
246+
// The trailing comments of the last heritage should be printed inside the class declaration
247+
joiner.entry(&FormatNodeWithoutTrailingComments(&heritage));
248+
} else {
249+
joiner.entry(&heritage);
250+
}
251+
}
252+
253+
joiner.finish()
246254
})
247255
)))
248256
]

crates/oxc_formatter/src/write/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1628,6 +1628,7 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, TSInterfaceHeritage<'a>>> {
16281628
.enumerate()
16291629
{
16301630
if i == last_index {
1631+
// The trailing comments of the last heritage should be printed inside the interface declaration
16311632
joiner.entry(&FormatNodeWithoutTrailingComments(&heritage));
16321633
} else {
16331634
joiner.entry(&heritage);

crates/oxc_formatter/tests/fixtures/js/comments/type-cast-node.js.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ source: crates/oxc_formatter/tests/fixtures/mod.rs
2323
(a) === "call" ||
2424
/** @type {Identifier} */
2525
(b) === "bind"
26+
// ^^^^^^^^^^^^^^ No need to wrap with parentheses here because the type cast node is already wrapped with parentheses.
2627
) &&
27-
// ^^^^^^^^^^^^^^ No need to wrap with parentheses here because the type cast node is already wrapped with parentheses.
2828
right;
2929

3030
/** @type {Number} */ (a + b)();

tasks/ast_tools/src/generators/formatter/format.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ const AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST: &[&str] = &[
2424
// Manually prints it because class's decorators can be appears before `export class Cls {}`.
2525
"ExportNamedDeclaration",
2626
"ExportDefaultDeclaration",
27-
"TSClassImplements",
2827
//
2928
"JSXElement",
3029
"JSXFragment",

0 commit comments

Comments
 (0)