From d2a130e14c42ecbaa8e1d9c53f2edb1333ace834 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Thu, 27 Nov 2025 17:59:18 +0800 Subject: [PATCH 1/4] fix(formatter): incorrect printing of union types with comments --- .../src/ast_nodes/generated/format.rs | 1 - .../oxc_formatter/src/formatter/comments.rs | 10 ++-- .../src/utils/assignment_like.rs | 5 +- crates/oxc_formatter/src/write/union_type.rs | 49 +++++++++++++------ .../tests/fixtures/ts/union/issue-16176.ts | 12 +++++ .../fixtures/ts/union/issue-16176.ts.snap | 49 +++++++++++++++++++ .../src/generators/formatter/format.rs | 21 ++++---- .../snapshots/prettier.ts.snap.md | 7 ++- 8 files changed, 118 insertions(+), 36 deletions(-) create mode 100644 crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts create mode 100644 crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts.snap diff --git a/crates/oxc_formatter/src/ast_nodes/generated/format.rs b/crates/oxc_formatter/src/ast_nodes/generated/format.rs index 2147673b6f475..458091fe1165e 100644 --- a/crates/oxc_formatter/src/ast_nodes/generated/format.rs +++ b/crates/oxc_formatter/src/ast_nodes/generated/format.rs @@ -4377,7 +4377,6 @@ impl<'a> Format<'a> for AstNode<'a, TSUnionType<'a>> { if !is_suppressed && format_type_cast_comment_node(self, false, f) { return; } - self.format_leading_comments(f); let needs_parentheses = self.needs_parentheses(f); if needs_parentheses { "(".fmt(f); diff --git a/crates/oxc_formatter/src/formatter/comments.rs b/crates/oxc_formatter/src/formatter/comments.rs index 4428007fd49ec..326ce794a56a3 100644 --- a/crates/oxc_formatter/src/formatter/comments.rs +++ b/crates/oxc_formatter/src/formatter/comments.rs @@ -369,10 +369,12 @@ impl<'a> Comments<'a> { /// Checks if the node has a suppression comment (prettier-ignore). pub fn is_suppressed(&self, start: u32) -> bool { - self.comments_before(start).iter().any(|comment| { - // TODO: Consider using `oxc-formatter-ignore` instead of `prettier-ignore` - self.source_text.text_for(&comment.content_span()).trim() == "prettier-ignore" - }) + self.comments_before(start).iter().any(|comment| self.is_suppression_comment(comment)) + } + + pub fn is_suppression_comment(&self, comment: &Comment) -> bool { + // TODO: Consider using `oxfmt-ignore` instead of `prettier-ignore` + self.source_text.text_for(&comment.content_span()).trim() == "prettier-ignore" } /// Checks if a comment is a type cast comment containing `@type` or `@satisfies`. diff --git a/crates/oxc_formatter/src/utils/assignment_like.rs b/crates/oxc_formatter/src/utils/assignment_like.rs index 55a028675ce2e..4ca502e16f6cc 100644 --- a/crates/oxc_formatter/src/utils/assignment_like.rs +++ b/crates/oxc_formatter/src/utils/assignment_like.rs @@ -554,13 +554,14 @@ impl<'a> AssignmentLike<'a, '_> { _ => false, } }; - is_generic(&conditional_type.check_type) || is_generic(&conditional_type.extends_type) || comments.has_comment_before(decl.type_annotation.span().start) } + // `TSUnionType` has its own indentation logic + TSType::TSUnionType(_) => false, _ => { - // Check for leading comments on any other type + // Check for leading comments on a,ny other type comments.has_comment_before(decl.type_annotation.span().start) } } diff --git a/crates/oxc_formatter/src/write/union_type.rs b/crates/oxc_formatter/src/write/union_type.rs index 33ca1b6a7a27b..ed2523d722909 100644 --- a/crates/oxc_formatter/src/write/union_type.rs +++ b/crates/oxc_formatter/src/write/union_type.rs @@ -52,29 +52,33 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSUnionType<'a>> { union_type_at_top = parent; } - let should_indent = { + let should_indent = !has_leading_comments && { let parent = union_type_at_top.parent; // These parents have indent for their content, so we don't need to indent here - !match parent { - AstNodes::TSTypeAliasDeclaration(_) => has_leading_comments, + match parent { + AstNodes::TSTypeAliasDeclaration(alias) => { + !f.comments().printed_comments().last().is_some_and(|comment| { + comment.span.start + > alias.type_parameters().map_or(alias.id.span.end, |tp| tp.span.end) + && f.comments().is_end_of_line_comment(comment) + }) + } AstNodes::TSTypeAssertion(_) | AstNodes::TSTupleType(_) - | AstNodes::TSTypeParameterInstantiation(_) => true, - _ => false, + | AstNodes::TSTypeParameterInstantiation(_) => false, + _ => true, } }; let types = format_with(|f| { - let suppressed_node_span = if f.comments().is_suppressed(self.span.start) { - self.types.first().unwrap().span() - } else { - Span::default() - }; + let is_suppressed = leading_comments + .iter() + .rev() + .any(|comment| f.comments().is_suppression_comment(comment)); - if has_leading_comments { - write!(f, FormatLeadingComments::Comments(leading_comments)); - } + let suppressed_node_span = + if is_suppressed { self.types.first().unwrap().span() } else { Span::default() }; let leading_soft_line_break_or_space = should_indent && !has_leading_comments; @@ -123,7 +127,24 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSUnionType<'a>> { } }); - write!(f, [group(&content)]); + if has_leading_comments { + let has_own_line_leading_comment = union_type_at_top.types.len() > 1 + && leading_comments.iter().any(|comment| f.comments().is_own_line_comment(comment)); + let is_end_of_line_comment = leading_comments + .last() + .is_some_and(|comment| f.comments().is_end_of_line_comment(comment)); + write!( + f, + [group(&indent(&format_args!( + has_own_line_leading_comment.then(soft_line_break), + FormatLeadingComments::Comments(leading_comments), + (!is_end_of_line_comment).then(soft_line_break), + group(&content) + )))] + ); + } else { + write!(f, [group(&content)]); + } } } diff --git a/crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts b/crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts new file mode 100644 index 0000000000000..23fe6c52c78a5 --- /dev/null +++ b/crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts @@ -0,0 +1,12 @@ +export default class TestUnionTypeAnnotation1 { + private prop!: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; + + private accessor prop2!: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; +} + +export interface TestUnionTypeAnnotation2 { + property: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; +} diff --git a/crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts.snap b/crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts.snap new file mode 100644 index 0000000000000..d8c54e2cd4d5e --- /dev/null +++ b/crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts.snap @@ -0,0 +1,49 @@ +--- +source: crates/oxc_formatter/tests/fixtures/mod.rs +--- +==================== Input ==================== +export default class TestUnionTypeAnnotation1 { + private prop!: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; + + private accessor prop2!: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; +} + +export interface TestUnionTypeAnnotation2 { + property: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; +} + +==================== Output ==================== +------------------ +{ printWidth: 80 } +------------------ +export default class TestUnionTypeAnnotation1 { + private prop!: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; + + private accessor prop2: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; +} + +export interface TestUnionTypeAnnotation2 { + property: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; +} + +------------------- +{ printWidth: 100 } +------------------- +export default class TestUnionTypeAnnotation1 { + private prop!: /* comment */ LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; + + private accessor prop2: /* comment */ + LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; +} + +export interface TestUnionTypeAnnotation2 { + property: /* comment */ LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[]; +} + +===================== End ===================== diff --git a/tasks/ast_tools/src/generators/formatter/format.rs b/tasks/ast_tools/src/generators/formatter/format.rs index 375419c47bf2a..c319077da4755 100644 --- a/tasks/ast_tools/src/generators/formatter/format.rs +++ b/tasks/ast_tools/src/generators/formatter/format.rs @@ -31,6 +31,8 @@ const AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST: &[&str] = &[ "TemplateElement", ]; +const AST_NODE_WITHOUT_PRINTING_LEADING_COMMENTS_LIST: &[&str] = &["TSUnionType"]; + const AST_NODE_NEEDS_PARENTHESES: &[&str] = &[ "TSTypeAssertion", "TSInferType", @@ -109,22 +111,19 @@ fn generate_struct_implementation( let struct_name = struct_def.name(); let do_not_print_comment = AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST.contains(&struct_name); + let do_not_print_leading_comment = do_not_print_comment + || AST_NODE_WITHOUT_PRINTING_LEADING_COMMENTS_LIST.contains(&struct_name); - let leading_comments = if do_not_print_comment { - quote! {} - } else { + let leading_comments = (!do_not_print_leading_comment).then(|| { quote! { self.format_leading_comments(f); } - }; - - let trailing_comments = if do_not_print_comment { - quote! {} - } else { + }); + let trailing_comments = (!do_not_print_comment).then(|| { quote! { self.format_trailing_comments(f); } - }; + }); let needs_parentheses = parenthesis_type_ids.contains(&struct_def.id); @@ -171,7 +170,7 @@ fn generate_struct_implementation( let write_implementation = if suppressed_check.is_none() { write_call - } else if trailing_comments.is_empty() { + } else if trailing_comments.is_none() { quote! { if is_suppressed { self.format_leading_comments(f); @@ -214,7 +213,7 @@ fn generate_struct_implementation( } }); - if needs_parentheses_before.is_empty() && trailing_comments.is_empty() { + if needs_parentheses_before.is_empty() && trailing_comments.is_none() { quote! { #suppressed_check #type_cast_comment_formatting diff --git a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md index 55a7a75a35821..6ee659f3a9c82 100644 --- a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md @@ -1,4 +1,4 @@ -ts compatibility: 571/602 (94.85%) +ts compatibility: 572/602 (95.02%) # Failed @@ -32,6 +32,5 @@ ts compatibility: 571/602 (94.85%) | typescript/object-multiline/multiline.ts | 💥✨ | 23.21% | | typescript/property-signature/consistent-with-flow/comments.ts | 💥 | 80.00% | | typescript/union/union-parens.ts | 💥 | 92.59% | -| typescript/union/comments/18106.ts | 💥 | 90.48% | -| typescript/union/consistent-with-flow/comment.ts | 💥 | 82.61% | -| typescript/union/single-type/single-type.ts | 💥 | 0.00% | +| typescript/union/comments/18106.ts | 💥 | 92.68% | +| typescript/union/single-type/single-type.ts | 💥 | 66.67% | From 4dd0e4877196e3cd21e2ed9a2aa65785b72bcb3d Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 27 Nov 2025 19:21:15 +0900 Subject: [PATCH 2/4] Update snapshots --- tasks/prettier_conformance/snapshots/prettier.js.snap.md | 6 +++--- tasks/prettier_conformance/snapshots/prettier.ts.snap.md | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tasks/prettier_conformance/snapshots/prettier.js.snap.md b/tasks/prettier_conformance/snapshots/prettier.js.snap.md index 13f2faa5bdebf..9fab95118edf7 100644 --- a/tasks/prettier_conformance/snapshots/prettier.js.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.js.snap.md @@ -1,4 +1,4 @@ -js compatibility: 729/759 (96.05%) +js compatibility: 728/758 (96.04%) # Failed @@ -12,11 +12,11 @@ js compatibility: 729/759 (96.05%) | js/comments/if.js | 💥💥 | 74.83% | | js/comments/return-statement.js | 💥💥 | 98.28% | | js/explicit-resource-management/for-await-using-of-comments.js | 💥 | 0.00% | -| js/explicit-resource-management/valid-await-using-comments.js | 💥 | 68.57% | +| js/explicit-resource-management/valid-await-using-comments.js | 💥 | 66.67% | | js/for/9812-unstable.js | 💥 | 45.45% | | js/for/9812.js | 💥 | 82.83% | | js/for/for-in-with-initializer.js | 💥 | 37.50% | -| js/for/parentheses.js | 💥 | 97.96% | +| js/for/parentheses.js | 💥 | 96.00% | | js/identifier/for-of/let.js | 💥 | 92.31% | | js/identifier/parentheses/let.js | 💥💥 | 82.27% | | js/if/comment-between-condition-and-body.js | 💥 | 65.79% | diff --git a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md index 6ee659f3a9c82..8aca3b3aba3b9 100644 --- a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md @@ -1,4 +1,4 @@ -ts compatibility: 572/602 (95.02%) +ts compatibility: 572/601 (95.17%) # Failed @@ -21,7 +21,6 @@ ts compatibility: 572/602 (95.02%) | typescript/comments/mapped_types.ts | 💥 | 96.77% | | typescript/comments/method_types.ts | 💥 | 82.05% | | typescript/decorators-ts/angular.ts | 💥 | 87.50% | -| typescript/interface2/comments-ts-only/18278.ts | 💥 | 95.65% | | typescript/intersection/intersection-parens.ts | 💥💥 | 86.17% | | typescript/intersection/consistent-with-flow/intersection-parens.ts | 💥 | 69.77% | | typescript/last-argument-expansion/decorated-function.tsx | 💥 | 29.06% | From 7fbb5a2da59c2e89462168aadb9a8ab01a5b2f82 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura <6259812+leaysgur@users.noreply.github.com> Date: Thu, 27 Nov 2025 19:29:09 +0900 Subject: [PATCH 3/4] Update crates/oxc_formatter/src/utils/assignment_like.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yuji Sugiura <6259812+leaysgur@users.noreply.github.com> --- crates/oxc_formatter/src/utils/assignment_like.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_formatter/src/utils/assignment_like.rs b/crates/oxc_formatter/src/utils/assignment_like.rs index 4ca502e16f6cc..644a3dd8ed8e5 100644 --- a/crates/oxc_formatter/src/utils/assignment_like.rs +++ b/crates/oxc_formatter/src/utils/assignment_like.rs @@ -561,7 +561,7 @@ impl<'a> AssignmentLike<'a, '_> { // `TSUnionType` has its own indentation logic TSType::TSUnionType(_) => false, _ => { - // Check for leading comments on a,ny other type + // Check for leading comments on any other type comments.has_comment_before(decl.type_annotation.span().start) } } From c2eb67752ba1ec80d55d3200f25c524fb0e274fb Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 27 Nov 2025 20:51:38 +0900 Subject: [PATCH 4/4] Update snapshots --- tasks/coverage/snapshots/formatter_typescript.snap | 4 +++- tasks/prettier_conformance/snapshots/prettier.js.snap.md | 6 +++--- tasks/prettier_conformance/snapshots/prettier.ts.snap.md | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tasks/coverage/snapshots/formatter_typescript.snap b/tasks/coverage/snapshots/formatter_typescript.snap index 7540025f47880..416fb002c6ca8 100644 --- a/tasks/coverage/snapshots/formatter_typescript.snap +++ b/tasks/coverage/snapshots/formatter_typescript.snap @@ -2,13 +2,15 @@ commit: 669c25c0 formatter_typescript Summary: AST Parsed : 9822/9822 (100.00%) -Positive Passed: 9815/9822 (99.93%) +Positive Passed: 9814/9822 (99.92%) Mismatch: tasks/coverage/typescript/tests/cases/compiler/amdLikeInputDeclarationEmit.ts Expect to Parse: tasks/coverage/typescript/tests/cases/compiler/genericTypeAssertions3.ts Unexpected token Expect to Parse: tasks/coverage/typescript/tests/cases/compiler/typeAssertionToGenericFunctionType.ts Unexpected token +Mismatch: tasks/coverage/typescript/tests/cases/compiler/unionTypeWithLeadingOperator.ts + Mismatch: tasks/coverage/typescript/tests/cases/conformance/expressions/contextualTyping/parenthesizedContexualTyping2.ts Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/expressions/elementAccess/letIdentifierInElementAccess01.ts diff --git a/tasks/prettier_conformance/snapshots/prettier.js.snap.md b/tasks/prettier_conformance/snapshots/prettier.js.snap.md index 9fab95118edf7..13f2faa5bdebf 100644 --- a/tasks/prettier_conformance/snapshots/prettier.js.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.js.snap.md @@ -1,4 +1,4 @@ -js compatibility: 728/758 (96.04%) +js compatibility: 729/759 (96.05%) # Failed @@ -12,11 +12,11 @@ js compatibility: 728/758 (96.04%) | js/comments/if.js | 💥💥 | 74.83% | | js/comments/return-statement.js | 💥💥 | 98.28% | | js/explicit-resource-management/for-await-using-of-comments.js | 💥 | 0.00% | -| js/explicit-resource-management/valid-await-using-comments.js | 💥 | 66.67% | +| js/explicit-resource-management/valid-await-using-comments.js | 💥 | 68.57% | | js/for/9812-unstable.js | 💥 | 45.45% | | js/for/9812.js | 💥 | 82.83% | | js/for/for-in-with-initializer.js | 💥 | 37.50% | -| js/for/parentheses.js | 💥 | 96.00% | +| js/for/parentheses.js | 💥 | 97.96% | | js/identifier/for-of/let.js | 💥 | 92.31% | | js/identifier/parentheses/let.js | 💥💥 | 82.27% | | js/if/comment-between-condition-and-body.js | 💥 | 65.79% | diff --git a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md index 8aca3b3aba3b9..6ee659f3a9c82 100644 --- a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md @@ -1,4 +1,4 @@ -ts compatibility: 572/601 (95.17%) +ts compatibility: 572/602 (95.02%) # Failed @@ -21,6 +21,7 @@ ts compatibility: 572/601 (95.17%) | typescript/comments/mapped_types.ts | 💥 | 96.77% | | typescript/comments/method_types.ts | 💥 | 82.05% | | typescript/decorators-ts/angular.ts | 💥 | 87.50% | +| typescript/interface2/comments-ts-only/18278.ts | 💥 | 95.65% | | typescript/intersection/intersection-parens.ts | 💥💥 | 86.17% | | typescript/intersection/consistent-with-flow/intersection-parens.ts | 💥 | 69.77% | | typescript/last-argument-expansion/decorated-function.tsx | 💥 | 29.06% |