Skip to content

Commit 6b9e19f

Browse files
committed
feat(formatter): correct printing for TSConditionalType
1 parent 8530b96 commit 6b9e19f

File tree

12 files changed

+164
-332
lines changed

12 files changed

+164
-332
lines changed

crates/oxc_formatter/src/formatter/comments.rs

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,17 +128,29 @@ pub struct Comments<'a> {
128128
/// The index of the type cast comment that has been printed already.
129129
/// Used to prevent duplicate processing of special TypeScript type cast comments.
130130
handled_type_cast_comment: usize,
131+
/// Optional limit for the unprinted_comments view.
132+
///
133+
/// When set, [`Self::unprinted_comments()`] will only return comments up to this index,
134+
/// effectively hiding comments beyond this point from the formatter.
135+
pub view_limit: Option<usize>,
131136
}
132137

133138
impl<'a> Comments<'a> {
134139
pub fn new(source_text: SourceText<'a>, comments: &'a [Comment]) -> Self {
135-
Comments { source_text, comments, printed_count: 0, handled_type_cast_comment: 0 }
140+
Comments {
141+
source_text,
142+
comments,
143+
printed_count: 0,
144+
handled_type_cast_comment: 0,
145+
view_limit: None,
146+
}
136147
}
137148

138149
/// Returns comments that have not been printed yet.
139150
#[inline]
140151
pub fn unprinted_comments(&self) -> &'a [Comment] {
141-
&self.comments[self.printed_count..]
152+
let end = self.view_limit.unwrap_or(self.comments.len());
153+
&self.comments[self.printed_count..end]
142154
}
143155

144156
/// Returns comments that have already been printed.
@@ -486,6 +498,34 @@ impl<'a> Comments<'a> {
486498
pub fn is_already_handled_type_cast_comment(&self) -> bool {
487499
self.printed_count == self.handled_type_cast_comment
488500
}
501+
502+
/// Temporarily limits the unprinted comments view to only those before the given position.
503+
/// Returns the previous view limit to allow restoration.
504+
pub fn limit_comments_up_to(&mut self, end_pos: u32) -> Option<usize> {
505+
// Save the original limit for restoration
506+
let original_limit = self.view_limit;
507+
508+
// Find the index of the first comment that starts at or after end_pos
509+
// Using binary search would be more efficient for large comment arrays
510+
let limit_index = self.comments[self.printed_count..]
511+
.iter()
512+
.position(|c| c.span.start >= end_pos)
513+
.map_or(self.comments.len(), |idx| self.printed_count + idx);
514+
515+
// Only update if we're actually limiting the view
516+
if limit_index < self.comments.len() {
517+
self.view_limit = Some(limit_index);
518+
}
519+
520+
original_limit
521+
}
522+
523+
/// Restores the view limit to a previously saved value.
524+
/// This is typically used after temporarily limiting the view with `limit_comments_up_to`.
525+
#[inline]
526+
pub fn restore_view_limit(&mut self, limit: Option<usize>) {
527+
self.view_limit = limit;
528+
}
489529
}
490530

491531
/// Checks if a pattern matches at the given position.

crates/oxc_formatter/src/utils/conditional.rs

Lines changed: 73 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
Format, FormatResult, FormatWrite,
88
formatter::{Formatter, prelude::*, trivia::FormatTrailingComments},
99
generated::ast_nodes::{AstNode, AstNodes},
10-
utils::expression::FormatExpressionWithoutTrailingComments,
10+
utils::format_node_without_trailing_comments::FormatNodeWithoutTrailingComments,
1111
write,
1212
};
1313

@@ -171,32 +171,35 @@ impl<'a> FormatConditionalLike<'a, '_> {
171171
fn layout(&self, f: &mut Formatter<'_, 'a>) -> ConditionalLayout {
172172
let self_span = self.span();
173173

174-
let (is_test, is_consequent) = match self.parent() {
174+
match self.parent() {
175175
AstNodes::ConditionalExpression(parent) => {
176176
let parent_expr = parent.as_ref();
177-
(parent_expr.test.span() == self_span, parent_expr.consequent.span() == self_span)
177+
if parent_expr.test.span() == self_span {
178+
ConditionalLayout::NestedTest
179+
} else if parent_expr.consequent.span() == self_span {
180+
ConditionalLayout::NestedConsequent
181+
} else {
182+
ConditionalLayout::NestedAlternate
183+
}
178184
}
179185
AstNodes::TSConditionalType(parent) => {
180186
let parent_type = parent.as_ref();
181187
// For TS conditional types, both check_type and extends_type are part of the test
182188
let is_test = parent_type.check_type.span() == self_span
183189
|| parent_type.extends_type.span() == self_span;
184-
let is_consequent = parent_type.true_type.span() == self_span;
185-
(is_test, is_consequent)
190+
if is_test {
191+
ConditionalLayout::NestedTest
192+
} else if parent_type.true_type.span() == self_span {
193+
ConditionalLayout::NestedConsequent
194+
} else {
195+
ConditionalLayout::NestedAlternate
196+
}
186197
}
187198
_ => {
188199
let jsx_chain =
189200
f.context().source_type().is_jsx() && self.is_jsx_conditional_chain();
190-
return ConditionalLayout::Root { jsx_chain };
201+
ConditionalLayout::Root { jsx_chain }
191202
}
192-
};
193-
194-
if is_test {
195-
ConditionalLayout::NestedTest
196-
} else if is_consequent {
197-
ConditionalLayout::NestedConsequent
198-
} else {
199-
ConditionalLayout::NestedAlternate
200203
}
201204
}
202205

@@ -374,7 +377,7 @@ impl<'a> FormatConditionalLike<'a, '_> {
374377
) -> FormatResult<()> {
375378
let format_inner = format_with(|f| match self.conditional {
376379
ConditionalLike::ConditionalExpression(conditional) => {
377-
write!(f, FormatExpressionWithoutTrailingComments(conditional.test()))?;
380+
write!(f, FormatNodeWithoutTrailingComments(conditional.test()))?;
378381
format_trailing_comments(
379382
conditional.test.span().end,
380383
conditional.consequent.span().start,
@@ -390,8 +393,15 @@ impl<'a> FormatConditionalLike<'a, '_> {
390393
space(),
391394
"extends",
392395
space(),
393-
conditional.extends_type()
396+
FormatNodeWithoutTrailingComments(conditional.extends_type())
394397
]
398+
)?;
399+
400+
format_trailing_comments(
401+
conditional.extends_type.span().end,
402+
conditional.true_type.span().start,
403+
b'?',
404+
f,
395405
)
396406
}
397407
});
@@ -411,53 +421,60 @@ impl<'a> FormatConditionalLike<'a, '_> {
411421
) -> FormatResult<()> {
412422
write!(f, [soft_line_break_or_space(), "?", space()])?;
413423

414-
let format_consequent = format_with(|f| match self.conditional {
415-
ConditionalLike::ConditionalExpression(conditional) => {
416-
let is_consequent_nested = match self.conditional {
424+
let format_consequent = format_with(|f| {
425+
let format_consequent_with_trailing_comments =
426+
format_once(|f| match self.conditional {
417427
ConditionalLike::ConditionalExpression(conditional) => {
418-
matches!(conditional.consequent, Expression::ConditionalExpression(_))
428+
write!(f, FormatNodeWithoutTrailingComments(conditional.consequent()))?;
429+
format_trailing_comments(
430+
conditional.consequent.span().end,
431+
conditional.alternate.span().start,
432+
b':',
433+
f,
434+
)
419435
}
420436
ConditionalLike::TSConditionalType(conditional) => {
421-
matches!(conditional.true_type, TSType::TSConditionalType(_))
422-
}
423-
};
424-
425-
let format_consequent = format_once(|f| {
426-
write!(f, FormatExpressionWithoutTrailingComments(conditional.consequent()))?;
427-
format_trailing_comments(
428-
conditional.consequent.span().end,
429-
conditional.alternate.span().start,
430-
b':',
431-
f,
432-
)
433-
});
434-
435-
let format_consequent = format_with(|f| {
436-
if f.options().indent_style.is_space() {
437-
write!(f, [align(2, &format_consequent)])
438-
} else {
439-
write!(f, [indent(&format_consequent)])
437+
write!(f, FormatNodeWithoutTrailingComments(conditional.true_type()))?;
438+
format_trailing_comments(
439+
conditional.true_type.span().end,
440+
conditional.false_type.span().start,
441+
b':',
442+
f,
443+
)
440444
}
441445
});
442446

443-
if is_consequent_nested {
444-
// Add parentheses around the consequent if it is a conditional expression and fits on the same line
445-
// so that it's easier to identify the parts that belong to a conditional expression.
446-
// `a ? b ? c: d : e` -> `a ? (b ? c: d) : e`
447-
write!(
448-
f,
449-
[
450-
if_group_fits_on_line(&text("(")),
451-
format_consequent,
452-
if_group_fits_on_line(&text(")"))
453-
]
454-
)
447+
let format_consequent_with_proper_indentation = format_with(|f| {
448+
if f.options().indent_style.is_space() {
449+
write!(f, [align(2, &format_consequent_with_trailing_comments)])
455450
} else {
456-
write!(f, format_consequent)
451+
write!(f, [indent(&format_consequent_with_trailing_comments)])
457452
}
458-
}
459-
ConditionalLike::TSConditionalType(conditional) => {
460-
write!(f, [conditional.true_type()])
453+
});
454+
455+
let is_nested_consequent = match self.conditional {
456+
ConditionalLike::ConditionalExpression(conditional) => {
457+
matches!(conditional.consequent, Expression::ConditionalExpression(_))
458+
}
459+
ConditionalLike::TSConditionalType(conditional) => {
460+
matches!(conditional.true_type, TSType::TSConditionalType(_))
461+
}
462+
};
463+
464+
if is_nested_consequent {
465+
// Add parentheses around the consequent if it is a conditional expression and fits on the same line
466+
// so that it's easier to identify the parts that belong to a conditional expression.
467+
// `a ? b ? c: d : e` -> `a ? (b ? c: d) : e`
468+
write!(
469+
f,
470+
[
471+
if_group_fits_on_line(&text("(")),
472+
format_consequent_with_proper_indentation,
473+
if_group_fits_on_line(&text(")"))
474+
]
475+
)
476+
} else {
477+
write!(f, format_consequent_with_proper_indentation)
461478
}
462479
});
463480

@@ -662,7 +679,7 @@ impl<'a> Format<'a> for FormatJsxChainExpression<'a, '_> {
662679
}
663680
.fmt(f)
664681
} else {
665-
FormatExpressionWithoutTrailingComments(self.expression).fmt(f)
682+
FormatNodeWithoutTrailingComments(self.expression).fmt(f)
666683
}
667684
});
668685

0 commit comments

Comments
 (0)