Skip to content

Commit 902ba3c

Browse files
DunqingDunqing
committed
fix(formatter): no need to wrap with parentheses for a type cast node
1 parent da01dd7 commit 902ba3c

File tree

5 files changed

+199
-28
lines changed

5 files changed

+199
-28
lines changed

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ pub struct Comments<'a> {
127127
printed_count: usize,
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.
130-
handled_type_cast_comment: usize,
130+
last_handled_type_cast_comment: usize,
131+
type_cast_node_span: Span,
131132
/// Optional limit for the unprinted_comments view.
132133
///
133134
/// When set, [`Self::unprinted_comments()`] will only return comments up to this index,
@@ -141,7 +142,8 @@ impl<'a> Comments<'a> {
141142
source_text,
142143
comments,
143144
printed_count: 0,
144-
handled_type_cast_comment: 0,
145+
last_handled_type_cast_comment: 0,
146+
type_cast_node_span: Span::default(),
145147
view_limit: None,
146148
}
147149
}
@@ -505,14 +507,20 @@ impl<'a> Comments<'a> {
505507
)
506508
}
507509

508-
/// Marks the most recently printed type cast comment as handled.
509-
pub fn mark_as_handled_type_cast_comment(&mut self) {
510-
self.handled_type_cast_comment = self.printed_count;
510+
/// Marks the given span as a type cast node.
511+
pub fn mark_as_type_cast_node(&mut self, node: &impl GetSpan) {
512+
self.type_cast_node_span = node.span();
513+
self.last_handled_type_cast_comment = self.printed_count;
511514
}
512515

513516
/// Checks if the most recently printed type cast comment has been handled.
514-
pub fn is_already_handled_type_cast_comment(&self) -> bool {
515-
self.printed_count == self.handled_type_cast_comment
517+
pub fn is_handled_type_cast_comment(&self) -> bool {
518+
self.printed_count == self.last_handled_type_cast_comment
519+
}
520+
521+
#[inline]
522+
pub fn is_type_cast_node(&self, node: &impl GetSpan) -> bool {
523+
self.type_cast_node_span == node.span()
516524
}
517525

518526
/// Temporarily limits the unprinted comments view to only those before the given position.

‎crates/oxc_formatter/src/parentheses/expression.rs‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Expression<'a>> {
7373

7474
impl<'a> NeedsParentheses<'a> for AstNode<'a, IdentifierReference<'a>> {
7575
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
76+
if f.comments().is_type_cast_node(self) {
77+
return false;
78+
}
79+
7680
match self.name.as_str() {
7781
"async" => {
7882
matches!(self.parent, AstNodes::ForOfStatement(stmt) if !stmt.r#await && stmt.left.span().contains_inclusive(self.span))
@@ -165,6 +169,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Super> {
165169

166170
impl<'a> NeedsParentheses<'a> for AstNode<'a, NumericLiteral<'a>> {
167171
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
172+
if f.comments().is_type_cast_node(self) {
173+
return false;
174+
}
175+
168176
if let AstNodes::StaticMemberExpression(member) = self.parent {
169177
return member.object.without_parentheses().span() == self.span();
170178
}
@@ -174,6 +182,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, NumericLiteral<'a>> {
174182

175183
impl<'a> NeedsParentheses<'a> for AstNode<'a, StringLiteral<'a>> {
176184
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
185+
if f.comments().is_type_cast_node(self) {
186+
return false;
187+
}
188+
177189
if let AstNodes::ExpressionStatement(stmt) = self.parent {
178190
// `() => "foo"`
179191
if let AstNodes::FunctionBody(arrow) = stmt.parent {
@@ -207,6 +219,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ArrayExpression<'a>> {
207219

208220
impl<'a> NeedsParentheses<'a> for AstNode<'a, ObjectExpression<'a>> {
209221
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
222+
if f.comments().is_type_cast_node(self) {
223+
return false;
224+
}
225+
210226
let parent = self.parent;
211227
is_class_extends(self.span, parent)
212228
|| is_first_in_statement(
@@ -239,6 +255,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ComputedMemberExpression<'a>> {
239255

240256
impl<'a> NeedsParentheses<'a> for AstNode<'a, StaticMemberExpression<'a>> {
241257
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
258+
if f.comments().is_type_cast_node(self) {
259+
return false;
260+
}
261+
242262
matches!(self.parent, AstNodes::NewExpression(_)) && {
243263
ExpressionLeftSide::Expression(self.object()).iter().any(|expr| {
244264
matches!(expr, ExpressionLeftSide::Expression(e) if
@@ -258,6 +278,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, PrivateFieldExpression<'a>> {
258278

259279
impl<'a> NeedsParentheses<'a> for AstNode<'a, CallExpression<'a>> {
260280
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
281+
if f.comments().is_type_cast_node(self) {
282+
return false;
283+
}
284+
261285
match self.parent {
262286
AstNodes::NewExpression(_) => true,
263287
AstNodes::ExportDefaultDeclaration(_) => {
@@ -280,12 +304,20 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, CallExpression<'a>> {
280304

281305
impl<'a> NeedsParentheses<'a> for AstNode<'a, NewExpression<'a>> {
282306
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
307+
if f.comments().is_type_cast_node(self) {
308+
return false;
309+
}
310+
283311
is_class_extends(self.span, self.parent)
284312
}
285313
}
286314

287315
impl<'a> NeedsParentheses<'a> for AstNode<'a, UpdateExpression<'a>> {
288316
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
317+
if f.comments().is_type_cast_node(self) {
318+
return false;
319+
}
320+
289321
let parent = self.parent;
290322
if self.prefix()
291323
&& let AstNodes::UnaryExpression(unary) = parent
@@ -303,6 +335,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, UpdateExpression<'a>> {
303335

304336
impl<'a> NeedsParentheses<'a> for AstNode<'a, UnaryExpression<'a>> {
305337
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
338+
if f.comments().is_type_cast_node(self) {
339+
return false;
340+
}
341+
306342
let parent = self.parent;
307343
match parent {
308344
AstNodes::UnaryExpression(parent_unary) => {
@@ -323,6 +359,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, UnaryExpression<'a>> {
323359

324360
impl<'a> NeedsParentheses<'a> for AstNode<'a, BinaryExpression<'a>> {
325361
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
362+
if f.comments().is_type_cast_node(self) {
363+
return false;
364+
}
365+
326366
(self.operator.is_in() && is_in_for_initializer(self))
327367
|| binary_like_needs_parens(BinaryLikeExpression::BinaryExpression(self))
328368
}
@@ -371,12 +411,20 @@ fn is_in_for_initializer(expr: &AstNode<'_, BinaryExpression<'_>>) -> bool {
371411
impl<'a> NeedsParentheses<'a> for AstNode<'a, PrivateInExpression<'a>> {
372412
#[inline]
373413
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
414+
if f.comments().is_type_cast_node(self) {
415+
return false;
416+
}
417+
374418
is_class_extends(self.span, self.parent)
375419
}
376420
}
377421

378422
impl<'a> NeedsParentheses<'a> for AstNode<'a, LogicalExpression<'a>> {
379423
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
424+
if f.comments().is_type_cast_node(self) {
425+
return false;
426+
}
427+
380428
let parent = self.parent;
381429
if let AstNodes::LogicalExpression(parent) = parent {
382430
parent.operator() != self.operator()
@@ -392,6 +440,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, LogicalExpression<'a>> {
392440

393441
impl<'a> NeedsParentheses<'a> for AstNode<'a, ConditionalExpression<'a>> {
394442
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
443+
if f.comments().is_type_cast_node(self) {
444+
return false;
445+
}
446+
395447
let parent = self.parent;
396448
if matches!(
397449
parent,
@@ -420,6 +472,11 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Function<'a>> {
420472
if self.r#type() != FunctionType::FunctionExpression {
421473
return false;
422474
}
475+
476+
if f.comments().is_type_cast_node(self) {
477+
return false;
478+
}
479+
423480
let parent = self.parent;
424481
matches!(
425482
parent,
@@ -436,6 +493,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Function<'a>> {
436493

437494
impl<'a> NeedsParentheses<'a> for AstNode<'a, AssignmentExpression<'a>> {
438495
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
496+
if f.comments().is_type_cast_node(self) {
497+
return false;
498+
}
499+
439500
match self.parent {
440501
// Expression statements, only object destructuring needs parens:
441502
// - `a = b` = no parens
@@ -519,6 +580,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, AssignmentExpression<'a>> {
519580

520581
impl<'a> NeedsParentheses<'a> for AstNode<'a, SequenceExpression<'a>> {
521582
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
583+
if f.comments().is_type_cast_node(self) {
584+
return false;
585+
}
586+
522587
!matches!(
523588
self.parent,
524589
AstNodes::ReturnStatement(_)
@@ -534,12 +599,20 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, SequenceExpression<'a>> {
534599

535600
impl<'a> NeedsParentheses<'a> for AstNode<'a, AwaitExpression<'a>> {
536601
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
602+
if f.comments().is_type_cast_node(self) {
603+
return false;
604+
}
605+
537606
await_or_yield_needs_parens(self.span(), self.parent)
538607
}
539608
}
540609

541610
impl<'a> NeedsParentheses<'a> for AstNode<'a, ChainExpression<'a>> {
542611
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
612+
if f.comments().is_type_cast_node(self) {
613+
return false;
614+
}
615+
543616
match self.parent {
544617
AstNodes::NewExpression(_) => true,
545618
AstNodes::CallExpression(call) => !call.optional,
@@ -557,6 +630,11 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Class<'a>> {
557630
if self.r#type() != ClassType::ClassExpression {
558631
return false;
559632
}
633+
634+
if f.comments().is_type_cast_node(self) {
635+
return false;
636+
}
637+
560638
let parent = self.parent;
561639
match parent {
562640
AstNodes::CallExpression(_)
@@ -583,6 +661,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ParenthesizedExpression<'a>> {
583661

584662
impl<'a> NeedsParentheses<'a> for AstNode<'a, ArrowFunctionExpression<'a>> {
585663
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
664+
if f.comments().is_type_cast_node(self) {
665+
return false;
666+
}
667+
586668
let parent = self.parent;
587669
if matches!(
588670
parent,
@@ -606,6 +688,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ArrowFunctionExpression<'a>> {
606688

607689
impl<'a> NeedsParentheses<'a> for AstNode<'a, YieldExpression<'a>> {
608690
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
691+
if f.comments().is_type_cast_node(self) {
692+
return false;
693+
}
694+
609695
let parent = self.parent;
610696
matches!(parent, AstNodes::AwaitExpression(_) | AstNodes::TSTypeAssertion(_))
611697
|| await_or_yield_needs_parens(self.span(), parent)
@@ -614,6 +700,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, YieldExpression<'a>> {
614700

615701
impl<'a> NeedsParentheses<'a> for AstNode<'a, ImportExpression<'a>> {
616702
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
703+
if f.comments().is_type_cast_node(self) {
704+
return false;
705+
}
706+
617707
matches!(self.parent, AstNodes::NewExpression(_))
618708
}
619709
}
@@ -1022,12 +1112,20 @@ fn jsx_element_or_fragment_needs_paren(span: Span, parent: &AstNodes<'_>) -> boo
10221112

10231113
impl NeedsParentheses<'_> for AstNode<'_, JSXElement<'_>> {
10241114
fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool {
1115+
if f.comments().is_type_cast_node(self) {
1116+
return false;
1117+
}
1118+
10251119
jsx_element_or_fragment_needs_paren(self.span, self.parent)
10261120
}
10271121
}
10281122

10291123
impl NeedsParentheses<'_> for AstNode<'_, JSXFragment<'_>> {
10301124
fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool {
1125+
if f.comments().is_type_cast_node(self) {
1126+
return false;
1127+
}
1128+
10311129
jsx_element_or_fragment_needs_paren(self.span, self.parent)
10321130
}
10331131
}

‎crates/oxc_formatter/src/utils/typecast.rs‎

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,39 @@ pub fn format_type_cast_comment_node<'a, T>(
3939
let source = f.source_text();
4040

4141
if !source.next_non_whitespace_byte_is(span.end, b')') {
42-
return Ok(false);
42+
let comments_after_node = comments.comments_after(span.end);
43+
let mut start = span.end;
44+
// Skip comments after the node to find the next non-whitespace byte whether it's a `)`
45+
for comment in comments_after_node {
46+
if !source.all_bytes_match(start, comment.span.start, |b| b.is_ascii_whitespace()) {
47+
break;
48+
}
49+
start = comment.span.end;
50+
}
51+
// Still not a `)`, return early because it's not a type cast
52+
if !source.next_non_whitespace_byte_is(start, b')') {
53+
return Ok(false);
54+
}
4355
}
4456

45-
if let Some(type_cast_comment_index) = comments.get_type_cast_comment_index(span) {
57+
if !comments.is_handled_type_cast_comment()
58+
&& let Some(last_printed_comment) = comments.printed_comments().last()
59+
&& last_printed_comment.span.end <= span.start
60+
&& f.source_text().next_non_whitespace_byte_is(last_printed_comment.span.end, b'(')
61+
&& f.comments().is_type_cast_comment(last_printed_comment)
62+
{
63+
// Get the source text from the end of type cast comment to the node span
64+
let node_source_text = source.bytes_range(last_printed_comment.span.end, span.end);
65+
66+
// `(/** @type {Number} */ (bar).zoo)`
67+
// ^^^^
68+
// Should wrap for `baz` rather than `baz.zoo`
69+
if has_closed_parentheses(node_source_text) {
70+
return Ok(false);
71+
}
72+
73+
f.context_mut().comments_mut().mark_as_type_cast_node(node);
74+
} else if let Some(type_cast_comment_index) = comments.get_type_cast_comment_index(span) {
4675
let comments = f.context().comments().unprinted_comments();
4776
let type_cast_comment = &comments[type_cast_comment_index];
4877

@@ -59,25 +88,11 @@ pub fn format_type_cast_comment_node<'a, T>(
5988
let type_cast_comments = &comments[..=type_cast_comment_index];
6089

6190
write!(f, [FormatLeadingComments::Comments(type_cast_comments)])?;
62-
f.context_mut().comments_mut().mark_as_handled_type_cast_comment();
63-
} else {
64-
let elements = f.elements().iter().rev();
65-
91+
f.context_mut().comments_mut().mark_as_type_cast_node(node);
6692
// If the printed cast comment is already handled, return early to avoid infinite recursion.
67-
if !comments.is_already_handled_type_cast_comment()
68-
&& comments.printed_comments().last().is_some_and(|c| {
69-
c.span.end <= span.start
70-
&& source.all_bytes_match(c.span.end, span.start, |c| {
71-
c.is_ascii_whitespace() || c == b'('
72-
})
73-
&& f.comments().is_type_cast_comment(c)
74-
})
75-
{
76-
f.context_mut().comments_mut().mark_as_handled_type_cast_comment();
77-
} else {
78-
// No typecast comment
79-
return Ok(false);
80-
}
93+
} else {
94+
// No typecast comment
95+
return Ok(false);
8196
}
8297

8398
// https://github.com/prettier/prettier/blob/7584432401a47a26943dd7a9ca9a8e032ead7285/src/language-js/print/estree.js#L117-L120
@@ -102,7 +117,12 @@ fn has_closed_parentheses(source: &[u8]) -> bool {
102117
while i < source.len() {
103118
match source[i] {
104119
b'(' => paren_count += 1,
105-
b')' => paren_count -= 1,
120+
b')' => {
121+
paren_count -= 1;
122+
if paren_count == 0 {
123+
return true;
124+
}
125+
}
106126
b'/' if i + 1 < source.len() => {
107127
match source[i + 1] {
108128
b'/' => {

0 commit comments

Comments
 (0)