Skip to content

feat(formatter): complete printing ForStatement, ForInStatement and ForOfStatement #12810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 7, 2025
Merged

feat(formatter): complete printing ForStatement, ForInStatement and ForOfStatement #12810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 102 additions & 17 deletions crates/oxc_formatter/src/formatter/comments/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,25 @@ impl<'a> Comments<'a> {
&comments[..index]
}

/// Returns own line comments that are before the given `pos`.
pub fn own_line_comments_before(&self, pos: u32) -> &'a [Comment] {
let mut index = 0;

let comments = self.unprinted_comments();
for comment in comments {
if !is_own_line_comment(comment, self.source_text) {
break;
}

if comment.span.end > pos {
break;
}
index += 1;
}

&comments[..index]
}

/// Returns the comments that after the given `start` position, even if they were already printed.
pub fn comments_after(&self, pos: u32) -> &'a [Comment] {
let mut index = self.printed_count;
Expand Down Expand Up @@ -279,15 +298,39 @@ impl<'a> Comments<'a> {
&self,
enclosing_node: &SiblingNode<'a>,
preceding_node: &SiblingNode<'a>,
following_node: Option<&SiblingNode<'a>>,
mut following_node: Option<&SiblingNode<'a>>,
) -> &'a [Comment] {
if !matches!(
enclosing_node,
SiblingNode::Program(_)
| SiblingNode::BlockStatement(_)
| SiblingNode::FunctionBody(_)
| SiblingNode::TSModuleBlock(_)
| SiblingNode::SwitchStatement(_)
| SiblingNode::StaticBlock(_)
) && matches!(following_node, Some(SiblingNode::EmptyStatement(_)))
{
let enclosing_span = enclosing_node.span();
return self.comments_before(enclosing_span.end);
}

// The preceding_node is the callee of the call expression or new expression, let following node to print it.
// Based on https://github.com/prettier/prettier/blob/7584432401a47a26943dd7a9ca9a8e032ead7285/src/language-js/comments/handle-comments.js#L726-L741
if matches!(enclosing_node, SiblingNode::CallExpression(CallExpression { callee, ..}) | SiblingNode::NewExpression(NewExpression { callee, ..}) if callee.span().contains_inclusive(preceding_node.span()))
{
return &[];
}

// No need to print trailing comments for the most right side for `BinaryExpression` and `LogicalExpression`,
// instead, print trailing comments for expression itself.
if matches!(
enclosing_node,
SiblingNode::BinaryExpression(_) | SiblingNode::LogicalExpression(_)
) && matches!(following_node, Some(SiblingNode::ExpressionStatement(_)))
{
return &[];
}

let comments = self.unprinted_comments();
if comments.is_empty() {
return &[];
Expand All @@ -303,15 +346,11 @@ impl<'a> Comments<'a> {

let Some(following_node) = following_node else {
let enclosing_span = enclosing_node.span();
return comments
.iter()
.enumerate()
.take_while(|(_, comment)| comment.span.end <= enclosing_span.end)
.last()
.map_or(&[], |(index, _)| &comments[..=index]);
return self.comments_before(enclosing_span.end);
};

let following_span = following_node.span();

let mut comment_index = 0;
while let Some(comment) = comments.get(comment_index) {
// Check if the comment is before the following node's span
Expand All @@ -322,21 +361,42 @@ impl<'a> Comments<'a> {
if is_own_line_comment(comment, source_text) {
// TODO: describe the logic here
// Reached an own line comment, which means it is the leading comment for the next node.

if matches!(enclosing_node, SiblingNode::IfStatement(stmt) if stmt.test.span() == preceding_span)
|| matches!(enclosing_node, SiblingNode::WhileStatement(stmt) if stmt.test.span() == preceding_span)
{
return handle_if_and_while_statement_comments(
following_span.start,
comment_index,
comments,
source_text,
);
}

break;
} else if is_end_of_line_comment(comment, source_text) {
let first_comment_start = comments[0].span.start;
// TODO: Maybe we can remove this check once we complete the logic of handling comments.
if preceding_span.end < first_comment_start {
let is_break = source_text.as_bytes()
[(preceding_span.end as usize)..(first_comment_start as usize)]
.iter()
.any(|&b| matches!(b, b'\n' | b'\r' | b')'));

if is_break {
return &[];
if let SiblingNode::IfStatement(if_stmt) = enclosing_node {
if if_stmt.consequent.span() == preceding_span {
// If comment is after the `else` keyword, it is not a trailing comment of consequent.
if source_text[preceding_span.end as usize..comment.span.start as usize]
.contains("else")
{
return &[];
}
}
}

if matches!(enclosing_node, SiblingNode::IfStatement(stmt) if stmt.test.span() == preceding_span)
|| matches!(enclosing_node, SiblingNode::WhileStatement(stmt) if stmt.test.span() == preceding_span)
{
return handle_if_and_while_statement_comments(
following_span.start,
comment_index,
comments,
source_text,
);
}

// Should be a leading comment of following node.
// Based on https://github.com/prettier/prettier/blob/7584432401a47a26943dd7a9ca9a8e032ead7285/src/language-js/comments/handle-comments.js#L852-L883
if matches!(
Expand Down Expand Up @@ -383,6 +443,31 @@ impl<'a> Comments<'a> {
}
}

fn handle_if_and_while_statement_comments<'a>(
mut end: u32,
mut comment_index: usize,
comments: &'a [Comment],
source_text: &'a str,
) -> &'a [Comment] {
// `if (a /* comment before paren */) // comment after paren`
loop {
let cur_comment_span = comments[comment_index].span;
if source_text.as_bytes()[cur_comment_span.end as usize..end as usize].contains(&b')') {
return &comments[..=comment_index];
}

end = cur_comment_span.start;

if comment_index == 0 {
return &[];
}

comment_index -= 1;
}

unreachable!()
}

#[inline]
pub fn is_new_line(char: char) -> ControlFlow<bool> {
if char == ' ' || char == '\t' {
Expand Down
15 changes: 7 additions & 8 deletions crates/oxc_formatter/src/formatter/trivia.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,13 @@ impl<'a> Format<'a> for FormatTrailingComments<'a, '_> {

match self {
Self::Node((enclosing_node, preceding_node, following_node)) => {
format_trailing_comments_impl(
f.context().comments().get_trailing_comments(
enclosing_node,
preceding_node,
*following_node,
),
f,
)
let comments = f.context().comments().get_trailing_comments(
enclosing_node,
preceding_node,
*following_node,
);

format_trailing_comments_impl(comments, f)
}
Self::Comments(comments) => format_trailing_comments_impl(*comments, f),
}
Expand Down
24 changes: 12 additions & 12 deletions crates/oxc_formatter/src/generated/ast_nodes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5842,7 +5842,7 @@ impl<'a> GetSpan for AstNode<'a, Hashbang<'a>> {
impl<'a> AstNode<'a, BlockStatement<'a>> {
#[inline]
pub fn body(&self) -> &AstNode<'a, Vec<'a, Statement<'a>>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.body,
allocator: self.allocator,
Expand Down Expand Up @@ -6178,7 +6178,7 @@ impl<'a> AstNode<'a, DoWhileStatement<'a>> {

#[inline]
pub fn test(&self) -> &AstNode<'a, Expression<'a>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.test,
allocator: self.allocator,
Expand Down Expand Up @@ -6222,7 +6222,7 @@ impl<'a> AstNode<'a, WhileStatement<'a>> {

#[inline]
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.body,
allocator: self.allocator,
Expand Down Expand Up @@ -6305,7 +6305,7 @@ impl<'a> AstNode<'a, ForStatement<'a>> {

#[inline]
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.body,
allocator: self.allocator,
Expand Down Expand Up @@ -6396,7 +6396,7 @@ impl<'a> AstNode<'a, ForInStatement<'a>> {

#[inline]
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.body,
allocator: self.allocator,
Expand Down Expand Up @@ -6492,7 +6492,7 @@ impl<'a> AstNode<'a, ForOfStatement<'a>> {

#[inline]
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.body,
allocator: self.allocator,
Expand Down Expand Up @@ -6525,7 +6525,7 @@ impl<'a> GetSpan for AstNode<'a, ForOfStatement<'a>> {
impl<'a> AstNode<'a, ContinueStatement<'a>> {
#[inline]
pub fn label(&self) -> Option<&AstNode<'a, LabelIdentifier<'a>>> {
let following_node = self.following_node;
let following_node = None;
self.allocator
.alloc(self.inner.label.as_ref().map(|inner| AstNode {
inner,
Expand Down Expand Up @@ -6560,7 +6560,7 @@ impl<'a> GetSpan for AstNode<'a, ContinueStatement<'a>> {
impl<'a> AstNode<'a, BreakStatement<'a>> {
#[inline]
pub fn label(&self) -> Option<&AstNode<'a, LabelIdentifier<'a>>> {
let following_node = self.following_node;
let following_node = None;
self.allocator
.alloc(self.inner.label.as_ref().map(|inner| AstNode {
inner,
Expand Down Expand Up @@ -6641,7 +6641,7 @@ impl<'a> AstNode<'a, WithStatement<'a>> {

#[inline]
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.body,
allocator: self.allocator,
Expand Down Expand Up @@ -6692,7 +6692,7 @@ impl<'a> AstNode<'a, SwitchStatement<'a>> {

#[inline]
pub fn cases(&self) -> &AstNode<'a, Vec<'a, SwitchCase<'a>>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.cases,
allocator: self.allocator,
Expand Down Expand Up @@ -6789,7 +6789,7 @@ impl<'a> AstNode<'a, LabeledStatement<'a>> {

#[inline]
pub fn body(&self) -> &AstNode<'a, Statement<'a>> {
let following_node = self.following_node;
let following_node = None;
self.allocator.alloc(AstNode {
inner: &self.inner.body,
allocator: self.allocator,
Expand Down Expand Up @@ -6886,7 +6886,7 @@ impl<'a> AstNode<'a, TryStatement<'a>> {

#[inline]
pub fn finalizer(&self) -> Option<&AstNode<'a, BlockStatement<'a>>> {
let following_node = self.following_node;
let following_node = None;
self.allocator
.alloc(self.inner.finalizer.as_ref().map(|inner| AstNode {
inner: inner.as_ref(),
Expand Down
30 changes: 27 additions & 3 deletions crates/oxc_formatter/src/generated/format_write.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentTarget<'a>> {
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
let allocator = self.allocator;
let parent = self.parent;
match self.inner {
let needs_parentheses = self.needs_parentheses(f);
if needs_parentheses {
"(".fmt(f)?;
}
let result = match self.inner {
it @ match_simple_assignment_target!(AssignmentTarget) => {
let inner = it.to_simple_assignment_target();
allocator
Expand All @@ -546,7 +550,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentTarget<'a>> {
})
.fmt(f)
}
};
if needs_parentheses {
")".fmt(f)?;
}
result
}
}

Expand All @@ -555,7 +563,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, SimpleAssignmentTarget<'a>> {
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
let allocator = self.allocator;
let parent = self.parent;
match self.inner {
let needs_parentheses = self.needs_parentheses(f);
if needs_parentheses {
"(".fmt(f)?;
}
let result = match self.inner {
SimpleAssignmentTarget::AssignmentTargetIdentifier(inner) => allocator
.alloc(AstNode::<IdentifierReference> {
inner,
Expand Down Expand Up @@ -607,7 +619,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, SimpleAssignmentTarget<'a>> {
})
.fmt(f)
}
};
if needs_parentheses {
")".fmt(f)?;
}
result
}
}

Expand All @@ -616,7 +632,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentTargetPattern<'a>> {
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
let allocator = self.allocator;
let parent = self.parent;
match self.inner {
let needs_parentheses = self.needs_parentheses(f);
if needs_parentheses {
"(".fmt(f)?;
}
let result = match self.inner {
AssignmentTargetPattern::ArrayAssignmentTarget(inner) => allocator
.alloc(AstNode::<ArrayAssignmentTarget> {
inner,
Expand All @@ -633,7 +653,11 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentTargetPattern<'a>> {
following_node: self.following_node,
})
.fmt(f),
};
if needs_parentheses {
")".fmt(f)?;
}
result
}
}

Expand Down
Loading
Loading