Skip to content

refactor(formatter): improve printing comments for special nodes #14544

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
Oct 14, 2025
Merged

refactor(formatter): improve printing comments for special nodes #14544

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
117 changes: 5 additions & 112 deletions crates/oxc_formatter/src/formatter/comments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,10 @@ impl<'a> Comments<'a> {
pub fn end_of_line_comments_after(&self, mut pos: u32) -> &'a [Comment] {
let comments = self.unprinted_comments();
for (index, comment) in comments.iter().enumerate() {
if self
.source_text
.all_bytes_match(pos, comment.span.start, |b| matches!(b, b'\t' | b' ' | b')'))
{
if !self.source_text.is_own_line_comment(comment)
&& (comment.is_line() || self.source_text.is_end_of_line_comment(comment))
{
if self.source_text.all_bytes_match(pos, comment.span.start, |b| {
matches!(b, b'\t' | b' ' | b')' | b'=')
}) {
if comment.is_line() || self.source_text.is_end_of_line_comment(comment) {
return &comments[..=index];
}
pos = comment.span.end;
Expand Down Expand Up @@ -288,7 +285,7 @@ impl<'a> Comments<'a> {
/// Used when multiple comments are processed in batch. Each unit of `count`
/// represents one comment that has been formatted and should be marked as processed.
///
/// Like [`increment_printed_count`], this is essential for maintaining the
/// Like [`Comments::increment_printed_count`], this is essential for maintaining the
/// integrity of the comment tracking system.
#[inline]
pub fn increase_printed_count_by(&mut self, count: usize) {
Expand All @@ -303,27 +300,6 @@ impl<'a> Comments<'a> {
preceding_node: &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);
}

// If preceding_node is a callee, let the following node handle its comments
// Based on Prettier's comment handling logic
if matches!(enclosing_node, SiblingNode::CallExpression(CallExpression { callee, ..}) | SiblingNode::NewExpression(NewExpression { callee, ..}) if callee.span().contains_inclusive(preceding_node.span()))
{
return &[];
}

let comments = self.unprinted_comments();
if comments.is_empty() {
return &[];
Expand Down Expand Up @@ -377,78 +353,14 @@ impl<'a> Comments<'a> {

if source_text.is_own_line_comment(comment) {
// Own line comments are typically leading comments 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 self.source_text.is_end_of_line_comment(comment) {
if let SiblingNode::IfStatement(if_stmt) = enclosing_node
&& 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,
);
}

// End-of-line comments in specific contexts should be leading comments
if matches!(
enclosing_node,
SiblingNode::VariableDeclarator(_)
| SiblingNode::AssignmentExpression(_)
| SiblingNode::TSTypeAliasDeclaration(_)
) && (comment.is_block()
|| matches!(
following_node,
SiblingNode::ObjectExpression(_)
| SiblingNode::ArrayExpression(_)
| SiblingNode::TSTypeLiteral(_)
| SiblingNode::TemplateLiteral(_)
| SiblingNode::TaggedTemplateExpression(_)
))
{
return &[];
}
return &comments[..=comment_index];
}

comment_index += 1;
}

if comment_index == 0 {
// No comments to print
return &[];
}

if matches!(
enclosing_node,
SiblingNode::ImportDeclaration(_) | SiblingNode::ExportAllDeclaration(_)
) {
return &comments[..comment_index];
}

// Find the first comment (from the end) that has non-whitespace/non-paren content after it
let mut gap_end = following_span.start;
for (idx, comment) in comments[..comment_index].iter().enumerate().rev() {
Expand Down Expand Up @@ -557,22 +469,3 @@ fn matches_pattern_at(bytes: &[u8], pos: usize, pattern: &[u8]) -> bool {
bytes[pos..].starts_with(pattern)
&& matches!(bytes.get(pos + pattern.len()), Some(b' ' | b'\t' | b'\n' | b'\r' | b'{'))
}

/// Handles comment placement logic for if and while statements.
fn handle_if_and_while_statement_comments<'a>(
mut end: u32,
comment_index: usize,
comments: &'a [Comment],
source_text: SourceText,
) -> &'a [Comment] {
// Handle pattern: `if (a /* comment */) // trailing comment`
// Find the last comment that contains ')' between its end and the current end
for (idx, comment) in comments[..=comment_index].iter().enumerate().rev() {
if source_text.bytes_contain(comment.span.end, end, b')') {
return &comments[..=idx];
}
end = comment.span.start;
}

&[]
}
81 changes: 69 additions & 12 deletions crates/oxc_formatter/src/utils/assignment_like.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ use crate::{
formatter::{
Buffer, BufferExtensions, Format, FormatResult, Formatter, VecBuffer,
prelude::{FormatElements, format_once, line_suffix_boundary, *},
trivia::{FormatLeadingComments, format_dangling_comments},
trivia::{FormatLeadingComments, FormatTrailingComments},
},
generated::ast_nodes::{AstNode, AstNodes},
options::Expand,
parentheses::NeedsParentheses,
utils::{
format_node_without_trailing_comments::FormatNodeWithoutTrailingComments,
member_chain::is_member_call_chain,
Expand Down Expand Up @@ -149,20 +150,66 @@ pub enum AssignmentLikeLayout {
SuppressedInitializer,
}

const MIN_OVERLAP_FOR_BREAK: u8 = 3;
/// Based on Prettier's behavior:
/// <https://github.com/prettier/prettier/blob/7584432401a47a26943dd7a9ca9a8e032ead7285/src/language-js/comments/handle-comments.js#L853-L883>
fn format_left_trailing_comments(
start: u32,
should_print_as_leading: bool,
f: &mut Formatter<'_, '_>,
) -> FormatResult<()> {
let end_of_line_comments = f.context().comments().end_of_line_comments_after(start);

let comments = if end_of_line_comments.is_empty() {
let comments = f.context().comments().comments_before_character(start, b'=');
if comments.iter().any(|c| f.source_text().is_own_line_comment(c)) { &[] } else { comments }
} else if should_print_as_leading || end_of_line_comments.last().is_some_and(|c| c.is_block()) {
// No trailing comments for these expressions or if the trailing comment is a block comment
&[]
} else {
end_of_line_comments
};

FormatTrailingComments::Comments(comments).fmt(f)
}

fn should_print_as_leading(expr: &Expression) -> bool {
matches!(
expr,
Expression::ObjectExpression(_)
| Expression::ArrayExpression(_)
| Expression::TemplateLiteral(_)
| Expression::TaggedTemplateExpression(_)
)
}

impl<'a> AssignmentLike<'a, '_> {
fn write_left(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<bool> {
match self {
AssignmentLike::VariableDeclarator(variable_declarator) => {
write!(f, variable_declarator.id())?;
AssignmentLike::VariableDeclarator(declarator) => {
if let Some(init) = &declarator.init {
write!(f, [FormatNodeWithoutTrailingComments(&declarator.id())])?;
format_left_trailing_comments(
declarator.id.span().end,
should_print_as_leading(init),
f,
)?;
} else {
write!(f, declarator.id())?;
}
Ok(false)
}
AssignmentLike::AssignmentExpression(assignment) => {
write!(f, [assignment.left()]);
write!(f, [FormatNodeWithoutTrailingComments(&assignment.left()),])?;
format_left_trailing_comments(
assignment.left.span().end,
should_print_as_leading(&assignment.right),
f,
)?;
Ok(false)
}
AssignmentLike::ObjectProperty(property) => {
const MIN_OVERLAP_FOR_BREAK: u8 = 3;

let text_width_for_break =
(f.options().indent_width.value() + MIN_OVERLAP_FOR_BREAK) as usize;

Expand Down Expand Up @@ -226,15 +273,25 @@ impl<'a> AssignmentLike<'a, '_> {
Ok(false) // Class properties don't use "short" key logic
}
AssignmentLike::TSTypeAliasDeclaration(declaration) => {
write!(
write!(f, [declaration.declare.then_some("declare "), "type "])?;

let start = if let Some(type_parameters) = &declaration.type_parameters() {
write!(
f,
[declaration.id(), FormatNodeWithoutTrailingComments(type_parameters)]
)?;
type_parameters.span.end
} else {
write!(f, [FormatNodeWithoutTrailingComments(declaration.id())])?;
declaration.id.span.end
};

format_left_trailing_comments(
start,
matches!(&declaration.type_annotation, TSType::TSTypeLiteral(_)),
f,
[
declaration.declare.then_some("declare "),
"type ",
declaration.id(),
declaration.type_parameters()
]
)?;

Ok(false)
}
}
Expand Down
13 changes: 9 additions & 4 deletions crates/oxc_formatter/src/utils/member_chain/chain_member.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
prelude::*,
trivia::{FormatLeadingComments, FormatTrailingComments},
},
generated::ast_nodes::AstNode,
generated::ast_nodes::{AstNode, AstNodes},
write,
};
use oxc_ast::{AstKind, ast::*};
Expand Down Expand Up @@ -77,15 +77,20 @@ impl<'a> Format<'a> for ChainMember<'a, '_> {
member.property()
]
)?;
member.format_trailing_comments(f)
}

// `A.b /* comment */ (c)` -> `A.b(/* comment */ c)`
if !matches!(member.parent, AstNodes::CallExpression(call) if call.type_arguments.is_none())
{
member.format_trailing_comments(f)?;
}

Ok(())
}
Self::TSNonNullExpression(e) => {
e.format_leading_comments(f)?;
write!(f, ["!"])?;
e.format_trailing_comments(f)
}

Self::CallExpression { expression, position } => match *position {
CallExpressionPosition::Start => write!(f, expression),
CallExpressionPosition::Middle => {
Expand Down
19 changes: 11 additions & 8 deletions crates/oxc_formatter/src/write/export_declarations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ use crate::{
},
generated::ast_nodes::{AstNode, AstNodes},
write,
write::semicolon::OptionalSemicolon,
write::{
import_declaration::format_import_and_export_source_with_clause,
semicolon::OptionalSemicolon,
},
};

use super::FormatWrite;
Expand Down Expand Up @@ -78,7 +81,10 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ExportAllDeclaration<'a>> {
if let Some(name) = &self.exported() {
write!(f, ["as", space(), name, space()])?;
}
write!(f, ["from", space(), self.source(), self.with_clause(), OptionalSemicolon])
write!(f, ["from", space()]);

format_import_and_export_source_with_clause(self.source(), self.with_clause(), f)?;
write!(f, [OptionalSemicolon])
}
}

Expand All @@ -88,7 +94,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ExportNamedDeclaration<'a>> {
let export_kind = self.export_kind();
let specifiers = self.specifiers();
let source = self.source();
let with_clause = self.with_clause();

if let Some(decl) = declaration {
format_export_keyword_with_class_decorators(
Expand Down Expand Up @@ -147,12 +152,10 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ExportNamedDeclaration<'a>> {
}
write!(f, "}")?;

let with_clause = self.with_clause();
if let Some(source) = source {
write!(f, [space(), "from", space(), source])?;
}

if let Some(with_clause) = with_clause {
write!(f, [space(), with_clause])?;
write!(f, [space(), "from", space()])?;
format_import_and_export_source_with_clause(source, with_clause, f)?;
}
}

Expand Down
23 changes: 22 additions & 1 deletion crates/oxc_formatter/src/write/import_declaration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,25 @@ impl<'a> Format<'a> for ImportOrExportKind {
}
}

pub fn format_import_and_export_source_with_clause<'a>(
source: &AstNode<'a, StringLiteral>,
with_clause: Option<&AstNode<'a, WithClause>>,
f: &mut Formatter<'_, 'a>,
) -> FormatResult<()> {
source.format_leading_comments(f)?;
source.write(f)?;

if let Some(with_clause) = with_clause {
if f.comments().has_comment_before(with_clause.span.start) {
write!(f, [space()])?;
}

write!(f, [with_clause])?;
}

Ok(())
}

impl<'a> FormatWrite<'a> for AstNode<'a, ImportDeclaration<'a>> {
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
let decl = &format_once(|f| {
Expand All @@ -34,7 +53,9 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ImportDeclaration<'a>> {
write!(f, [specifiers, space(), "from", space()])?;
}

write!(f, [self.source(), self.with_clause(), OptionalSemicolon])
format_import_and_export_source_with_clause(self.source(), self.with_clause(), f)?;

write!(f, [OptionalSemicolon])
});

write!(f, [labelled(LabelId::of(JsLabels::ImportDeclaration), decl)])
Expand Down
Loading
Loading