Skip to content

Commit 0150ad5

Browse files
committed
fix(formatter): should group type parameters and parameters for method-like and function-like nodes (#14659)
Align Prettier's behavior. The biome's implementation missed some nodes to do the same thing.
1 parent 392bf74 commit 0150ad5

File tree

7 files changed

+130
-32
lines changed

7 files changed

+130
-32
lines changed

crates/oxc_formatter/src/write/class.rs

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, MethodDefinition<'a>> {
102102
write!(f, "?")?;
103103
}
104104

105-
if value.type_parameters.is_none() {
106-
// // Handle comments between method name and parameters
107-
// // Example: method /* comment */ (param) {}
108-
// let comments = f.context().comments().comments_before(value.params().span.start);
109-
// if !comments.is_empty() {
110-
// write!(f, [space(), FormatTrailingComments::Comments(comments)])?;
111-
// }
112-
}
113-
114105
format_grouped_parameters_with_return_type(
115106
value.type_parameters(),
116107
value.this_param.as_deref(),
@@ -639,30 +630,28 @@ pub fn format_grouped_parameters_with_return_type<'a>(
639630
return_type: Option<&AstNode<'a, TSTypeAnnotation<'a>>>,
640631
f: &mut Formatter<'_, 'a>,
641632
) -> FormatResult<()> {
642-
write!(f, [type_parameters])?;
643-
644633
group(&format_once(|f| {
634+
let mut format_type_parameters = type_parameters.memoized();
645635
let mut format_parameters = params.memoized();
646636
let mut format_return_type = return_type.memoized();
647637

648638
// Inspect early, in case the `return_type` is formatted before `parameters`
649639
// in `should_group_function_parameters`.
640+
format_type_parameters.inspect(f)?;
650641
format_parameters.inspect(f)?;
651642

652643
let group_parameters = should_group_function_parameters(
653644
type_parameters.map(AsRef::as_ref),
654-
params.items.len()
655-
+ usize::from(params.rest.is_some())
656-
+ usize::from(this_param.is_some()),
645+
params.parameters_count() + usize::from(this_param.is_some()),
657646
return_type.map(AsRef::as_ref),
658647
&mut format_return_type,
659648
f,
660649
)?;
661650

662651
if group_parameters {
663-
write!(f, [group(&format_parameters)])
652+
write!(f, [group(&format_args!(format_type_parameters, format_parameters))])
664653
} else {
665-
write!(f, [format_parameters])
654+
write!(f, [format_type_parameters, format_parameters])
666655
}?;
667656

668657
write!(f, [format_return_type])

crates/oxc_formatter/src/write/function.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,7 @@ impl<'a> FormatWrite<'a> for FormatFunction<'a, '_> {
108108

109109
let group_parameters = should_group_function_parameters(
110110
self.type_parameters.as_deref(),
111-
params.items.len()
112-
+ usize::from(params.rest.is_some())
113-
+ usize::from(self.this_param.is_some()),
111+
params.parameters_count() + usize::from(self.this_param.is_some()),
114112
self.return_type.as_deref(),
115113
&mut format_return_type,
116114
f,

crates/oxc_formatter/src/write/mod.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -173,16 +173,16 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ObjectProperty<'a>> {
173173
};
174174

175175
if self.method || is_accessor {
176-
let AstNodes::Function(func) = self.value().as_ast_nodes() else {
176+
let AstNodes::Function(value) = self.value().as_ast_nodes() else {
177177
unreachable!(
178178
"The `value` always be a function node if `method` or `accessor` is true"
179179
)
180180
};
181181

182-
if func.r#async() {
182+
if value.r#async() {
183183
write!(f, ["async", space()])?;
184184
}
185-
if func.generator() {
185+
if value.generator() {
186186
write!(f, "*")?;
187187
}
188188
if self.computed {
@@ -191,14 +191,15 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ObjectProperty<'a>> {
191191
format_property_key(self.key(), f)?;
192192
}
193193

194-
if let Some(type_parameters) = &func.type_parameters() {
195-
write!(f, type_parameters)?;
196-
}
197-
write!(f, group(&func.params()))?;
198-
if let Some(return_type) = &func.return_type() {
199-
write!(f, return_type)?;
200-
}
201-
if let Some(body) = &func.body() {
194+
format_grouped_parameters_with_return_type(
195+
value.type_parameters(),
196+
value.this_param.as_deref(),
197+
value.params(),
198+
value.return_type(),
199+
f,
200+
)?;
201+
202+
if let Some(body) = &value.body() {
202203
write!(f, [space(), body])?;
203204
}
204205

@@ -1561,7 +1562,13 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, TSSignature<'a>>> {
15611562

15621563
impl<'a> FormatWrite<'a> for AstNode<'a, TSCallSignatureDeclaration<'a>> {
15631564
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1564-
write!(f, group(&format_args!(self.type_parameters(), self.params(), self.return_type())))
1565+
format_grouped_parameters_with_return_type(
1566+
self.type_parameters(),
1567+
None,
1568+
self.params(),
1569+
self.return_type(),
1570+
f,
1571+
)
15651572
}
15661573
}
15671574

@@ -1600,7 +1607,13 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSMethodSignature<'a>> {
16001607
impl<'a> FormatWrite<'a> for AstNode<'a, TSConstructSignatureDeclaration<'a>> {
16011608
fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
16021609
write!(f, ["new", space()])?;
1603-
write!(f, group(&format_args!(self.type_parameters(), self.params(), self.return_type())))
1610+
format_grouped_parameters_with_return_type(
1611+
self.type_parameters(),
1612+
None,
1613+
self.params(),
1614+
self.return_type(),
1615+
f,
1616+
)
16041617
}
16051618
}
16061619

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
2+
type A = {
3+
new(...args): T<{
4+
A
5+
}
6+
>
7+
};
8+
9+
10+
type A1 = {
11+
(...args): T<{
12+
A
13+
}
14+
>
15+
};
16+
17+
type A2 = {
18+
bar(
19+
...args
20+
): T<{
21+
A
22+
}>
23+
}
24+
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+
6+
type A = {
7+
new(...args): T<{
8+
A
9+
}
10+
>
11+
};
12+
13+
14+
type A1 = {
15+
(...args): T<{
16+
A
17+
}
18+
>
19+
};
20+
21+
type A2 = {
22+
bar(
23+
...args
24+
): T<{
25+
A
26+
}>
27+
}
28+
29+
30+
==================== Output ====================
31+
type A = {
32+
new (...args): T<{
33+
A;
34+
}>;
35+
};
36+
37+
type A1 = {
38+
(...args): T<{
39+
A;
40+
}>;
41+
};
42+
43+
type A2 = {
44+
bar(...args): T<{
45+
A;
46+
}>;
47+
};
48+
49+
===================== End =====================
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
const builder = {
2+
build(environment: BuildEnvironment): Promise<
3+
RollupOutput | RollupOutput[]
4+
> {
5+
},
6+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
source: crates/oxc_formatter/tests/fixtures/mod.rs
3+
---
4+
==================== Input ====================
5+
const builder = {
6+
build(environment: BuildEnvironment): Promise<
7+
RollupOutput | RollupOutput[]
8+
> {
9+
},
10+
}
11+
12+
==================== Output ====================
13+
const builder = {
14+
build(
15+
environment: BuildEnvironment,
16+
): Promise<RollupOutput | RollupOutput[]> {},
17+
};
18+
19+
===================== End =====================

0 commit comments

Comments
 (0)