fix(formatter): correct comments checking for some places

Dunqing · Dunqing · commit 20c2f93b2b92 · 2025-09-26T19:55:20.000+08:00
diff --git a/crates/oxc_formatter/src/formatter/comments.rs b/crates/oxc_formatter/src/formatter/comments.rs
@@ -236,7 +236,7 @@ impl<'a> Comments<'a> {
 
     /// Checks if there are any comments between the given positions.
     pub fn has_comment_in_range(&self, start: u32, end: u32) -> bool {
-        self.comments_before_iter(end).any(|comment| comment.span.end >= start)
+        self.comments_before_iter(end).any(|comment| comment.span.end > start)
     }
 
     /// Checks if there are any comments within the given span.
@@ -253,10 +253,8 @@ impl<'a> Comments<'a> {
 
     /// Checks if there are any leading own-line comments before the given position.
     pub fn has_leading_own_line_comment(&self, start: u32) -> bool {
-        self.comments_before_iter(start).any(|comment| {
-            self.source_text.is_own_line_comment(comment)
-                || self.source_text.lines_after(comment.span.end) > 0
-        })
+        self.comments_before_iter(start)
+            .any(|comment| self.source_text.lines_after(comment.span.end) > 0)
     }
 
     /// Checks if there are leading or trailing comments around `current_span`.
diff --git a/crates/oxc_formatter/src/formatter/source_text.rs b/crates/oxc_formatter/src/formatter/source_text.rs
@@ -146,7 +146,7 @@ impl<'a> SourceText<'a> {
 
         // Should skip the leading comments of the node.
         if let Some(comment) = comments.first()
-            && comment.span.end < start
+            && comment.span.end <= start
         {
             start = comment.span.start;
         }
diff --git a/crates/oxc_formatter/src/utils/assignment_like.rs b/crates/oxc_formatter/src/utils/assignment_like.rs
@@ -314,19 +314,20 @@ impl<'a> AssignmentLike<'a, '_> {
         }
 
         let right_expression = self.get_right_expression();
+        if let Some(expr) = right_expression {
+            if let Some(layout) = self.chain_formatting_layout(expr) {
+                return layout;
+            }
 
-        if let Some(layout) = right_expression.and_then(|expr| self.chain_formatting_layout(expr)) {
-            return layout;
-        }
-
-        if let Some(Expression::CallExpression(call_expression)) =
-            &right_expression.map(AsRef::as_ref)
-            && call_expression
-                .callee
-                .get_identifier_reference()
-                .is_some_and(|ident| ident.name == "require")
-        {
-            return AssignmentLikeLayout::NeverBreakAfterOperator;
+            if let Expression::CallExpression(call_expression) = expr.as_ref()
+                && call_expression
+                    .callee
+                    .get_identifier_reference()
+                    .is_some_and(|ident| ident.name == "require")
+            {
+                
+                return AssignmentLikeLayout::NeverBreakAfterOperator;
+            }
         }
 
         if self.should_break_after_operator(right_expression, f) {
@@ -359,10 +360,8 @@ impl<'a> AssignmentLike<'a, '_> {
             return AssignmentLikeLayout::BreakAfterOperator;
         }
 
-        let is_poorly_breakable = match &right_expression {
-            Some(expression) => is_poorly_breakable_member_or_call_chain(expression, f),
-            None => false,
-        };
+        let is_poorly_breakable =
+            right_expression.is_some_and(|expr| is_poorly_breakable_member_or_call_chain(expr, f));
 
         if is_poorly_breakable {
             return AssignmentLikeLayout::BreakAfterOperator;
@@ -581,17 +580,19 @@ fn should_break_after_operator<'a>(
 ) -> bool {
     let is_jsx = matches!(right.as_ref(), Expression::JSXElement(_) | Expression::JSXFragment(_));
 
+    if is_jsx {
+        return false;
+    }
+
+    let comments = f.comments();
     let source_text = f.source_text();
-    for comment in f.comments().comments_before(right.span().start) {
-        if !is_jsx
-            && (source_text.lines_after(comment.span.end) > 0
-                || source_text.is_own_line_comment(comment))
-        {
+    for comment in comments.comments_before(right.span().start) {
+        if source_text.lines_after(comment.span.end) > 0 {
             return true;
         }
 
         // Needs to wrap a parenthesis for the node, so it won't break.
-        if f.comments().is_type_cast_comment(comment) {
+        if comments.is_type_cast_comment(comment) {
             return false;
         }
     }
diff --git a/crates/oxc_formatter/src/write/member_expression.rs b/crates/oxc_formatter/src/write/member_expression.rs
@@ -1,11 +1,18 @@
 use std::ops::Deref;
 
 use oxc_ast::ast::*;
+use oxc_span::GetSpan;
 
 use crate::{
     JsLabels, format_args,
     formatter::{
-        Buffer, Format, FormatResult, Formatter, prelude::*, trivia::format_dangling_comments,
+        Buffer, Format, FormatResult, Formatter,
+        buffer::RemoveSoftLinesBuffer,
+        prelude::*,
+        trivia::{
+            DanglingIndentMode, FormatDanglingComments, FormatLeadingComments,
+            FormatTrailingComments, format_dangling_comments,
+        },
     },
     generated::ast_nodes::{AstNode, AstNodes},
     options::Expand,
@@ -30,7 +37,14 @@ impl<'a> FormatWrite<'a> for AstNode<'a, StaticMemberExpression<'a>> {
             recording.stop().has_label(LabelId::of(JsLabels::MemberChain))
         };
 
-        match layout(self, is_member_chain) {
+        let comments = f.context().comments().block_comments_before(self.property.span.start);
+        if !comments.is_empty() {
+            write!(f, [space()])?;
+            f.context_mut().comments_mut().increase_printed_count_by(comments.len());
+            f.join_with(space()).entries(comments.iter()).finish()?;
+        }
+
+        match layout(self, is_member_chain, f) {
             StaticMemberLayout::NoBreak => {
                 let format_no_break =
                     format_with(|f| write!(f, [operator_token(self.optional()), self.property()]));
@@ -46,6 +60,17 @@ impl<'a> FormatWrite<'a> for AstNode<'a, StaticMemberExpression<'a>> {
                     f,
                     [group(&indent(&format_args!(
                         soft_line_break(),
+                        &format_once(|f| {
+                            let comments =
+                                f.context().comments().comments_before(self.property.span.start);
+                            if !comments.is_empty() {
+                                write!(
+                                    f,
+                                    [FormatLeadingComments::Comments(comments), soft_line_break()]
+                                )?;
+                            }
+                            Ok(())
+                        }),
                         operator_token(self.optional()),
                         self.property(),
                     )))]
@@ -71,7 +96,15 @@ fn operator_token(optional: bool) -> &'static str {
 fn layout<'a>(
     node: &AstNode<'a, StaticMemberExpression<'a>>,
     is_member_chain: bool,
+    f: &Formatter<'_, 'a>,
 ) -> StaticMemberLayout {
+    if f.comments()
+        .comments_before_iter(node.property.span.start)
+        .any(|c| f.source_text().is_own_line_comment(c))
+    {
+        return StaticMemberLayout::BreakAfterObject;
+    }
+
     // `a.b.c!` and `a.b?.c`
     // `TSNonNullExpression` is a wrapper node for `!`, and `ChainExpression` is a wrapper node for `?.`,
     // so we need to skip them to find the real parent node.
diff --git a/crates/oxc_formatter/src/write/mod.rs b/crates/oxc_formatter/src/write/mod.rs
@@ -83,6 +83,7 @@ use self::{
     object_like::ObjectLike,
     object_pattern_like::ObjectPatternLike,
     parameters::{ParameterLayout, ParameterList},
+    return_or_throw_statement::FormatAdjacentArgument,
     semicolon::OptionalSemicolon,
     type_parameters::{FormatTSTypeParameters, FormatTSTypeParametersOptions},
     utils::{
@@ -659,7 +660,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ForStatement<'a>> {
 
 impl<'a> FormatWrite<'a> for AstNode<'a, ForInStatement<'a>> {
     fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
-        let comments = f.context().comments().own_line_comments_before(self.body.span().start);
+        let comments = f.context().comments().own_line_comments_before(self.right.span().start);
         write!(
             f,
             [
@@ -683,7 +684,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ForInStatement<'a>> {
 
 impl<'a> FormatWrite<'a> for AstNode<'a, ForOfStatement<'a>> {
     fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
-        let comments = f.context().comments().own_line_comments_before(self.body.span().start);
+        let comments = f.context().comments().own_line_comments_before(self.right.span().start);
 
         let r#await = self.r#await();
         let left = self.left();
@@ -938,12 +939,9 @@ impl<'a> FormatWrite<'a, FormatJsArrowFunctionExpressionOptions>
 
 impl<'a> FormatWrite<'a> for AstNode<'a, YieldExpression<'a>> {
     fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
-        write!(f, "yield")?;
-        if self.delegate() {
-            write!(f, "*")?;
-        }
+        write!(f, ["yield", self.delegate().then_some("*")])?;
         if let Some(argument) = &self.argument() {
-            write!(f, [space(), argument])?;
+            write!(f, [space(), FormatAdjacentArgument(argument)])?;
         }
         Ok(())
     }
diff --git a/crates/oxc_formatter/src/write/return_or_throw_statement.rs b/crates/oxc_formatter/src/write/return_or_throw_statement.rs
@@ -60,7 +60,7 @@ impl<'a> Format<'a> for ReturnAndThrowStatement<'a, '_> {
         write!(f, self.keyword())?;
 
         if let Some(argument) = self.argument() {
-            write!(f, [space(), FormatReturnOrThrowArgument(argument)])?;
+            write!(f, [space(), FormatAdjacentArgument(argument)])?;
         }
 
         let dangling_comments = f.context().comments().comments_before(self.span().end);
@@ -84,9 +84,9 @@ impl<'a> Format<'a> for ReturnAndThrowStatement<'a, '_> {
     }
 }
 
-pub struct FormatReturnOrThrowArgument<'a, 'b>(&'b AstNode<'a, Expression<'a>>);
+pub struct FormatAdjacentArgument<'a, 'b>(pub &'b AstNode<'a, Expression<'a>>);
 
-impl<'a> Format<'a> for FormatReturnOrThrowArgument<'a, '_> {
+impl<'a> Format<'a> for FormatAdjacentArgument<'a, '_> {
     fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
         let argument = self.0;
 
diff --git a/crates/oxc_formatter/src/write/try_statement.rs b/crates/oxc_formatter/src/write/try_statement.rs
@@ -9,7 +9,10 @@ use crate::{
         Formatter,
         prelude::*,
         separated::FormatSeparatedIter,
-        trivia::{FormatLeadingComments, FormatTrailingComments},
+        trivia::{
+            DanglingIndentMode, FormatDanglingComments, FormatLeadingComments,
+            FormatTrailingComments,
+        },
     },
     generated::ast_nodes::{AstNode, AstNodes},
     write,
@@ -40,15 +43,27 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TryStatement<'a>> {
 
 impl<'a> FormatWrite<'a> for AstNode<'a, CatchClause<'a>> {
     fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
-        // `try {} /* comment */ catch (e) {}`
-        // should be formatted like:
-        // `try {} catch (e) { /* comment */ }`
-        //
-        // Comments before the catch clause should be printed in the block statement.
-        // We cache them here to avoid the `params` printing them accidentally.
-        let printed_comments = f.intern(&format_leading_comments(self.span));
-        if let Ok(Some(comments)) = printed_comments {
-            f.context_mut().cache_element(&self.span, comments);
+        let comments = f.context().comments().comments_before(self.span.start);
+        let has_line_comment = comments.iter().any(|comment| {
+            comment.is_line()
+                || f.source_text().is_own_line_comment(comment)
+                || f.source_text().is_end_of_line_comment(comment)
+        });
+
+        if has_line_comment {
+            // `try {} /* comment */\n catch (e) {}`
+            // should be formatted like:
+            // `try {} catch (e) { /* comment */ }`
+            //
+            // Comments before the catch clause should be printed in the block statement.
+            // We cache them here to avoid the `params` printing them accidentally.
+            let printed_comments = f.intern(&FormatLeadingComments::Comments(comments));
+            if let Ok(Some(comments)) = printed_comments {
+                f.context_mut().cache_element(&self.span, comments);
+            }
+        } else if !comments.is_empty() {
+            // otherwise, print them before `catch`
+            write!(f, [FormatTrailingComments::Comments(comments), space()]);
         }
 
         write!(f, ["catch", space(), self.param(), space()])?;
diff --git a/tasks/coverage/snapshots/formatter_babel.snap b/tasks/coverage/snapshots/formatter_babel.snap
@@ -2,9 +2,7 @@ commit: 41d96516
 
 formatter_babel Summary:
 AST Parsed     : 2423/2423 (100.00%)
-Positive Passed: 2420/2423 (99.88%)
-Mismatch: tasks/coverage/babel/packages/babel-parser/test/fixtures/comments/basic/try-statement/input.js
-
+Positive Passed: 2421/2423 (99.92%)
 Mismatch: tasks/coverage/babel/packages/babel-parser/test/fixtures/es2015/class/division/input.js
 
 Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/es2022/top-level-await-unambiguous/module/input.js
diff --git a/tasks/coverage/snapshots/formatter_typescript.snap b/tasks/coverage/snapshots/formatter_typescript.snap
@@ -2,7 +2,7 @@ commit: 261630d6
 
 formatter_typescript Summary:
 AST Parsed     : 8816/8816 (100.00%)
-Positive Passed: 8806/8816 (99.89%)
+Positive Passed: 8808/8816 (99.91%)
 Mismatch: tasks/coverage/typescript/tests/cases/compiler/amdLikeInputDeclarationEmit.ts
 
 Expect to Parse: tasks/coverage/typescript/tests/cases/compiler/arrayFromAsync.ts
@@ -11,10 +11,6 @@ Mismatch: tasks/coverage/typescript/tests/cases/compiler/declarationEmitCastReus
 
 Expect to Parse: tasks/coverage/typescript/tests/cases/compiler/genericTypeAssertions3.ts
 Unexpected token
-Mismatch: tasks/coverage/typescript/tests/cases/compiler/propertyAccessExpressionInnerComments.ts
-
-Mismatch: tasks/coverage/typescript/tests/cases/compiler/tryStatementInternalComments.ts
-
 Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflicts.ts
 Classes may not have a static property named prototypeClasses may not have a static property named prototypeClasses may not have a static property named prototypeClasses may not have a static property named prototypeClasses may not have a static property named prototypeClasses may not have a static property named prototype
 Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/expressions/elementAccess/letIdentifierInElementAccess01.ts

Original file line number	Diff line number	Diff line change
`@@ -146,7 +146,7 @@ impl<'a> SourceText<'a> {`
`146`	`146`
`147`	`147`	`// Should skip the leading comments of the node.`
`148`	`148`	`if let Some(comment) = comments.first()`
`149`		`- && comment.span.end < start`
	`149`	`+ && comment.span.end <= start`
`150`	`150`	`{`
`151`	`151`	`start = comment.span.start;`
`152`	`152`	`}`
Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ impl<'a> Format<'a> for ReturnAndThrowStatement<'a, '_> {`
`60`	`60`	`write!(f, self.keyword())?;`
`61`	`61`
`62`	`62`	`if let Some(argument) = self.argument() {`
`63`		`- write!(f, [space(), FormatReturnOrThrowArgument(argument)])?;`
	`63`	`+ write!(f, [space(), FormatAdjacentArgument(argument)])?;`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`let dangling_comments = f.context().comments().comments_before(self.span().end);`
`@@ -84,9 +84,9 @@ impl<'a> Format<'a> for ReturnAndThrowStatement<'a, '_> {`
`84`	`84`	`}`
`85`	`85`	`}`
`86`	`86`
`87`		`-pub struct FormatReturnOrThrowArgument<'a, 'b>(&'b AstNode<'a, Expression<'a>>);`
	`87`	`+pub struct FormatAdjacentArgument<'a, 'b>(pub &'b AstNode<'a, Expression<'a>>);`
`88`	`88`
`89`		`-impl<'a> Format<'a> for FormatReturnOrThrowArgument<'a, '_> {`
	`89`	`+impl<'a> Format<'a> for FormatAdjacentArgument<'a, '_> {`
`90`	`90`	`fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {`
`91`	`91`	`let argument = self.0;`
`92`	`92`