Skip to content

Commit 144e284

Browse files
committed
fix(formatter): algin printing comments behavior with prettier for some nodes (#12420)
Our Rust side AST is different from `prettier`, so we should skip some nodes to not print comments, which aligns with the Prettier behavior.
1 parent 34c5a2f commit 144e284

File tree

6 files changed

+43
-35
lines changed

6 files changed

+43
-35
lines changed

crates/oxc_formatter/src/generated/ast_nodes.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4814,7 +4814,7 @@ impl<'a> AstNode<'a, AssignmentExpression<'a>> {
48144814

48154815
#[inline]
48164816
pub fn right(&self) -> &AstNode<'a, Expression<'a>> {
4817-
let following_node = self.following_node;
4817+
let following_node = None;
48184818
self.allocator.alloc(AstNode {
48194819
inner: &self.inner.right,
48204820
allocator: self.allocator,
@@ -5971,7 +5971,7 @@ impl<'a> AstNode<'a, ExpressionStatement<'a>> {
59715971

59725972
#[inline]
59735973
pub fn expression(&self) -> &AstNode<'a, Expression<'a>> {
5974-
let following_node = self.following_node;
5974+
let following_node = None;
59755975
self.allocator.alloc(AstNode {
59765976
inner: &self.inner.expression,
59775977
allocator: self.allocator,
@@ -7344,7 +7344,7 @@ impl<'a> AstNode<'a, FormalParameters<'a>> {
73447344

73457345
#[inline]
73467346
pub fn rest(&self) -> Option<&AstNode<'a, BindingRestElement<'a>>> {
7347-
let following_node = self.following_node;
7347+
let following_node = None;
73487348
self.allocator
73497349
.alloc(self.inner.rest.as_ref().map(|inner| AstNode {
73507350
inner: inner.as_ref(),

crates/oxc_formatter/src/generated/format.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -913,10 +913,7 @@ impl<'a> Format<'a, FormatFunctionOptions> for AstNode<'a, Function<'a>> {
913913

914914
impl<'a> Format<'a> for AstNode<'a, FormalParameters<'a>> {
915915
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
916-
self.format_leading_comments(f)?;
917-
let result = self.write(f);
918-
self.format_trailing_comments(f)?;
919-
result
916+
self.write(f)
920917
}
921918
}
922919

tasks/ast_tools/src/generators/formatter/ast_nodes.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ pub fn get_node_type(ty: &TokenStream) -> TokenStream {
1818

1919
const FORMATTER_CRATE_PATH: &str = "crates/oxc_formatter";
2020

21+
/// Based on the printing comments algorithm, the last child of these AST nodes don't need to print comments.
22+
/// Without following nodes could lead to only print comments that before the end of the node, which is what we want.
23+
const AST_NODE_WITHOUT_FOLLOWING_NODE_LIST: &[&str] =
24+
&["ExpressionStatement", "AssignmentExpression", "FormalParameters"];
25+
2126
pub struct FormatterAstNodesGenerator;
2227

2328
define_generator!(FormatterAstNodesGenerator);
@@ -255,8 +260,16 @@ fn generate_struct_impls(struct_def: &StructDef, schema: &Schema) -> TokenStream
255260
quote! { &self.inner.#field_name }
256261
};
257262

258-
let mut following_node = quote! {
259-
self.following_node
263+
let mut following_node = if index == fields.len() - 1
264+
&& AST_NODE_WITHOUT_FOLLOWING_NODE_LIST.contains(&struct_def.name.as_str())
265+
{
266+
quote! {
267+
None
268+
}
269+
} else {
270+
quote! {
271+
self.following_node
272+
}
260273
};
261274

262275
let mut next_field_index = index + 1;

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ use crate::{
1313

1414
const FORMATTER_CRATE_PATH: &str = "crates/oxc_formatter";
1515

16+
/// Based on the prettier printing comments algorithm, these nodes don't need to print comments.
17+
const AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST: &[&str] = &["FormalParameters"];
18+
1619
const NEEDS_PARENTHESES: &[&str] = &[
1720
"Class",
1821
"Function",
@@ -96,16 +99,17 @@ fn implementation(type_def: &TypeDef, schema: &Schema) -> TokenStream {
9699
}
97100

98101
let is_program = type_def.as_struct().is_some_and(|s| s.name == "Program");
102+
let do_not_print_comment = AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST.contains(&type_def.name());
99103

100-
let leading_comments = if type_def.is_enum() || is_program {
104+
let leading_comments = if type_def.is_enum() || is_program || do_not_print_comment {
101105
quote! {}
102106
} else {
103107
quote! {
104108
self.format_leading_comments(f)?;
105109
}
106110
};
107111

108-
let trailing_comments = if type_def.is_enum() {
112+
let trailing_comments = if type_def.is_enum() || do_not_print_comment {
109113
quote! {}
110114
} else if is_program {
111115
quote! {

tasks/prettier_conformance/snapshots/prettier.js.snap.md

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
1-
js compatibility: 394/699 (56.37%)
1+
js compatibility: 400/699 (57.22%)
22

33
# Failed
44

55
| Spec path | Failed or Passed | Match ratio |
66
| :-------- | :--------------: | :---------: |
7-
| js/arrays/preserve_empty_lines.js | 💥 | 97.30% |
87
| js/arrow-call/arrow_call.js | 💥💥💥 | 94.85% |
98
| js/arrows/call.js | 💥💥 | 75.68% |
10-
| js/arrows/comment.js | 💥💥 | 63.75% |
9+
| js/arrows/comment.js | 💥💥 | 94.55% |
1110
| js/arrows/curried.js | 💥💥 | 92.55% |
1211
| js/arrows/currying-4.js | 💥💥 | 94.34% |
1312
| js/arrows/issue-1389-curry.js | 💥💥 | 86.96% |
@@ -20,12 +19,11 @@ js compatibility: 394/699 (56.37%)
2019
| js/assignment/issue-2482-2.js | 💥 | 62.50% |
2120
| js/assignment/issue-7572.js | 💥 | 72.73% |
2221
| js/assignment/sequence.js | 💥 | 71.43% |
23-
| js/assignment-comments/function.js | 💥 | 92.73% |
22+
| js/assignment-comments/function.js | 💥 | 78.18% |
2423
| js/async/inline-await.js | 💥 | 25.00% |
2524
| js/async/nested.js | 💥 | 16.67% |
2625
| js/binary-expressions/arrow.js | 💥 | 66.67% |
2726
| js/binary-expressions/comment.js | 💥 | 86.36% |
28-
| js/binary-expressions/in_instanceof.js | 💥 | 97.96% |
2927
| js/binary-expressions/inline-jsx.js | 💥 | 40.00% |
3028
| js/binary-expressions/inline-object-array.js | 💥 | 85.71% |
3129
| js/binary-expressions/jsx_parent.js | 💥 | 33.85% |
@@ -45,6 +43,7 @@ js compatibility: 394/699 (56.37%)
4543
| js/chain-expression/test-3.js | 💥 | 75.00% |
4644
| js/chain-expression/test.js | 💥 | 25.00% |
4745
| js/class-comment/class-property.js | 💥 | 30.77% |
46+
| js/class-comment/misc.js | 💥 | 72.73% |
4847
| js/class-comment/superclass.js | 💥 | 57.83% |
4948
| js/class-extends/complex.js | 💥 | 89.47% |
5049
| js/class-extends/extends.js | 💥 | 94.74% |
@@ -54,36 +53,35 @@ js compatibility: 394/699 (56.37%)
5453
| js/classes/method.js | 💥 | 71.43% |
5554
| js/classes/property.js | 💥 | 62.86% |
5655
| js/classes-private-fields/with_comments.js | 💥💥 | 30.77% |
57-
| js/comments/15661.js | 💥💥 | 46.15% |
56+
| js/comments/15661.js | 💥💥 | 53.63% |
5857
| js/comments/16398.js | 💥💥 | 80.00% |
5958
| js/comments/blank.js | 💥💥 | 95.24% |
6059
| js/comments/call_comment.js | 💥💥 | 90.91% |
6160
| js/comments/dangling.js | 💥💥 | 93.33% |
6261
| js/comments/dangling_array.js | 💥💥 | 25.00% |
6362
| js/comments/dangling_for.js | 💥💥 | 22.22% |
64-
| js/comments/dynamic_imports.js | 💥💥 | 66.67% |
65-
| js/comments/empty-statements.js | 💥💥 | 87.88% |
63+
| js/comments/dynamic_imports.js | 💥💥 | 71.43% |
64+
| js/comments/empty-statements.js | 💥💥 | 90.91% |
6665
| js/comments/export-and-import.js | 💥💥 | 78.05% |
6766
| js/comments/export.js | 💥💥 | 84.93% |
68-
| js/comments/function-declaration.js | 💥💥 | 67.77% |
67+
| js/comments/function-declaration.js | 💥💥 | 66.67% |
6968
| js/comments/if.js | 💥💥 | 38.16% |
7069
| js/comments/issue-3532.js | 💥💥 | 80.82% |
71-
| js/comments/issues.js | 💥💥 | 73.68% |
70+
| js/comments/issues.js | 💥💥 | 74.63% |
7271
| js/comments/jsdoc-nestled-dangling.js | 💥💥 | 93.02% |
7372
| js/comments/jsdoc-nestled.js | 💥💥 | 89.29% |
7473
| js/comments/jsdoc.js | 💥💥 | 47.83% |
7574
| js/comments/jsx.js | 💥💥 | 41.63% |
7675
| js/comments/last-arg.js | 💥💥 | 80.65% |
77-
| js/comments/multi-comments-on-same-line.js | 💥✨ | 42.78% |
78-
| js/comments/multi-comments.js | 💥✨ | 36.84% |
76+
| js/comments/multi-comments.js | 💥✨ | 44.74% |
7977
| js/comments/return-statement.js | 💥💥 | 52.32% |
8078
| js/comments/tagged-template-literal.js | 💥💥 | 69.23% |
8179
| js/comments/template-literal.js | 💥💥 | 30.43% |
8280
| js/comments/trailing_space.js | 💥💥 | 60.00% |
8381
| js/comments/try.js | 💥💥 | 71.43% |
8482
| js/comments/variable_declarator.js | 💥✨ | 49.31% |
8583
| js/comments/while.js | 💥💥 | 62.75% |
86-
| js/comments/flow-types/inline.js | 💥 | 53.33% |
84+
| js/comments/flow-types/inline.js | 💥 | 62.50% |
8785
| js/comments/function/between-parentheses-and-function-body.js | 💥 | 55.17% |
8886
| js/comments/html-like/comment.js | 💥 | 0.00% |
8987
| js/comments-closure-typecast/binary-expr.js | 💥 | 0.00% |
@@ -121,7 +119,6 @@ js compatibility: 394/699 (56.37%)
121119
| js/destructuring/destructuring.js | 💥 | 78.90% |
122120
| js/destructuring/issue-5988.js | 💥 | 0.00% |
123121
| js/destructuring-ignore/ignore.js | 💥💥💥 | 77.11% |
124-
| js/empty-paren-comment/empty_paren_comment.js | 💥 | 94.44% |
125122
| js/empty-statement/no-newline.js | 💥 | 85.71% |
126123
| js/explicit-resource-management/for-await-using-of-comments.js | 💥 | 0.00% |
127124
| js/explicit-resource-management/using-declarations.js | 💥 | 70.00% |
@@ -136,7 +133,7 @@ js compatibility: 394/699 (56.37%)
136133
| js/for/in.js | 💥 | 50.00% |
137134
| js/for/parentheses.js | 💥 | 72.00% |
138135
| js/for-of/async-identifier.js | 💥 | 90.00% |
139-
| js/function-comments/params-trail-comments.js | 💥 | 82.61% |
136+
| js/function-comments/params-trail-comments.js | 💥 | 95.83% |
140137
| js/function-single-destructuring/array.js | 💥 | 35.42% |
141138
| js/functional-composition/functional_compose.js | 💥 | 93.20% |
142139
| js/functional-composition/pipe-function-calls-with-comments.js | 💥 | 77.08% |
@@ -147,7 +144,7 @@ js compatibility: 394/699 (56.37%)
147144
| js/identifier/parentheses/const.js | 💥💥 | 0.00% |
148145
| js/identifier/parentheses/let.js | 💥💥 | 79.09% |
149146
| js/if/comment_before_else.js | 💥 | 61.54% |
150-
| js/if/expr_and_same_line_comments.js | 💥 | 73.33% |
147+
| js/if/expr_and_same_line_comments.js | 💥 | 86.67% |
151148
| js/if/if_comments.js | 💥 | 54.72% |
152149
| js/if/non-block.js | 💥 | 66.67% |
153150
| js/if/trailing_comment.js | 💥 | 72.73% |
@@ -194,8 +191,6 @@ js compatibility: 394/699 (56.37%)
194191
| js/method-chain/test.js | 💥 | 54.55% |
195192
| js/method-chain/this.js | 💥 | 0.00% |
196193
| js/new-expression/new_expression.js | 💥 | 55.56% |
197-
| js/newline/backslash_2028.js | 💥 | 50.00% |
198-
| js/newline/backslash_2029.js | 💥 | 50.00% |
199194
| js/no-semi/class.js | 💥✨ | 46.55% |
200195
| js/no-semi/comments.js | 💥✨ | 36.36% |
201196
| js/no-semi/issue2006.js | 💥✨ | 37.50% |
@@ -258,7 +253,6 @@ js compatibility: 394/699 (56.37%)
258253
| js/trailing-comma/jsx.js | 💥💥💥 | 0.00% |
259254
| js/try/catch.js | 💥 | 52.63% |
260255
| js/try/try.js | 💥 | 50.00% |
261-
| js/unary-expression/comments.js | 💥 | 76.58% |
262256
| js/unicode/nbsp-jsx.js | 💥 | 22.22% |
263257
| js/variable_declarator/multiple.js | 💥 | 87.27% |
264258
| js/yield/jsx-without-parenthesis.js | 💥 | 50.00% |

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ ts compatibility: 233/573 (40.66%)
7676
| typescript/assignment/issue-12413.ts | 💥 | 3.03% |
7777
| typescript/assignment/issue-2485.ts | 💥 | 44.44% |
7878
| typescript/call-signature/call-signature.ts | 💥 | 79.66% |
79-
| typescript/cast/as-const.ts | 💥 | 54.55% |
79+
| typescript/cast/as-const.ts | 💥 | 60.00% |
8080
| typescript/cast/generic-cast.ts | 💥 | 39.46% |
8181
| typescript/cast/tuple-and-record.ts | 💥 | 0.00% |
8282
| typescript/chain-expression/call-expression.ts | 💥 | 32.81% |
@@ -105,9 +105,9 @@ ts compatibility: 233/573 (40.66%)
105105
| typescript/comments/jsx.tsx | 💥 | 20.00% |
106106
| typescript/comments/location.ts | 💥 | 75.00% |
107107
| typescript/comments/mapped_types.ts | 💥 | 58.82% |
108-
| typescript/comments/method_types.ts | 💥 | 69.23% |
108+
| typescript/comments/method_types.ts | 💥 | 74.36% |
109109
| typescript/comments/methods.ts | 💥 | 97.96% |
110-
| typescript/comments/type-parameters.ts | 💥 | 39.29% |
110+
| typescript/comments/type-parameters.ts | 💥 | 40.00% |
111111
| typescript/comments/type_literals.ts | 💥 | 55.17% |
112112
| typescript/comments/union.ts | 💥 | 5.26% |
113113
| typescript/comments-2/dangling.ts | 💥💥 | 75.00% |
@@ -167,12 +167,12 @@ ts compatibility: 233/573 (40.66%)
167167
| typescript/conformance/types/typeOperator/typeOperator.ts | 💥 | 0.00% |
168168
| typescript/conformance/types/typeParameter/typeParameter.ts | 💥 | 66.67% |
169169
| typescript/conformance/types/typeParameters/typeParameterLists/innerTypeParameterShadowingOuterOne2.ts | 💥 | 93.33% |
170-
| typescript/conformance/types/union/unionTypeCallSignatures.ts | 💥 | 49.51% |
170+
| typescript/conformance/types/union/unionTypeCallSignatures.ts | 💥 | 61.22% |
171171
| typescript/conformance/types/union/unionTypeCallSignatures3.ts | 💥 | 68.97% |
172172
| typescript/conformance/types/union/unionTypeConstructSignatures.ts | 💥 | 61.29% |
173173
| typescript/conformance/types/union/unionTypeEquivalence.ts | 💥 | 90.00% |
174174
| typescript/conformance/types/union/unionTypeFromArrayLiteral.ts | 💥 | 93.55% |
175-
| typescript/conformance/types/union/unionTypeIndexSignature.ts | 💥 | 29.85% |
175+
| typescript/conformance/types/union/unionTypeIndexSignature.ts | 💥 | 56.67% |
176176
| typescript/const/initializer-ambient-context.ts | 💥 | 93.33% |
177177
| typescript/custom/abstract/abstractProperties.ts | 💥 | 75.00% |
178178
| typescript/custom/computedProperties/string.ts | 💥 | 73.33% |

0 commit comments

Comments
 (0)