Skip to content

fix(formatter): no need to wrap with parentheses for a type cast node #14484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 12, 2025
Merged

fix(formatter): no need to wrap with parentheses for a type cast node #14484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions crates/oxc_formatter/src/formatter/comments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ pub struct Comments<'a> {
printed_count: usize,
/// The index of the type cast comment that has been printed already.
/// Used to prevent duplicate processing of special TypeScript type cast comments.
handled_type_cast_comment: usize,
last_handled_type_cast_comment: usize,
type_cast_node_span: Span,
/// Optional limit for the unprinted_comments view.
///
/// When set, [`Self::unprinted_comments()`] will only return comments up to this index,
Expand All @@ -141,7 +142,8 @@ impl<'a> Comments<'a> {
source_text,
comments,
printed_count: 0,
handled_type_cast_comment: 0,
last_handled_type_cast_comment: 0,
type_cast_node_span: Span::default(),
view_limit: None,
}
}
Expand Down Expand Up @@ -505,14 +507,20 @@ impl<'a> Comments<'a> {
)
}

/// Marks the most recently printed type cast comment as handled.
pub fn mark_as_handled_type_cast_comment(&mut self) {
self.handled_type_cast_comment = self.printed_count;
/// Marks the given span as a type cast node.
pub fn mark_as_type_cast_node(&mut self, node: &impl GetSpan) {
self.type_cast_node_span = node.span();
self.last_handled_type_cast_comment = self.printed_count;
}

/// Checks if the most recently printed type cast comment has been handled.
pub fn is_already_handled_type_cast_comment(&self) -> bool {
self.printed_count == self.handled_type_cast_comment
pub fn is_handled_type_cast_comment(&self) -> bool {
self.printed_count == self.last_handled_type_cast_comment
}

#[inline]
pub fn is_type_cast_node(&self, node: &impl GetSpan) -> bool {
self.type_cast_node_span == node.span()
}

/// Temporarily limits the unprinted comments view to only those before the given position.
Expand Down
98 changes: 98 additions & 0 deletions crates/oxc_formatter/src/parentheses/expression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Expression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, IdentifierReference<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

match self.name.as_str() {
"async" => {
matches!(self.parent, AstNodes::ForOfStatement(stmt) if !stmt.r#await && stmt.left.span().contains_inclusive(self.span))
Expand Down Expand Up @@ -165,6 +169,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Super> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, NumericLiteral<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

if let AstNodes::StaticMemberExpression(member) = self.parent {
return member.object.without_parentheses().span() == self.span();
}
Expand All @@ -174,6 +182,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, NumericLiteral<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, StringLiteral<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

if let AstNodes::ExpressionStatement(stmt) = self.parent {
// `() => "foo"`
if let AstNodes::FunctionBody(arrow) = stmt.parent {
Expand Down Expand Up @@ -207,6 +219,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ArrayExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, ObjectExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
is_class_extends(self.span, parent)
|| is_first_in_statement(
Expand Down Expand Up @@ -239,6 +255,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ComputedMemberExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, StaticMemberExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

matches!(self.parent, AstNodes::NewExpression(_)) && {
ExpressionLeftSide::Expression(self.object()).iter().any(|expr| {
matches!(expr, ExpressionLeftSide::Expression(e) if
Expand All @@ -258,6 +278,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, PrivateFieldExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, CallExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

match self.parent {
AstNodes::NewExpression(_) => true,
AstNodes::ExportDefaultDeclaration(_) => {
Expand All @@ -280,12 +304,20 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, CallExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, NewExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

is_class_extends(self.span, self.parent)
}
}

impl<'a> NeedsParentheses<'a> for AstNode<'a, UpdateExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
if self.prefix()
&& let AstNodes::UnaryExpression(unary) = parent
Expand All @@ -303,6 +335,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, UpdateExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, UnaryExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
match parent {
AstNodes::UnaryExpression(parent_unary) => {
Expand All @@ -323,6 +359,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, UnaryExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, BinaryExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

(self.operator.is_in() && is_in_for_initializer(self))
|| binary_like_needs_parens(BinaryLikeExpression::BinaryExpression(self))
}
Expand Down Expand Up @@ -371,12 +411,20 @@ fn is_in_for_initializer(expr: &AstNode<'_, BinaryExpression<'_>>) -> bool {
impl<'a> NeedsParentheses<'a> for AstNode<'a, PrivateInExpression<'a>> {
#[inline]
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

is_class_extends(self.span, self.parent)
}
}

impl<'a> NeedsParentheses<'a> for AstNode<'a, LogicalExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
if let AstNodes::LogicalExpression(parent) = parent {
parent.operator() != self.operator()
Expand All @@ -392,6 +440,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, LogicalExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, ConditionalExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
if matches!(
parent,
Expand Down Expand Up @@ -420,6 +472,11 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Function<'a>> {
if self.r#type() != FunctionType::FunctionExpression {
return false;
}

if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
matches!(
parent,
Expand All @@ -436,6 +493,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Function<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, AssignmentExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

match self.parent {
// Expression statements, only object destructuring needs parens:
// - `a = b` = no parens
Expand Down Expand Up @@ -519,6 +580,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, AssignmentExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, SequenceExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

!matches!(
self.parent,
AstNodes::ReturnStatement(_)
Expand All @@ -534,12 +599,20 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, SequenceExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, AwaitExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

await_or_yield_needs_parens(self.span(), self.parent)
}
}

impl<'a> NeedsParentheses<'a> for AstNode<'a, ChainExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

match self.parent {
AstNodes::NewExpression(_) => true,
AstNodes::CallExpression(call) => !call.optional,
Expand All @@ -557,6 +630,11 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, Class<'a>> {
if self.r#type() != ClassType::ClassExpression {
return false;
}

if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
match parent {
AstNodes::CallExpression(_)
Expand All @@ -583,6 +661,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ParenthesizedExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, ArrowFunctionExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
if matches!(
parent,
Expand All @@ -606,6 +688,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, ArrowFunctionExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, YieldExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

let parent = self.parent;
matches!(parent, AstNodes::AwaitExpression(_) | AstNodes::TSTypeAssertion(_))
|| await_or_yield_needs_parens(self.span(), parent)
Expand All @@ -614,6 +700,10 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, YieldExpression<'a>> {

impl<'a> NeedsParentheses<'a> for AstNode<'a, ImportExpression<'a>> {
fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

matches!(self.parent, AstNodes::NewExpression(_))
}
}
Expand Down Expand Up @@ -1022,12 +1112,20 @@ fn jsx_element_or_fragment_needs_paren(span: Span, parent: &AstNodes<'_>) -> boo

impl NeedsParentheses<'_> for AstNode<'_, JSXElement<'_>> {
fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

jsx_element_or_fragment_needs_paren(self.span, self.parent)
}
}

impl NeedsParentheses<'_> for AstNode<'_, JSXFragment<'_>> {
fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool {
if f.comments().is_type_cast_node(self) {
return false;
}

jsx_element_or_fragment_needs_paren(self.span, self.parent)
}
}
62 changes: 41 additions & 21 deletions crates/oxc_formatter/src/utils/typecast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,39 @@ pub fn format_type_cast_comment_node<'a, T>(
let source = f.source_text();

if !source.next_non_whitespace_byte_is(span.end, b')') {
return Ok(false);
let comments_after_node = comments.comments_after(span.end);
let mut start = span.end;
// Skip comments after the node to find the next non-whitespace byte whether it's a `)`
for comment in comments_after_node {
if !source.all_bytes_match(start, comment.span.start, |b| b.is_ascii_whitespace()) {
break;
}
start = comment.span.end;
}
// Still not a `)`, return early because it's not a type cast
if !source.next_non_whitespace_byte_is(start, b')') {
return Ok(false);
}
}

if let Some(type_cast_comment_index) = comments.get_type_cast_comment_index(span) {
if !comments.is_handled_type_cast_comment()
&& let Some(last_printed_comment) = comments.printed_comments().last()
&& last_printed_comment.span.end <= span.start
&& f.source_text().next_non_whitespace_byte_is(last_printed_comment.span.end, b'(')
&& f.comments().is_type_cast_comment(last_printed_comment)
{
// Get the source text from the end of type cast comment to the node span
let node_source_text = source.bytes_range(last_printed_comment.span.end, span.end);

// `(/** @type {Number} */ (bar).zoo)`
// ^^^^
// Should wrap for `baz` rather than `baz.zoo`
if has_closed_parentheses(node_source_text) {
return Ok(false);
}

f.context_mut().comments_mut().mark_as_type_cast_node(node);
} else if let Some(type_cast_comment_index) = comments.get_type_cast_comment_index(span) {
let comments = f.context().comments().unprinted_comments();
let type_cast_comment = &comments[type_cast_comment_index];

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

write!(f, [FormatLeadingComments::Comments(type_cast_comments)])?;
f.context_mut().comments_mut().mark_as_handled_type_cast_comment();
} else {
let elements = f.elements().iter().rev();

f.context_mut().comments_mut().mark_as_type_cast_node(node);
// If the printed cast comment is already handled, return early to avoid infinite recursion.
if !comments.is_already_handled_type_cast_comment()
&& comments.printed_comments().last().is_some_and(|c| {
c.span.end <= span.start
&& source.all_bytes_match(c.span.end, span.start, |c| {
c.is_ascii_whitespace() || c == b'('
})
&& f.comments().is_type_cast_comment(c)
})
{
f.context_mut().comments_mut().mark_as_handled_type_cast_comment();
} else {
// No typecast comment
return Ok(false);
}
} else {
// No typecast comment
return Ok(false);
}

// https://github.com/prettier/prettier/blob/7584432401a47a26943dd7a9ca9a8e032ead7285/src/language-js/print/estree.js#L117-L120
Expand All @@ -102,7 +117,12 @@ fn has_closed_parentheses(source: &[u8]) -> bool {
while i < source.len() {
match source[i] {
b'(' => paren_count += 1,
b')' => paren_count -= 1,
b')' => {
paren_count -= 1;
if paren_count == 0 {
return true;
}
}
b'/' if i + 1 < source.len() => {
match source[i + 1] {
b'/' => {
Expand Down
Loading
Loading