diff --git a/crates/oxc_formatter/src/parentheses/expression.rs b/crates/oxc_formatter/src/parentheses/expression.rs index 0236f6c96fae3..e4b2f0aa5c32a 100644 --- a/crates/oxc_formatter/src/parentheses/expression.rs +++ b/crates/oxc_formatter/src/parentheses/expression.rs @@ -77,18 +77,52 @@ impl NeedsParentheses<'_> for AstNode<'_, IdentifierReference<'_>> { matches!(self.parent, AstNodes::ForOfStatement(stmt) if !stmt.r#await && stmt.left.span().contains_inclusive(self.span)) } "let" => { - // Walk up ancestors to find the relevant context for `let` keyword + // `let[a]` at statement start looks like a lexical declaration, needs parens + // Only applies when `let` is the object of a computed member expression + if !matches!(self.parent, AstNodes::ComputedMemberExpression(m) if m.object.span() == self.span()) + { + // Not `let[...]` - check special cases only + return self.ancestors().any(|parent| match parent { + AstNodes::ForOfStatement(s) => s.left.span().contains_inclusive(self.span), + AstNodes::ForInStatement(s) => { + s.left.span().contains_inclusive(self.span) + && !matches!(self.parent, AstNodes::StaticMemberExpression(_)) + } + AstNodes::TSSatisfiesExpression(e) => e.expression.span() == self.span(), + _ => false, + }); + } + + // Check if `let[...]` is at the leftmost position of a statement + let mut child_span = self.span; for parent in self.ancestors() { - match parent { - AstNodes::ExpressionStatement(_) => return false, - AstNodes::ForOfStatement(stmt) => { - return stmt.left.span().contains_inclusive(self.span); + let dominated = match parent { + AstNodes::ExpressionStatement(s) => return !s.is_arrow_function_body(), + AstNodes::ForStatement(_) => return true, + AstNodes::ForOfStatement(s) => { + return s.left.span().contains_inclusive(self.span); + } + AstNodes::ForInStatement(s) => { + return s.left.span().contains_inclusive(self.span); } - AstNodes::TSSatisfiesExpression(expr) => { - return expr.expression.span() == self.span(); + AstNodes::ComputedMemberExpression(m) => m.object.span() == child_span, + AstNodes::StaticMemberExpression(m) => m.object.span() == child_span, + AstNodes::CallExpression(c) => c.callee.span() == child_span, + AstNodes::ChainExpression(c) => c.expression.span() == child_span, + AstNodes::AssignmentExpression(a) => a.left.span() == child_span, + AstNodes::BinaryExpression(b) => b.left.span() == child_span, + AstNodes::LogicalExpression(l) => l.left.span() == child_span, + AstNodes::ConditionalExpression(c) => c.test.span() == child_span, + AstNodes::SequenceExpression(s) => { + s.expressions.first().is_some_and(|e| e.span() == child_span) } - _ => {} + AstNodes::TaggedTemplateExpression(t) => t.tag.span() == child_span, + _ => false, + }; + if !dominated { + return false; } + child_span = parent.span(); } false } diff --git a/crates/oxc_formatter/src/print/mod.rs b/crates/oxc_formatter/src/print/mod.rs index 620deee974591..ff6be7be1665c 100644 --- a/crates/oxc_formatter/src/print/mod.rs +++ b/crates/oxc_formatter/src/print/mod.rs @@ -480,6 +480,16 @@ fn expression_statement_needs_semicolon<'a>( return !can_avoid_parentheses(arrow, f); } + // Expressions starting with keywords don't need ASI protection + if matches!( + &stmt.expression, + Expression::NewExpression(_) + | Expression::AwaitExpression(_) + | Expression::YieldExpression(_) + ) { + return false; + } + // First check if the expression itself needs protection let expr = stmt.expression(); @@ -631,19 +641,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ForStatement<'a>> { let body = self.body(); let format_body = FormatStatementBody::new(body); if init.is_none() && test.is_none() && update.is_none() { - let comments = f.context().comments().comments_before(body.span().start); - if !comments.is_empty() { - write!( - f, - [ - FormatDanglingComments::Comments { - comments, - indent: DanglingIndentMode::None - }, - soft_line_break_or_space() - ] - ); - } return write!(f, [group(&format_args!("for", space(), "(;;)", format_body))]); } @@ -725,7 +722,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ForOfStatement<'a>> { "of", space(), right, - FormatCommentForEmptyStatement(body), ")", FormatStatementBody::new(body) ] diff --git a/crates/oxc_formatter/src/print/return_or_throw_statement.rs b/crates/oxc_formatter/src/print/return_or_throw_statement.rs index a7d8768b5a683..b3d4e9c4e883d 100644 --- a/crates/oxc_formatter/src/print/return_or_throw_statement.rs +++ b/crates/oxc_formatter/src/print/return_or_throw_statement.rs @@ -89,7 +89,24 @@ impl<'a> Format<'a> for FormatAdjacentArgument<'a, '_> { let argument = self.0; if !argument.is_jsx() && has_argument_leading_comments(argument, f) { - write!(f, [token("("), &block_indent(&argument), token(")")]); + // When we have leading comments and a sequence expression, we need inner parentheses + // e.g. `return ( // comment\n a, b )` -> `return (\n // comment\n (a, b)\n)` + let inner = format_with(|f| { + if matches!(argument.as_ref(), Expression::SequenceExpression(_)) { + write!( + f, + [ + format_leading_comments(argument.span()), + token("("), + argument, + token(")") + ] + ); + } else { + write!(f, argument); + } + }); + write!(f, [token("("), &block_indent(&inner), token(")")]); } else if argument.is_binaryish() { write!( f, diff --git a/crates/oxc_formatter/src/utils/statement_body.rs b/crates/oxc_formatter/src/utils/statement_body.rs index 256647b09b382..37ec54ba80421 100644 --- a/crates/oxc_formatter/src/utils/statement_body.rs +++ b/crates/oxc_formatter/src/utils/statement_body.rs @@ -34,6 +34,12 @@ impl<'a, 'b> FormatStatementBody<'a, 'b> { impl<'a> Format<'a> for FormatStatementBody<'a, '_> { fn fmt(&self, f: &mut Formatter<'_, 'a>) { if let AstNodes::EmptyStatement(empty) = self.body.as_ast_nodes() { + // Add space before empty statement if it has leading comments + // e.g., `for (x of y) /*comment*/ ;` + let has_leading_comments = f.context().comments().has_comment_before(empty.span.start); + if has_leading_comments { + write!(f, [space()]); + } write!(f, empty); } else if let AstNodes::BlockStatement(block) = self.body.as_ast_nodes() { write!(f, [space()]); diff --git a/tasks/coverage/snapshots/formatter_typescript.snap b/tasks/coverage/snapshots/formatter_typescript.snap index afb21c2cb8944..20ae206d3e8ba 100644 --- a/tasks/coverage/snapshots/formatter_typescript.snap +++ b/tasks/coverage/snapshots/formatter_typescript.snap @@ -2,7 +2,7 @@ commit: f5ccf434 formatter_typescript Summary: AST Parsed : 9845/9845 (100.00%) -Positive Passed: 9792/9845 (99.46%) +Positive Passed: 9793/9845 (99.47%) Mismatch: tasks/coverage/typescript/tests/cases/compiler/amdLikeInputDeclarationEmit.ts Mismatch: tasks/coverage/typescript/tests/cases/compiler/castExpressionParentheses.ts @@ -87,8 +87,6 @@ Mismatch: tasks/coverage/typescript/tests/cases/conformance/expressions/commaOpe Mismatch: tasks/coverage/typescript/tests/cases/conformance/expressions/contextualTyping/parenthesizedContexualTyping2.ts -Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/expressions/elementAccess/letIdentifierInElementAccess01.ts -Unexpected token Mismatch: tasks/coverage/typescript/tests/cases/conformance/expressions/optionalChaining/optionalChainingInTypeAssertions.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/generators/generatorAssignability.ts diff --git a/tasks/prettier_conformance/snapshots/prettier.js.snap.md b/tasks/prettier_conformance/snapshots/prettier.js.snap.md index edcfbb17875fa..c96b7c6776f16 100644 --- a/tasks/prettier_conformance/snapshots/prettier.js.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.js.snap.md @@ -1,4 +1,4 @@ -js compatibility: 737/761 (96.85%) +js compatibility: 742/761 (97.50%) # Failed @@ -6,18 +6,13 @@ js compatibility: 737/761 (96.85%) | :-------- | :--------------: | :---------: | | js/arrows/comment.js | 💥💥 | 88.89% | | js/comments/15661.js | 💥💥 | 55.17% | -| js/comments/dangling_for.js | 💥💥 | 22.22% | | js/comments/empty-statements.js | 💥💥 | 90.91% | | js/comments/function-declaration.js | 💥💥 | 92.80% | -| js/comments/return-statement.js | 💥💥 | 98.28% | -| js/explicit-resource-management/for-await-using-of-comments.js | 💥 | 0.00% | -| js/explicit-resource-management/valid-await-using-comments.js | 💥 | 68.57% | -| js/for/9812-unstable.js | 💥 | 45.45% | -| js/for/9812.js | 💥 | 82.83% | +| js/comments/return-statement.js | 💥💥 | 98.85% | +| js/explicit-resource-management/valid-await-using-comments.js | 💥 | 80.00% | +| js/for/9812-unstable.js | 💥 | 63.64% | | js/for/for-in-with-initializer.js | 💥 | 37.50% | | js/for/parentheses.js | 💥 | 97.96% | -| js/identifier/for-of/let.js | 💥 | 92.31% | -| js/identifier/parentheses/let.js | 💥💥 | 82.27% | | js/last-argument-expansion/dangling-comment-in-arrow-function.js | 💥 | 22.22% | | js/quote-props/objects.js | 💥💥✨✨ | 48.04% | | js/quote-props/with_numbers.js | 💥💥✨✨ | 46.43% |