Skip to content

Commit 9097167

Browse files
DunqingleaysgurCopilot
authored
fix(formatter): incorrect printing of union types with comments (#16205)
close: #16176 --------- Signed-off-by: Yuji Sugiura <[email protected]> Co-authored-by: Yuji Sugiura <[email protected]> Co-authored-by: Yuji Sugiura <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 79b78b3 commit 9097167

File tree

9 files changed

+120
-36
lines changed

9 files changed

+120
-36
lines changed

‎crates/oxc_formatter/src/ast_nodes/generated/format.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4377,7 +4377,6 @@ impl<'a> Format<'a> for AstNode<'a, TSUnionType<'a>> {
43774377
if !is_suppressed && format_type_cast_comment_node(self, false, f) {
43784378
return;
43794379
}
4380-
self.format_leading_comments(f);
43814380
let needs_parentheses = self.needs_parentheses(f);
43824381
if needs_parentheses {
43834382
"(".fmt(f);

‎crates/oxc_formatter/src/formatter/comments.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,12 @@ impl<'a> Comments<'a> {
369369

370370
/// Checks if the node has a suppression comment (prettier-ignore).
371371
pub fn is_suppressed(&self, start: u32) -> bool {
372-
self.comments_before(start).iter().any(|comment| {
373-
// TODO: Consider using `oxc-formatter-ignore` instead of `prettier-ignore`
374-
self.source_text.text_for(&comment.content_span()).trim() == "prettier-ignore"
375-
})
372+
self.comments_before(start).iter().any(|comment| self.is_suppression_comment(comment))
373+
}
374+
375+
pub fn is_suppression_comment(&self, comment: &Comment) -> bool {
376+
// TODO: Consider using `oxfmt-ignore` instead of `prettier-ignore`
377+
self.source_text.text_for(&comment.content_span()).trim() == "prettier-ignore"
376378
}
377379

378380
/// Checks if a comment is a type cast comment containing `@type` or `@satisfies`.

‎crates/oxc_formatter/src/utils/assignment_like.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,11 +554,12 @@ impl<'a> AssignmentLike<'a, '_> {
554554
_ => false,
555555
}
556556
};
557-
558557
is_generic(&conditional_type.check_type)
559558
|| is_generic(&conditional_type.extends_type)
560559
|| comments.has_comment_before(decl.type_annotation.span().start)
561560
}
561+
// `TSUnionType` has its own indentation logic
562+
TSType::TSUnionType(_) => false,
562563
_ => {
563564
// Check for leading comments on any other type
564565
comments.has_comment_before(decl.type_annotation.span().start)

‎crates/oxc_formatter/src/write/union_type.rs‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,29 +52,33 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSUnionType<'a>> {
5252
union_type_at_top = parent;
5353
}
5454

55-
let should_indent = {
55+
let should_indent = !has_leading_comments && {
5656
let parent = union_type_at_top.parent;
5757

5858
// These parents have indent for their content, so we don't need to indent here
59-
!match parent {
60-
AstNodes::TSTypeAliasDeclaration(_) => has_leading_comments,
59+
match parent {
60+
AstNodes::TSTypeAliasDeclaration(alias) => {
61+
!f.comments().printed_comments().last().is_some_and(|comment| {
62+
comment.span.start
63+
> alias.type_parameters().map_or(alias.id.span.end, |tp| tp.span.end)
64+
&& f.comments().is_end_of_line_comment(comment)
65+
})
66+
}
6167
AstNodes::TSTypeAssertion(_)
6268
| AstNodes::TSTupleType(_)
63-
| AstNodes::TSTypeParameterInstantiation(_) => true,
64-
_ => false,
69+
| AstNodes::TSTypeParameterInstantiation(_) => false,
70+
_ => true,
6571
}
6672
};
6773

6874
let types = format_with(|f| {
69-
let suppressed_node_span = if f.comments().is_suppressed(self.span.start) {
70-
self.types.first().unwrap().span()
71-
} else {
72-
Span::default()
73-
};
75+
let is_suppressed = leading_comments
76+
.iter()
77+
.rev()
78+
.any(|comment| f.comments().is_suppression_comment(comment));
7479

75-
if has_leading_comments {
76-
write!(f, FormatLeadingComments::Comments(leading_comments));
77-
}
80+
let suppressed_node_span =
81+
if is_suppressed { self.types.first().unwrap().span() } else { Span::default() };
7882

7983
let leading_soft_line_break_or_space = should_indent && !has_leading_comments;
8084

@@ -123,7 +127,24 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSUnionType<'a>> {
123127
}
124128
});
125129

126-
write!(f, [group(&content)]);
130+
if has_leading_comments {
131+
let has_own_line_leading_comment = union_type_at_top.types.len() > 1
132+
&& leading_comments.iter().any(|comment| f.comments().is_own_line_comment(comment));
133+
let is_end_of_line_comment = leading_comments
134+
.last()
135+
.is_some_and(|comment| f.comments().is_end_of_line_comment(comment));
136+
write!(
137+
f,
138+
[group(&indent(&format_args!(
139+
has_own_line_leading_comment.then(soft_line_break),
140+
FormatLeadingComments::Comments(leading_comments),
141+
(!is_end_of_line_comment).then(soft_line_break),
142+
group(&content)
143+
)))]
144+
);
145+
} else {
146+
write!(f, [group(&content)]);
147+
}
127148
}
128149
}
129150

‎crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export default class TestUnionTypeAnnotation1 {
2+
private prop!: /* comment */
3+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
4+
5+
private accessor prop2!: /* comment */
6+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
7+
}
8+
9+
export interface TestUnionTypeAnnotation2 {
10+
property: /* comment */
11+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
12+
}

‎crates/oxc_formatter/tests/fixtures/ts/union/issue-16176.ts.snap‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
source: crates/oxc_formatter/tests/fixtures/mod.rs
3+
---
4+
==================== Input ====================
5+
export default class TestUnionTypeAnnotation1 {
6+
private prop!: /* comment */
7+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
8+
9+
private accessor prop2!: /* comment */
10+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
11+
}
12+
13+
export interface TestUnionTypeAnnotation2 {
14+
property: /* comment */
15+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
16+
}
17+
18+
==================== Output ====================
19+
------------------
20+
{ printWidth: 80 }
21+
------------------
22+
export default class TestUnionTypeAnnotation1 {
23+
private prop!: /* comment */
24+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
25+
26+
private accessor prop2: /* comment */
27+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
28+
}
29+
30+
export interface TestUnionTypeAnnotation2 {
31+
property: /* comment */
32+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
33+
}
34+
35+
-------------------
36+
{ printWidth: 100 }
37+
-------------------
38+
export default class TestUnionTypeAnnotation1 {
39+
private prop!: /* comment */ LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
40+
41+
private accessor prop2: /* comment */
42+
LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
43+
}
44+
45+
export interface TestUnionTypeAnnotation2 {
46+
property: /* comment */ LongLongLongLongLongLongType[] | LongLongLongLongLongLongType[];
47+
}
48+
49+
===================== End =====================

‎tasks/ast_tools/src/generators/formatter/format.rs‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ const AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST: &[&str] = &[
3131
"TemplateElement",
3232
];
3333

34+
const AST_NODE_WITHOUT_PRINTING_LEADING_COMMENTS_LIST: &[&str] = &["TSUnionType"];
35+
3436
const AST_NODE_NEEDS_PARENTHESES: &[&str] = &[
3537
"TSTypeAssertion",
3638
"TSInferType",
@@ -109,22 +111,19 @@ fn generate_struct_implementation(
109111

110112
let struct_name = struct_def.name();
111113
let do_not_print_comment = AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST.contains(&struct_name);
114+
let do_not_print_leading_comment = do_not_print_comment
115+
|| AST_NODE_WITHOUT_PRINTING_LEADING_COMMENTS_LIST.contains(&struct_name);
112116

113-
let leading_comments = if do_not_print_comment {
114-
quote! {}
115-
} else {
117+
let leading_comments = (!do_not_print_leading_comment).then(|| {
116118
quote! {
117119
self.format_leading_comments(f);
118120
}
119-
};
120-
121-
let trailing_comments = if do_not_print_comment {
122-
quote! {}
123-
} else {
121+
});
122+
let trailing_comments = (!do_not_print_comment).then(|| {
124123
quote! {
125124
self.format_trailing_comments(f);
126125
}
127-
};
126+
});
128127

129128
let needs_parentheses = parenthesis_type_ids.contains(&struct_def.id);
130129

@@ -171,7 +170,7 @@ fn generate_struct_implementation(
171170

172171
let write_implementation = if suppressed_check.is_none() {
173172
write_call
174-
} else if trailing_comments.is_empty() {
173+
} else if trailing_comments.is_none() {
175174
quote! {
176175
if is_suppressed {
177176
self.format_leading_comments(f);
@@ -214,7 +213,7 @@ fn generate_struct_implementation(
214213
}
215214
});
216215

217-
if needs_parentheses_before.is_empty() && trailing_comments.is_empty() {
216+
if needs_parentheses_before.is_empty() && trailing_comments.is_none() {
218217
quote! {
219218
#suppressed_check
220219
#type_cast_comment_formatting

‎tasks/coverage/snapshots/formatter_typescript.snap‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ commit: 669c25c0
22

33
formatter_typescript Summary:
44
AST Parsed : 9822/9822 (100.00%)
5-
Positive Passed: 9815/9822 (99.93%)
5+
Positive Passed: 9814/9822 (99.92%)
66
Mismatch: tasks/coverage/typescript/tests/cases/compiler/amdLikeInputDeclarationEmit.ts
77

88
Expect to Parse: tasks/coverage/typescript/tests/cases/compiler/genericTypeAssertions3.ts
99
Unexpected token
1010
Expect to Parse: tasks/coverage/typescript/tests/cases/compiler/typeAssertionToGenericFunctionType.ts
1111
Unexpected token
12+
Mismatch: tasks/coverage/typescript/tests/cases/compiler/unionTypeWithLeadingOperator.ts
13+
1214
Mismatch: tasks/coverage/typescript/tests/cases/conformance/expressions/contextualTyping/parenthesizedContexualTyping2.ts
1315

1416
Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/expressions/elementAccess/letIdentifierInElementAccess01.ts

‎tasks/prettier_conformance/snapshots/prettier.ts.snap.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
ts compatibility: 571/602 (94.85%)
1+
ts compatibility: 572/602 (95.02%)
22

33
# Failed
44

@@ -32,6 +32,5 @@ ts compatibility: 571/602 (94.85%)
3232
| typescript/object-multiline/multiline.ts | 💥✨ | 23.21% |
3333
| typescript/property-signature/consistent-with-flow/comments.ts | 💥 | 80.00% |
3434
| typescript/union/union-parens.ts | 💥 | 92.59% |
35-
| typescript/union/comments/18106.ts | 💥 | 90.48% |
36-
| typescript/union/consistent-with-flow/comment.ts | 💥 | 82.61% |
37-
| typescript/union/single-type/single-type.ts | 💥 | 0.00% |
35+
| typescript/union/comments/18106.ts | 💥 | 92.68% |
36+
| typescript/union/single-type/single-type.ts | 💥 | 66.67% |

0 commit comments

Comments
 (0)