Skip to content

Commit 8486d8d

Browse files
committed
feat(formatter): complete printing Class and decorators (#13018)
| Language | This Stack | Main Branch | Improvement | |----------|---------------|-------------|-------------| | **TypeScript** | 277/573 (48.34%) | 249/573 (43.46%) | **+28 tests** (+4.88%) | | **JavaScript** | 525/699 (75.11%) | 505/699 (72.25%) | **+20 tests** (+2.86%) | Total improvement: **+48 tests** passing compared to main branch
1 parent 21b4b72 commit 8486d8d

File tree

12 files changed

+569
-329
lines changed

12 files changed

+569
-329
lines changed

crates/oxc_formatter/src/formatter/builders.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2439,17 +2439,17 @@ where
24392439
});
24402440
}
24412441

2442-
// /// Adds an iterator of entries to the output. Each entry is a `(node, content)` tuple.
2443-
// pub fn entries<F, I>(&mut self, entries: I) -> &mut Self
2444-
// where
2445-
// F: Format,
2446-
// I: IntoIterator<Item = ((), F)>,
2447-
// {
2448-
// for (node, content) in entries {
2449-
// self.entry(node, &content)
2450-
// }
2451-
// self
2452-
// }
2442+
/// Adds an iterator of entries to the output. Each entry is a `(node, content)` tuple.
2443+
pub fn entries<'a, F, I>(&mut self, entries: I) -> &mut Self
2444+
where
2445+
F: Format<'ast> + GetSpan + 'a,
2446+
I: IntoIterator<Item = &'a F>,
2447+
{
2448+
for content in entries {
2449+
self.entry(content.span(), &content);
2450+
}
2451+
self
2452+
}
24532453

24542454
pub fn finish(&mut self) -> FormatResult<()> {
24552455
self.result

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ impl<'a> Comments<'a> {
9898
let comments = self.comments_after(start);
9999
while index < comments.len() {
100100
let comment = &comments[index];
101-
if self.source_text[start as usize..comment.span.end as usize]
101+
if self.source_text[start as usize..comment.span.start as usize]
102102
.contains(character as char)
103103
{
104104
return &comments[..index];

crates/oxc_formatter/src/generated/format.rs

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -988,10 +988,7 @@ impl<'a> Format<'a> for AstNode<'a, Class<'a>> {
988988

989989
impl<'a> Format<'a> for AstNode<'a, ClassBody<'a>> {
990990
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
991-
self.format_leading_comments(f)?;
992-
let result = self.write(f);
993-
self.format_trailing_comments(f)?;
994-
result
991+
self.write(f)
995992
}
996993
}
997994

@@ -1136,19 +1133,13 @@ impl<'a> Format<'a> for AstNode<'a, ImportAttributeKey<'a>> {
11361133

11371134
impl<'a> Format<'a> for AstNode<'a, ExportNamedDeclaration<'a>> {
11381135
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1139-
self.format_leading_comments(f)?;
1140-
let result = self.write(f);
1141-
self.format_trailing_comments(f)?;
1142-
result
1136+
self.write(f)
11431137
}
11441138
}
11451139

11461140
impl<'a> Format<'a> for AstNode<'a, ExportDefaultDeclaration<'a>> {
11471141
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1148-
self.format_leading_comments(f)?;
1149-
let result = self.write(f);
1150-
self.format_trailing_comments(f)?;
1151-
result
1142+
self.write(f)
11521143
}
11531144
}
11541145

@@ -1822,10 +1813,7 @@ impl<'a> Format<'a> for AstNode<'a, TSTypeAliasDeclaration<'a>> {
18221813

18231814
impl<'a> Format<'a> for AstNode<'a, TSClassImplements<'a>> {
18241815
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
1825-
self.format_leading_comments(f)?;
1826-
let result = self.write(f);
1827-
self.format_trailing_comments(f)?;
1828-
result
1816+
self.write(f)
18291817
}
18301818
}
18311819

@@ -2144,10 +2132,7 @@ impl<'a> Format<'a> for AstNode<'a, TSNonNullExpression<'a>> {
21442132

21452133
impl<'a> Format<'a> for AstNode<'a, Decorator<'a>> {
21462134
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
2147-
self.format_leading_comments(f)?;
2148-
let result = self.write(f);
2149-
self.format_trailing_comments(f)?;
2150-
result
2135+
self.write(f)
21512136
}
21522137
}
21532138

crates/oxc_formatter/src/parentheses/expression.rs

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ArrayExpression<'a>> {
113113
impl<'a> NeedsParentheses<'a> for AstNode<'a, ObjectExpression<'a>> {
114114
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
115115
let parent = self.parent;
116-
is_class_extends(parent, self.span())
116+
is_class_extends(self.span, parent)
117117
|| is_first_in_statement(
118118
self.span,
119119
parent,
@@ -136,7 +136,7 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, MemberExpression<'a>> {
136136

137137
impl<'a> NeedsParentheses<'a> for AstNode<'a, ComputedMemberExpression<'a>> {
138138
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
139-
false
139+
matches!(self.parent, AstNodes::Decorator(_))
140140
}
141141
}
142142

@@ -152,10 +152,27 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, PrivateFieldExpression<'a>> {
152152
}
153153
}
154154

155+
fn is_identifier_or_static_member_only(callee: &Expression) -> bool {
156+
let mut expr = callee;
157+
loop {
158+
match expr {
159+
Expression::Identifier(_) => return true,
160+
Expression::StaticMemberExpression(static_member) => {
161+
expr = &static_member.object;
162+
}
163+
_ => break,
164+
}
165+
}
166+
167+
false
168+
}
169+
155170
impl<'a> NeedsParentheses<'a> for AstNode<'a, CallExpression<'a>> {
156171
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
157-
matches!(self.parent, AstNodes::NewExpression(_))
158-
|| matches!(self.parent, AstNodes::ExportDefaultDeclaration(_)) && {
172+
match self.parent {
173+
AstNodes::NewExpression(_) => true,
174+
AstNodes::Decorator(_) => !is_identifier_or_static_member_only(&self.callee),
175+
AstNodes::ExportDefaultDeclaration(_) => {
159176
let callee = &self.callee;
160177
let callee_span = callee.span();
161178
let leftmost = ExpressionLeftSide::leftmost(callee);
@@ -169,12 +186,14 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, CallExpression<'a>> {
169186
)
170187
)
171188
}
189+
_ => false,
190+
}
172191
}
173192
}
174193

175194
impl<'a> NeedsParentheses<'a> for AstNode<'a, NewExpression<'a>> {
176195
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
177-
is_class_extends(self.parent, self.span())
196+
is_class_extends(self.span, self.parent)
178197
}
179198
}
180199

@@ -373,9 +392,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, AwaitExpression<'a>> {
373392
impl<'a> NeedsParentheses<'a> for AstNode<'a, ChainExpression<'a>> {
374393
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
375394
match self.parent {
376-
AstNodes::CallExpression(call) => true,
377-
AstNodes::NewExpression(new) => true,
378-
AstNodes::StaticMemberExpression(member) => true,
395+
AstNodes::CallExpression(_)
396+
| AstNodes::NewExpression(_)
397+
| AstNodes::StaticMemberExpression(_)
398+
| AstNodes::Decorator(_) => true,
379399
AstNodes::ComputedMemberExpression(member) => member.object.span() == self.span(),
380400
_ => false,
381401
}
@@ -392,11 +412,15 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Class<'a>> {
392412
AstNodes::CallExpression(_)
393413
| AstNodes::NewExpression(_)
394414
| AstNodes::ExportDefaultDeclaration(_) => true,
395-
_ => is_first_in_statement(
396-
self.span,
397-
parent,
398-
FirstInStatementMode::ExpressionOrExportDefault,
399-
),
415+
416+
_ => {
417+
(is_class_extends(self.span, self.parent) && !self.decorators.is_empty())
418+
|| is_first_in_statement(
419+
self.span,
420+
parent,
421+
FirstInStatementMode::ExpressionOrExportDefault,
422+
)
423+
}
400424
}
401425
}
402426
}
@@ -497,7 +521,7 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, TSTypeAssertion<'a>> {
497521
impl<'a> NeedsParentheses<'a> for AstNode<'a, TSNonNullExpression<'a>> {
498522
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
499523
let parent = self.parent;
500-
is_class_extends(parent, self.span())
524+
is_class_extends(self.span, parent)
501525
|| (matches!(parent, AstNodes::NewExpression(_))
502526
&& member_chain_callee_needs_parens(self.expression()))
503527
}
@@ -626,7 +650,7 @@ fn update_or_lower_expression_needs_parens(span: Span, parent: &AstNodes<'_>) ->
626650
| AstNodes::StaticMemberExpression(_)
627651
| AstNodes::TemplateLiteral(_)
628652
| AstNodes::TaggedTemplateExpression(_)
629-
) || is_class_extends(parent, span)
653+
) || is_class_extends(span, parent)
630654
{
631655
return true;
632656
}
@@ -763,7 +787,7 @@ fn ts_as_or_satisfies_needs_parens(parent: &AstNodes<'_>) -> bool {
763787
)
764788
}
765789

766-
fn is_class_extends(parent: &AstNodes<'_>, span: Span) -> bool {
790+
fn is_class_extends(span: Span, parent: &AstNodes<'_>) -> bool {
767791
if let AstNodes::Class(c) = parent {
768792
return c.super_class.as_ref().is_some_and(|c| c.without_parentheses().span() == span);
769793
}

0 commit comments

Comments
 (0)