fix(formatter): skip test-call layout when arguments have comments (#27119)

Fixes #24960, closes #26926.

crates/oxc_formatter/DIVERGENCES.md

@@ -705,3 +705,37 @@ Prettier keeps the Markdown hard break in `/**` only, following jsdoc3.
 Tools disagree on `/***`: TypeScript (`isJSDocLikeText`, so editor hovers) treats it as JSDoc, jsdoc3 ignores it.
 Formatting it as JSDoc does not break jsdoc3, which ignores it either way.
 Type cast comments already follow the rule, in Prettier too.
+
+## test-call-comment-order
+
+- Why: invariant (prettier/prettier#20029, prettier/prettier#20043)
+- Pin: `tests/fixtures/js/calls/test-call-comment-order.js`
+
+```js
+// input
+test("x", () => {
+  run();
+}, // first
+// second
+60000);
+
+// ours
+test(
+  "x",
+  () => {
+    run();
+  }, // first
+  // second
+  60000,
+);
+
+// prettier
+test("x", () => {
+  run();
+}, // second // first
+60000);
+```
+
+Prettier's compact test call layout merges and reverses the comments, and is not a fixpoint.
+We fall back to the regular argument layout when comments sit around the arguments, following prettier/prettier#20043.
+A same-line block comment also falls back, the same output as the other special call layouts (React hook, `require`).

crates/oxc_formatter/src/print/call_like_expression/arguments.rs

@@ -73,7 +73,7 @@ impl<'a> Format<'a, JsFormatContext<'a>> for AstNode<'a, ArenaVec<'a, Argument<'
                                     | Argument::TaggedTemplateExpression(_)
                             )
                         ))
-                        && is_test_call_expression(call))
+                        && is_test_call_expression(call, f.comments()))
             })
             || is_multiline_template_only_args(self, f.source_text())
             || is_graphql_call_with_single_template_arg(self, call_expression)

crates/oxc_formatter/src/print/call_like_expression/mod.rs

@@ -55,7 +55,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, CallExpression<'a>> {
                 Expression::StaticMemberExpression(_) | Expression::ComputedMemberExpression(_)
             )
             && !is_simple_module_import(self.arguments(), f.comments())
-            && !is_test_call_expression(self)
+            && !is_test_call_expression(self, f.comments())
         {
             MemberChain::from_call_expression(self, f).fmt(f);
         } else {

crates/oxc_formatter/src/print/parameters.rs

@@ -67,7 +67,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, FormalParameters<'a>> {
                 // Check if parent is a test call, but exclude angular test wrappers
                 // like `inject`, `async`, `fakeAsync`, `waitForAsync` because their
                 // inner function parameters should still be allowed to break.
-                is_test_call_expression(call) && !is_angular_test_wrapper(call)
+                is_test_call_expression(call, f.comments()) && !is_angular_test_wrapper(call)
             } else {
                 false
             }

crates/oxc_formatter/src/print/type_parameters.rs

@@ -147,7 +147,7 @@ impl<'a> Format<'a, JsFormatContext<'a>> for FormatTSTypeParameters<'a, '_> {
             write!(
                 f,
                 [group(&format_args!("<", format_with(|f| {
-                    if matches!(self.decl.grand_parent(), AstNodes::CallExpression(call) if is_test_call_expression(call))
+                    if matches!(self.decl.grand_parent(), AstNodes::CallExpression(call) if is_test_call_expression(call, f.comments()))
                     {
                         f.join_nodes_with_space().entries_with_trailing_separator(params, ",", TrailingSeparator::Omit);
                     } else {

crates/oxc_formatter/src/utils/call_expression.rs

@@ -1,6 +1,12 @@
-use oxc_ast::ast::*;
+use std::iter;
 
-use crate::ast_nodes::{AstNode, AstNodes};
+use oxc_ast::ast::*;
+use oxc_span::GetSpan;
+
+use crate::{
+    ast_nodes::{AstNode, AstNodes},
+    formatter::Comments,
+};
 
 /// This is a specialized function that checks if the current [call expression]
 /// resembles a call expression usually used by a testing frameworks.
@@ -17,13 +23,26 @@ use crate::ast_nodes::{AstNode, AstNodes};
 /// 4. The third argument, if present, has to be a number literal
 /// 5. The second argument has to be an [arrow function expression] or [function expression]
 /// 6. Both function must have zero or one parameters
+/// 7. No comments around the [arguments]
 ///
 /// [call expression]: CallExpression
 /// [callee]: Expression
 /// [arguments]: CallExpression::arguments
 /// [arrow function expression]: ArrowFunctionExpression
 /// [function expression]: Function
-pub fn is_test_call_expression(call: &AstNode<CallExpression<'_>>) -> bool {
+pub fn is_test_call_expression(call: &AstNode<CallExpression<'_>>, comments: &Comments) -> bool {
+    is_test_call_shape(call, comments) && !has_comment_around_arguments(call, comments)
+}
+
+/// The compact test call layout joins arguments with spaces, which breaks comment order.
+fn has_comment_around_arguments(call: &CallExpression, comments: &Comments) -> bool {
+    let first = call.type_arguments.as_ref().map_or(call.callee.span().end, |t| t.span.end);
+    let starts = iter::once(first).chain(call.arguments.iter().map(|a| a.span().end));
+    let ends = call.arguments.iter().map(|a| a.span().start).chain(iter::once(call.span.end));
+    starts.zip(ends).any(|(start, end)| comments.has_any_comment_in_range(start, end))
+}
+
+fn is_test_call_shape(call: &AstNode<CallExpression<'_>>, comments: &Comments) -> bool {
     if call.optional {
         return false;
     }
@@ -37,7 +56,7 @@ pub fn is_test_call_expression(call: &AstNode<CallExpression<'_>>) -> bool {
         (Some(argument), None, None) if arguments.len() == 1 => {
             if is_angular_test_wrapper(call) && {
                 if let AstNodes::CallExpression(call) = call.parent() {
-                    is_test_call_expression(call)
+                    is_test_call_expression(call, comments)
                 } else {
                     false
                 }

crates/oxc_formatter/tests/fixtures/js/calls/test-call-comment-order.js

@@ -0,0 +1,41 @@
+test("comments after callback", () => {
+  run();
+}, // first
+// second
+// third
+60000);
+
+it("comments before timeout", function (done) {
+  done();
+},
+// first
+// second
+60000);
+
+test("comments before callback", // first
+// second
+() => {
+  run();
+});
+
+describe.only(
+// description
+"suite", () => {
+  run();
+});
+
+test("trailing", () => {
+  run();
+}, 15000 // first
+// second
+);
+
+test("own-line block", () => {
+  run();
+},
+/* c */
+15000);
+
+it("params", /* c */ function (aaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccc) {
+  run();
+});

crates/oxc_formatter/tests/fixtures/js/calls/test-call-comment-order.js.snap

@@ -0,0 +1,180 @@
+---
+source: crates/oxc_formatter/tests/fixtures/mod.rs
+---
+==================== Input ====================
+test("comments after callback", () => {
+  run();
+}, // first
+// second
+// third
+60000);
+
+it("comments before timeout", function (done) {
+  done();
+},
+// first
+// second
+60000);
+
+test("comments before callback", // first
+// second
+() => {
+  run();
+});
+
+describe.only(
+// description
+"suite", () => {
+  run();
+});
+
+test("trailing", () => {
+  run();
+}, 15000 // first
+// second
+);
+
+test("own-line block", () => {
+  run();
+},
+/* c */
+15000);
+
+it("params", /* c */ function (aaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccc) {
+  run();
+});
+
+==================== Output ====================
+------------------
+{ printWidth: 80 }
+------------------
+test(
+  "comments after callback",
+  () => {
+    run();
+  }, // first
+  // second
+  // third
+  60000,
+);
+
+it(
+  "comments before timeout",
+  function (done) {
+    done();
+  },
+  // first
+  // second
+  60000,
+);
+
+test(
+  "comments before callback", // first
+  // second
+  () => {
+    run();
+  },
+);
+
+describe.only(
+  // description
+  "suite",
+  () => {
+    run();
+  },
+);
+
+test(
+  "trailing",
+  () => {
+    run();
+  },
+  15000, // first
+  // second
+);
+
+test(
+  "own-line block",
+  () => {
+    run();
+  },
+  /* c */
+  15000,
+);
+
+it(
+  "params",
+  /* c */ function (
+    aaaaaaaaaaaaaaaaaaaa,
+    bbbbbbbbbbbbbbbbbbbbbbbb,
+    ccccccccccccccc,
+  ) {
+    run();
+  },
+);
+
+-------------------
+{ printWidth: 100 }
+-------------------
+test(
+  "comments after callback",
+  () => {
+    run();
+  }, // first
+  // second
+  // third
+  60000,
+);
+
+it(
+  "comments before timeout",
+  function (done) {
+    done();
+  },
+  // first
+  // second
+  60000,
+);
+
+test(
+  "comments before callback", // first
+  // second
+  () => {
+    run();
+  },
+);
+
+describe.only(
+  // description
+  "suite",
+  () => {
+    run();
+  },
+);
+
+test(
+  "trailing",
+  () => {
+    run();
+  },
+  15000, // first
+  // second
+);
+
+test(
+  "own-line block",
+  () => {
+    run();
+  },
+  /* c */
+  15000,
+);
+
+it(
+  "params",
+  /* c */ function (aaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccc) {
+    run();
+  },
+);
+
+===================== End =====================

crates/oxc_formatter/tests/fixtures/ts/arguments/test-call-type-arguments-comment.ts

@@ -0,0 +1,3 @@
+test<T /* c */>("type arguments", () => {
+  run();
+});

crates/oxc_formatter/tests/fixtures/ts/arguments/test-call-type-arguments-comment.ts.snap

@@ -0,0 +1,24 @@
+---
+source: crates/oxc_formatter/tests/fixtures/mod.rs
+---
+==================== Input ====================
+test<T /* c */>("type arguments", () => {
+  run();
+});
+
+==================== Output ====================
+------------------
+{ printWidth: 80 }
+------------------
+test<T /* c */>("type arguments", () => {
+  run();
+});
+
+-------------------
+{ printWidth: 100 }
+-------------------
+test<T /* c */>("type arguments", () => {
+  run();
+});
+
+===================== End =====================