diff --git a/crates/oxc_formatter/src/formatter/comments.rs b/crates/oxc_formatter/src/formatter/comments.rs index 6e7d8631626c3..2efd53df81760 100644 --- a/crates/oxc_formatter/src/formatter/comments.rs +++ b/crates/oxc_formatter/src/formatter/comments.rs @@ -451,6 +451,17 @@ impl<'a> Comments<'a> { ) } + /// Checks if there is a type cast comment in the given range, + /// searching all comments regardless of print state. + pub fn has_type_cast_comment_in_range(&self, start: u32, end: u32) -> bool { + self.inner.iter().skip_while(|c| c.span.end < start).take_while(|c| c.span.end <= end).any( + |comment| { + self.source_text.next_non_whitespace_byte_is(comment.span.end, b'(') + && self.is_type_cast_comment(comment) + }, + ) + } + /// Marks the given span as a type cast node. pub fn mark_as_type_cast_node(&mut self, node: &impl GetSpan) { self.type_cast_node_span = node.span(); diff --git a/crates/oxc_formatter/src/print/call_like_expression/arguments.rs b/crates/oxc_formatter/src/print/call_like_expression/arguments.rs index f33ea30dff9b2..2760103201d32 100644 --- a/crates/oxc_formatter/src/print/call_like_expression/arguments.rs +++ b/crates/oxc_formatter/src/print/call_like_expression/arguments.rs @@ -626,10 +626,24 @@ fn can_group_arrow_function_expression_argument( Expression::ArrowFunctionExpression(inner_arrow_function) => { can_group_arrow_function_expression_argument(inner_arrow_function, true, f) } + // In Prettier's Babel AST, a JSDoc type cast like `/** @type {X} */ (expr)` preserves + // the `ParenthesizedExpression` wrapper, so `arg.body` is not a CallExpression and + // `couldExpandArg` naturally returns false. In oxc's AST the parens are stripped, so we + // must explicitly check for type cast comments to prevent incorrect grouping. + // https://github.com/prettier/prettier/blob/812a4d0071270f61a7aa549d625b618be7e09d71/src/language-js/print/call-arguments.js#L232-L234 Expression::ChainExpression(chain) => { - matches!(chain.expression, ChainElement::CallExpression(_)) && !is_arrow_recursion + matches!(chain.expression, ChainElement::CallExpression(_)) + && !is_arrow_recursion + && !f + .comments() + .has_type_cast_comment_in_range(arrow_function.span.start, expr.span().start) + } + Expression::CallExpression(_) | Expression::ConditionalExpression(_) => { + !is_arrow_recursion + && !f + .comments() + .has_type_cast_comment_in_range(arrow_function.span.start, expr.span().start) } - Expression::CallExpression(_) | Expression::ConditionalExpression(_) => !is_arrow_recursion, _ => false, }) } diff --git a/crates/oxc_formatter/tests/fixtures/js/comments/type-cast-arrow.js b/crates/oxc_formatter/tests/fixtures/js/comments/type-cast-arrow.js new file mode 100644 index 0000000000000..c2283c75d8aba --- /dev/null +++ b/crates/oxc_formatter/tests/fixtures/js/comments/type-cast-arrow.js @@ -0,0 +1,21 @@ +items.map((child) => /** @type {SomeType} */ (visit(child))); + +transform_body(first_argument, second_argument, (node) => /** @type {Node} */ (context.visit(node))); + +// #20180 Arrow function body breaks after => when JSDoc type cast is in body +const longer_variable = items_with_longer_name.map((child) => /** @type {SomeTypeThatIsLonger} */ (visit(child))); + +const body = transform_body(state.analysis.instance_body, b.id("$.run"), (node) => /** @type {Node} */ (context.visit(node))); + +// Edge case: very long type cast that doesn't fit even after expanding arguments +const x = items_with_longer_name.map((child) => /** @type {SomeVeryVeryVeryVeryVeryVeryVeryLongType} */ (visit(child))); + +// No false positive: type cast earlier in file should not affect unrelated arrow +const a = /** @type {X} */ (foo()); +const result = items_with_longer_name.map((child) => bar_without_typecast(child)); + +// @satisfies type cast +const z = items_with_longer_name.map((child) => /** @satisfies {SomeTypeThatIsLonger} */ (visit(child))); + +// Optional chaining with type cast +const w = items_with_longer_name.map((child) => /** @type {SomeTypeThatIsLonger} */ (child?.visit())); diff --git a/crates/oxc_formatter/tests/fixtures/js/comments/type-cast-arrow.js.snap b/crates/oxc_formatter/tests/fixtures/js/comments/type-cast-arrow.js.snap new file mode 100644 index 0000000000000..7ceced9a86041 --- /dev/null +++ b/crates/oxc_formatter/tests/fixtures/js/comments/type-cast-arrow.js.snap @@ -0,0 +1,113 @@ +--- +source: crates/oxc_formatter/tests/fixtures/mod.rs +--- +==================== Input ==================== +items.map((child) => /** @type {SomeType} */ (visit(child))); + +transform_body(first_argument, second_argument, (node) => /** @type {Node} */ (context.visit(node))); + +// #20180 Arrow function body breaks after => when JSDoc type cast is in body +const longer_variable = items_with_longer_name.map((child) => /** @type {SomeTypeThatIsLonger} */ (visit(child))); + +const body = transform_body(state.analysis.instance_body, b.id("$.run"), (node) => /** @type {Node} */ (context.visit(node))); + +// Edge case: very long type cast that doesn't fit even after expanding arguments +const x = items_with_longer_name.map((child) => /** @type {SomeVeryVeryVeryVeryVeryVeryVeryLongType} */ (visit(child))); + +// No false positive: type cast earlier in file should not affect unrelated arrow +const a = /** @type {X} */ (foo()); +const result = items_with_longer_name.map((child) => bar_without_typecast(child)); + +// @satisfies type cast +const z = items_with_longer_name.map((child) => /** @satisfies {SomeTypeThatIsLonger} */ (visit(child))); + +// Optional chaining with type cast +const w = items_with_longer_name.map((child) => /** @type {SomeTypeThatIsLonger} */ (child?.visit())); + +==================== Output ==================== +------------------ +{ printWidth: 80 } +------------------ +items.map((child) => /** @type {SomeType} */ (visit(child))); + +transform_body( + first_argument, + second_argument, + (node) => /** @type {Node} */ (context.visit(node)), +); + +// #20180 Arrow function body breaks after => when JSDoc type cast is in body +const longer_variable = items_with_longer_name.map( + (child) => /** @type {SomeTypeThatIsLonger} */ (visit(child)), +); + +const body = transform_body( + state.analysis.instance_body, + b.id("$.run"), + (node) => /** @type {Node} */ (context.visit(node)), +); + +// Edge case: very long type cast that doesn't fit even after expanding arguments +const x = items_with_longer_name.map( + (child) => + /** @type {SomeVeryVeryVeryVeryVeryVeryVeryLongType} */ (visit(child)), +); + +// No false positive: type cast earlier in file should not affect unrelated arrow +const a = /** @type {X} */ (foo()); +const result = items_with_longer_name.map((child) => + bar_without_typecast(child), +); + +// @satisfies type cast +const z = items_with_longer_name.map( + (child) => /** @satisfies {SomeTypeThatIsLonger} */ (visit(child)), +); + +// Optional chaining with type cast +const w = items_with_longer_name.map( + (child) => /** @type {SomeTypeThatIsLonger} */ (child?.visit()), +); + +------------------- +{ printWidth: 100 } +------------------- +items.map((child) => /** @type {SomeType} */ (visit(child))); + +transform_body( + first_argument, + second_argument, + (node) => /** @type {Node} */ (context.visit(node)), +); + +// #20180 Arrow function body breaks after => when JSDoc type cast is in body +const longer_variable = items_with_longer_name.map( + (child) => /** @type {SomeTypeThatIsLonger} */ (visit(child)), +); + +const body = transform_body( + state.analysis.instance_body, + b.id("$.run"), + (node) => /** @type {Node} */ (context.visit(node)), +); + +// Edge case: very long type cast that doesn't fit even after expanding arguments +const x = items_with_longer_name.map( + (child) => /** @type {SomeVeryVeryVeryVeryVeryVeryVeryLongType} */ (visit(child)), +); + +// No false positive: type cast earlier in file should not affect unrelated arrow +const a = /** @type {X} */ (foo()); +const result = items_with_longer_name.map((child) => bar_without_typecast(child)); + +// @satisfies type cast +const z = items_with_longer_name.map( + (child) => /** @satisfies {SomeTypeThatIsLonger} */ (visit(child)), +); + +// Optional chaining with type cast +const w = items_with_longer_name.map( + (child) => /** @type {SomeTypeThatIsLonger} */ (child?.visit()), +); + +===================== End =====================