Skip to content

Commit 16a9dc8

Browse files
DunqingDunqing
committed
fix(formatter): inconsistent printing of class extends and interface extends (#15892)
1 parent 300b496 commit 16a9dc8

File tree

3 files changed

+152
-165
lines changed

3 files changed

+152
-165
lines changed

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

Lines changed: 67 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -220,30 +220,22 @@ impl<'a> FormatWrite<'a> for AstNode<'a, TSIndexSignatureName<'a>> {
220220

221221
impl<'a> Format<'a> for AstNode<'a, Vec<'a, TSClassImplements<'a>>> {
222222
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
223-
write!(
224-
f,
225-
[
226-
"implements",
227-
group(&soft_line_indent_or_space(&format_once(|f| {
228-
let last_index = self.len().saturating_sub(1);
229-
let mut joiner = f.join_with(soft_line_break_or_space());
230-
231-
for (i, heritage) in FormatSeparatedIter::new(self.into_iter(), ",")
232-
.with_trailing_separator(TrailingSeparator::Disallowed)
233-
.enumerate()
234-
{
235-
if i == last_index {
236-
// The trailing comments of the last heritage should be printed inside the class declaration
237-
joiner.entry(&FormatNodeWithoutTrailingComments(&heritage));
238-
} else {
239-
joiner.entry(&heritage);
240-
}
241-
}
223+
let last_index = self.len().saturating_sub(1);
224+
let mut joiner = f.join_with(soft_line_break_or_space());
242225

243-
joiner.finish()
244-
})))
245-
]
246-
)
226+
for (i, heritage) in FormatSeparatedIter::new(self.into_iter(), ",")
227+
.with_trailing_separator(TrailingSeparator::Disallowed)
228+
.enumerate()
229+
{
230+
if i == last_index {
231+
// The trailing comments of the last heritage should be printed inside the class declaration
232+
joiner.entry(&FormatNodeWithoutTrailingComments(&heritage));
233+
} else {
234+
joiner.entry(&heritage);
235+
}
236+
}
237+
238+
joiner.finish()
247239
}
248240
}
249241

@@ -303,26 +295,6 @@ impl<'a> Format<'a> for FormatClass<'a, '_> {
303295
}
304296

305297
write!(f, "class")?;
306-
let indent_only_heritage = ((implements.is_empty() && super_class.is_some())
307-
|| (!implements.is_empty() && super_class.is_none()))
308-
&& type_parameters.as_ref().is_some_and(|type_parameters| {
309-
let current_node_end = type_parameters.span.end;
310-
let next_node_start = super_class
311-
.map(GetSpan::span)
312-
.or(implements.first().map(GetSpan::span))
313-
.unwrap()
314-
.start;
315-
!f.comments()
316-
.comments_in_range(current_node_end, next_node_start)
317-
.iter()
318-
.any(|c| c.is_line())
319-
});
320-
321-
let type_parameters_id = if indent_only_heritage && !implements.is_empty() {
322-
Some(f.group_id("type_parameters"))
323-
} else {
324-
None
325-
};
326298

327299
let head = format_with(|f| {
328300
if let Some(id) = self.id() {
@@ -339,6 +311,7 @@ impl<'a> Format<'a> for FormatClass<'a, '_> {
339311
}
340312

341313
if let Some(type_parameters) = &type_parameters {
314+
let type_parameters_id = Some(f.group_id("type_parameters"));
342315
write!(
343316
f,
344317
FormatTSTypeParameters::new(
@@ -364,8 +337,8 @@ impl<'a> Format<'a> for FormatClass<'a, '_> {
364337
// after the class name, maintaining their position before the extends clause.
365338
if let Some(super_class) = &super_class {
366339
let comments = f.context().comments().comments_before(super_class.span().start);
367-
if comments.iter().any(|c| c.is_line()) {
368-
FormatTrailingComments::Comments(comments).fmt(f)?;
340+
if comments.iter().any(|c| f.comments().is_own_line_comment(c)) {
341+
indent(&FormatTrailingComments::Comments(comments)).fmt(f)?;
369342
}
370343
}
371344

@@ -418,78 +391,86 @@ impl<'a> Format<'a> for FormatClass<'a, '_> {
418391
});
419392

420393
if matches!(extends.grand_parent(), AstNodes::AssignmentExpression(_)) {
421-
if has_trailing_comments {
422-
write!(f, [token("("), &content, token(")")])
423-
} else {
424-
let content = content.memoized();
425-
write!(
426-
f,
427-
[
428-
if_group_breaks(&format_args!(
429-
token("("),
430-
&soft_block_indent(&content),
431-
token(")"),
432-
)),
433-
if_group_fits_on_line(&content)
434-
]
435-
)
436-
}
394+
let content = content.memoized();
395+
write!(
396+
f,
397+
[group(&format_args!(
398+
&if_group_breaks(&format_args!(
399+
token("("),
400+
&soft_block_indent(&content),
401+
token(")"),
402+
)),
403+
&if_group_fits_on_line(&content)
404+
))]
405+
)
437406
} else {
438407
content.fmt(f)
439408
}
440409
});
441410

442-
let extends =
443-
format_once(|f| write!(f, [space(), "extends", space(), group(&format_super)]));
411+
let format_extends =
412+
format_once(|f| write!(f, [space(), "extends", space(), &format_super]));
444413

445414
if group_mode {
446-
write!(f, [soft_line_break_or_space(), group(&extends)])?;
415+
write!(f, [soft_line_break_or_space(), group(&format_extends)])?;
447416
} else {
448-
write!(f, extends)?;
417+
write!(f, format_extends)?;
449418
}
450419
}
451420

452421
if !implements.is_empty() {
453-
if indent_only_heritage {
422+
let leading_comments =
423+
f.context().comments().comments_before(implements[0].span().start);
424+
425+
if usize::from(super_class.is_some()) + implements.len() > 1 {
454426
write!(
455427
f,
456428
[
457-
if_group_breaks(&space()).with_group_id(type_parameters_id),
458-
if_group_fits_on_line(&soft_line_break_or_space())
459-
.with_group_id(type_parameters_id)
429+
soft_line_break_or_space(),
430+
FormatLeadingComments::Comments(leading_comments),
431+
(!leading_comments.is_empty()).then_some(hard_line_break()),
432+
"implements",
433+
group(&soft_line_indent_or_space(implements))
460434
]
461435
)?;
462436
} else {
463-
write!(f, [soft_line_break_or_space()])?;
464-
}
437+
let format_inner = format_once(|f| {
438+
write!(
439+
f,
440+
[
441+
FormatLeadingComments::Comments(leading_comments),
442+
"implements",
443+
space(),
444+
implements,
445+
]
446+
)
447+
});
465448

466-
let comments = f.context().comments().comments_before(implements[0].span().start);
467-
write!(f, [FormatLeadingComments::Comments(comments), implements])?;
449+
if group_mode {
450+
write!(f, [soft_line_break_or_space(), group(&format_inner)])?;
451+
} else {
452+
write!(f, [space(), format_inner])?;
453+
}
454+
}
468455
}
469456

470457
Ok(())
471458
});
472459

473460
if group_mode {
474-
let indented = format_with(|f| {
475-
if indent_only_heritage {
476-
write!(f, [head, indent(&format_heritage_clauses)])
477-
} else {
478-
write!(f, indent(&format_args!(head, format_heritage_clauses)))
479-
}
480-
});
461+
let indented = format_with(|f| write!(f, [head, indent(&format_heritage_clauses)]));
481462

482463
let heritage_id = f.group_id("heritageGroup");
483464
write!(f, [group(&indented).with_group_id(Some(heritage_id)), space()])?;
484465

485-
if !body.body().is_empty() {
466+
if !body.body.is_empty() {
486467
write!(f, [if_group_breaks(&hard_line_break()).with_group_id(Some(heritage_id))])?;
487468
}
488469
} else {
489470
write!(f, [head, format_heritage_clauses, space()])?;
490471
}
491472

492-
if body.body().is_empty() {
473+
if body.body.is_empty() {
493474
write!(f, ["{", format_dangling_comments(self.span).with_block_indent(), "}"])
494475
} else {
495476
body.fmt(f)
@@ -503,16 +484,16 @@ impl<'a> Format<'a> for FormatClass<'a, '_> {
503484
/// on the same line but break together if they don't fit.
504485
///
505486
/// Heritage clauses are grouped when:
506-
/// 1. The class has an `implements` clause
487+
/// 1. Superclass and/or implements are more than one
507488
/// 2. There are comments in the heritage clause area
508489
/// 3. There are trailing line comments after type parameters
509490
fn should_group<'a>(class: &Class<'a>, f: &Formatter<'_, 'a>) -> bool {
510-
let comments = f.comments();
511-
512-
if !class.implements.is_empty() {
491+
if usize::from(class.super_class.is_some()) + class.implements.len() > 1 {
513492
return true;
514493
}
515494

495+
let comments = f.comments();
496+
516497
let id_span = class.id.as_ref().map(GetSpan::span);
517498
let type_parameters_span = class.type_parameters.as_ref().map(|t| t.span);
518499
let super_class_span = class.super_class.as_ref().map(GetSpan::span);

0 commit comments

Comments
 (0)