feat(formatter): correctly handle semicolon

crates/oxc_formatter/examples/formatter.rs

@@ -8,19 +8,21 @@
 //! Create a `test.js` file and run:
 //! ```bash
 //! cargo run -p oxc_formatter --example formatter [filename]
+//! cargo run -p oxc_formatter --example formatter -- --no-semi [filename]
 //! ```
 
 use std::{fs, path::Path};
 
 use oxc_allocator::Allocator;
-use oxc_formatter::{BracketSameLine, FormatOptions, Formatter};
+use oxc_formatter::{BracketSameLine, FormatOptions, Formatter, Semicolons};
 use oxc_parser::{ParseOptions, Parser};
 use oxc_span::SourceType;
 use pico_args::Arguments;
 
 /// Format a JavaScript or TypeScript file
 fn main() -> Result<(), String> {
     let mut args = Arguments::from_env();
+    let no_semi = args.contains("--no-semi");
     let name = args.free_from_str().unwrap_or_else(|_| "test.js".to_string());
 
     // Read source file
@@ -46,8 +48,12 @@ fn main() -> Result<(), String> {
     }
 
     // Format the parsed code
-    let options =
-        FormatOptions { bracket_same_line: BracketSameLine::from(true), ..Default::default() };
+    let semicolons = if no_semi { Semicolons::AsNeeded } else { Semicolons::Always };
+    let options = FormatOptions {
+        bracket_same_line: BracketSameLine::from(true),
+        semicolons,
+        ..Default::default()
+    };
     let code = Formatter::new(&allocator, options).build(&ret.program);
 
     println!("{code}");

crates/oxc_formatter/src/parentheses/expression.rs

@@ -173,17 +173,16 @@ impl<'a> NeedsParentheses<'a> for AstNode<'a, CallExpression<'a>> {
             AstNodes::NewExpression(_) => true,
             AstNodes::Decorator(_) => !is_identifier_or_static_member_only(&self.callee),
             AstNodes::ExportDefaultDeclaration(_) => {
-                let callee = &self.callee;
+                let callee = &self.callee();
                 let callee_span = callee.span();
                 let leftmost = ExpressionLeftSide::leftmost(callee);
                 // require parens for iife and
                 // when the leftmost expression is not a class expression or a function expression
                 callee_span != leftmost.span()
                     && matches!(
                         leftmost,
-                        ExpressionLeftSide::Expression(
-                            Expression::ClassExpression(_) | Expression::FunctionExpression(_)
-                        )
+                        ExpressionLeftSide::Expression(e)
+                         if matches!(e.as_ref(), Expression::ClassExpression(_) | Expression::FunctionExpression(_))
                     )
             }
             _ => false,

crates/oxc_formatter/src/write/arrow_function_expression.rs

@@ -687,31 +687,31 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> {
 
 #[derive(Debug)]
 pub enum ExpressionLeftSide<'a, 'b> {
-    Expression(&'b Expression<'a>),
-    AssignmentTarget(&'b AssignmentTarget<'a>),
-    SimpleAssignmentTarget(&'b SimpleAssignmentTarget<'a>),
+    Expression(&'b AstNode<'a, Expression<'a>>),
+    AssignmentTarget(&'b AstNode<'a, AssignmentTarget<'a>>),
+    SimpleAssignmentTarget(&'b AstNode<'a, SimpleAssignmentTarget<'a>>),
 }
 
-impl<'a, 'b> From<&'b Expression<'a>> for ExpressionLeftSide<'a, 'b> {
-    fn from(value: &'b Expression<'a>) -> Self {
+impl<'a, 'b> From<&'b AstNode<'a, Expression<'a>>> for ExpressionLeftSide<'a, 'b> {
+    fn from(value: &'b AstNode<'a, Expression<'a>>) -> Self {
         Self::Expression(value)
     }
 }
 
-impl<'a, 'b> From<&'b AssignmentTarget<'a>> for ExpressionLeftSide<'a, 'b> {
-    fn from(value: &'b AssignmentTarget<'a>) -> Self {
+impl<'a, 'b> From<&'b AstNode<'a, AssignmentTarget<'a>>> for ExpressionLeftSide<'a, 'b> {
+    fn from(value: &'b AstNode<'a, AssignmentTarget<'a>>) -> Self {
         Self::AssignmentTarget(value)
     }
 }
 
-impl<'a, 'b> From<&'b SimpleAssignmentTarget<'a>> for ExpressionLeftSide<'a, 'b> {
-    fn from(value: &'b SimpleAssignmentTarget<'a>) -> Self {
+impl<'a, 'b> From<&'b AstNode<'a, SimpleAssignmentTarget<'a>>> for ExpressionLeftSide<'a, 'b> {
+    fn from(value: &'b AstNode<'a, SimpleAssignmentTarget<'a>>) -> Self {
         Self::SimpleAssignmentTarget(value)
     }
 }
 
 impl<'a, 'b> ExpressionLeftSide<'a, 'b> {
-    pub fn leftmost(expression: &'b Expression<'a>) -> Self {
+    pub fn leftmost(expression: &'b AstNode<'a, Expression<'a>>) -> Self {
         let mut current: Self = expression.into();
         loop {
             match current.left_expression() {
@@ -729,60 +729,46 @@ impl<'a, 'b> ExpressionLeftSide<'a, 'b> {
     /// if the expression has no left side.
     pub fn left_expression(&self) -> Option<Self> {
         match self {
-            Self::Expression(expression) => match expression {
-                Expression::SequenceExpression(expr) => expr.expressions.first().map(Into::into),
-                Expression::StaticMemberExpression(expr) => Some((&expr.object).into()),
-                Expression::ComputedMemberExpression(expr) => Some((&expr.object).into()),
-                Expression::PrivateFieldExpression(expr) => Some((&expr.object).into()),
-                Expression::TaggedTemplateExpression(expr) => Some((&expr.tag).into()),
-                Expression::NewExpression(expr) => Some((&expr.callee).into()),
-                Expression::CallExpression(expr) => Some((&expr.callee).into()),
-                Expression::ConditionalExpression(expr) => Some((&expr.test).into()),
-                Expression::TSAsExpression(expr) => Some((&expr.expression).into()),
-                Expression::TSSatisfiesExpression(expr) => Some((&expr.expression).into()),
-                Expression::TSNonNullExpression(expr) => Some((&expr.expression).into()),
-                Expression::AssignmentExpression(expr) => Some(Self::AssignmentTarget(&expr.left)),
-                Expression::UpdateExpression(expr) => {
+            Self::Expression(expression) => match expression.as_ast_nodes() {
+                AstNodes::SequenceExpression(expr) => expr.expressions().first().map(Into::into),
+                AstNodes::StaticMemberExpression(expr) => Some(expr.object().into()),
+                AstNodes::ComputedMemberExpression(expr) => Some(expr.object().into()),
+                AstNodes::PrivateFieldExpression(expr) => Some(expr.object().into()),
+                AstNodes::TaggedTemplateExpression(expr) => Some(expr.tag().into()),
+                AstNodes::NewExpression(expr) => Some(expr.callee().into()),
+                AstNodes::CallExpression(expr) => Some(expr.callee().into()),
+                AstNodes::ConditionalExpression(expr) => Some(expr.test().into()),
+                AstNodes::TSAsExpression(expr) => Some(expr.expression().into()),
+                AstNodes::TSSatisfiesExpression(expr) => Some(expr.expression().into()),
+                AstNodes::TSNonNullExpression(expr) => Some(expr.expression().into()),
+                AstNodes::AssignmentExpression(expr) => Some(Self::AssignmentTarget(expr.left())),
+                AstNodes::UpdateExpression(expr) => {
                     if expr.prefix {
                         None
                     } else {
-                        Some(Self::SimpleAssignmentTarget(&expr.argument))
+                        Some(Self::SimpleAssignmentTarget(expr.argument()))
                     }
                 }
-                Expression::BinaryExpression(binary) => Some((&binary.left).into()),
-                Expression::LogicalExpression(logical) => Some((&logical.left).into()),
-                Expression::ChainExpression(chain) => match &chain.expression {
-                    ChainElement::CallExpression(expr) => Some((&expr.callee).into()),
-                    ChainElement::TSNonNullExpression(expr) => Some((&expr.expression).into()),
-                    ChainElement::ComputedMemberExpression(expr) => Some((&expr.object).into()),
-                    ChainElement::StaticMemberExpression(expr) => Some((&expr.object).into()),
-                    ChainElement::PrivateFieldExpression(expr) => Some((&expr.object).into()),
+                AstNodes::BinaryExpression(binary) => Some(binary.left().into()),
+                AstNodes::LogicalExpression(logical) => Some(logical.left().into()),
+                AstNodes::ChainExpression(chain) => match &chain.expression().as_ast_nodes() {
+                    AstNodes::CallExpression(expr) => Some(expr.callee().into()),
+                    AstNodes::TSNonNullExpression(expr) => Some(expr.expression().into()),
+                    AstNodes::ComputedMemberExpression(expr) => Some(expr.object().into()),
+                    AstNodes::StaticMemberExpression(expr) => Some(expr.object().into()),
+                    AstNodes::PrivateFieldExpression(expr) => Some(expr.object().into()),
+                    _ => {
+                        unreachable!()
+                    }
                 },
                 _ => None,
             },
-            Self::AssignmentTarget(target) => match target {
-                match_simple_assignment_target!(AssignmentTarget) => {
-                    Self::SimpleAssignmentTarget(target.to_simple_assignment_target())
-                        .left_expression()
-                }
-                _ => None,
-            },
-            Self::SimpleAssignmentTarget(target) => match target {
-                SimpleAssignmentTarget::TSAsExpression(expr) => Some((&expr.expression).into()),
-                SimpleAssignmentTarget::TSSatisfiesExpression(expr) => {
-                    Some((&expr.expression).into())
-                }
-                SimpleAssignmentTarget::TSNonNullExpression(expr) => {
-                    Some((&expr.expression).into())
-                }
-                SimpleAssignmentTarget::TSTypeAssertion(expr) => Some((&expr.expression).into()),
-                SimpleAssignmentTarget::ComputedMemberExpression(expr) => {
-                    Some((&expr.object).into())
-                }
-                SimpleAssignmentTarget::StaticMemberExpression(expr) => Some((&expr.object).into()),
-                SimpleAssignmentTarget::PrivateFieldExpression(expr) => Some((&expr.object).into()),
-                SimpleAssignmentTarget::AssignmentTargetIdentifier(identifier_reference) => None,
-            },
+            Self::AssignmentTarget(target) => {
+                Self::get_left_side_of_assignment(target.as_ast_nodes())
+            }
+            Self::SimpleAssignmentTarget(target) => {
+                Self::get_left_side_of_assignment(target.as_ast_nodes())
+            }
         }
     }
 
@@ -793,10 +779,24 @@ impl<'a, 'b> ExpressionLeftSide<'a, 'b> {
             ExpressionLeftSide::SimpleAssignmentTarget(target) => target.span(),
         }
     }
+
+    fn get_left_side_of_assignment(node: &'b AstNodes<'a>) -> Option<ExpressionLeftSide<'a, 'b>> {
+        match node {
+            AstNodes::TSAsExpression(expr) => Some(expr.expression().into()),
+            AstNodes::TSSatisfiesExpression(expr) => Some(expr.expression().into()),
+            AstNodes::TSNonNullExpression(expr) => Some(expr.expression().into()),
+            AstNodes::TSTypeAssertion(expr) => Some(expr.expression().into()),
+            AstNodes::ComputedMemberExpression(expr) => Some(expr.object().into()),
+            AstNodes::StaticMemberExpression(expr) => Some(expr.object().into()),
+            AstNodes::PrivateFieldExpression(expr) => Some(expr.object().into()),
+            _ => None,
+        }
+    }
 }
 
-fn should_add_parens(body: &FunctionBody) -> bool {
-    let Statement::ExpressionStatement(stmt) = body.statements.first().unwrap() else {
+fn should_add_parens(body: &AstNode<'_, FunctionBody<'_>>) -> bool {
+    let AstNodes::ExpressionStatement(stmt) = body.statements().first().unwrap().as_ast_nodes()
+    else {
         unreachable!()
     };
 
@@ -805,11 +805,13 @@ fn should_add_parens(body: &FunctionBody) -> bool {
     // case and added by the object expression itself
     if matches!(&stmt.expression, Expression::ConditionalExpression(_)) {
         !matches!(
-            ExpressionLeftSide::leftmost(&stmt.expression),
+            ExpressionLeftSide::leftmost(stmt.expression()),
             ExpressionLeftSide::Expression(
+                e
+            ) if matches!(e.as_ref(),
                 Expression::ObjectExpression(_)
-                    | Expression::FunctionExpression(_)
-                    | Expression::ClassExpression(_)
+                | Expression::FunctionExpression(_)
+                | Expression::ClassExpression(_)
             )
         )
     } else {

crates/oxc_formatter/src/write/mod.rs

@@ -53,7 +53,7 @@ use crate::{
         },
     },
     generated::ast_nodes::{AstNode, AstNodes},
-    options::{FormatTrailingCommas, QuoteProperties, TrailingSeparator},
+    options::{FormatTrailingCommas, QuoteProperties, Semicolons, TrailingSeparator},
     parentheses::NeedsParentheses,
     utils::{
         assignment_like::AssignmentLike,
@@ -382,7 +382,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ArrayAssignmentTarget<'a>> {
                 group(&soft_block_indent(&format_once(|f| {
                     if !self.elements.is_empty() {
                         write_array_node(
-                            self.elements.len(),
+                            self.elements.len() + usize::from(self.rest.is_some()),
                             self.elements().iter().map(AstNode::as_ref),
                             f,
                         )?;
@@ -589,8 +589,97 @@ impl<'a> FormatWrite<'a> for AstNode<'a, EmptyStatement> {
     }
 }
 
+/// Returns `true` if the expression needs a leading semicolon to prevent ASI issues
+fn expression_statement_needs_semicolon<'a>(
+    stmt: &AstNode<'a, ExpressionStatement<'a>>,
+    f: &mut Formatter<'_, 'a>,
+) -> bool {
+    if matches!(
+        stmt.parent,
+        // `if (true) (() => {})`
+        AstNodes::IfStatement(_)
+        // `do ({} => {}) while (true)`
+        | AstNodes::DoWhileStatement(_)
+        // `while (true) (() => {})`
+        | AstNodes::WhileStatement(_)
+        // `for (;;) (() => {})`
+        | AstNodes::ForStatement(_)
+        // `for (i in o) (() => {})`
+        | AstNodes::ForInStatement(_)
+        // `for (i of o) (() => {})`
+        | AstNodes::ForOfStatement(_)
+        // `with(true) (() => {})`
+        | AstNodes::WithStatement(_)
+        // `label: (() => {})`
+        | AstNodes::LabeledStatement(_)
+    ) {
+        return false;
+    }
+    // Arrow functions need semicolon only if they will have parentheses
+    // e.g., `(a) => {}` needs `;(a) => {}` but `a => {}` doesn't need semicolon
+    if let Expression::ArrowFunctionExpression(arrow) = &stmt.expression {
+        return !can_avoid_parentheses(arrow, f);
+    }
+
+    // First check if the expression itself needs protection
+    let expr = stmt.expression();
+
+    // Get the leftmost expression to check what the line starts with
+    let mut current = ExpressionLeftSide::Expression(expr);
+    loop {
+        let needs_semi = match current {
+            ExpressionLeftSide::Expression(expr) => {
+                expr.needs_parentheses(f)
+                    || match expr.as_ref() {
+                        Expression::ArrayExpression(_)
+                        | Expression::RegExpLiteral(_)
+                        | Expression::TSTypeAssertion(_)
+                        | Expression::ArrowFunctionExpression(_)
+                        | Expression::JSXElement(_) => true,
+
+                        Expression::TemplateLiteral(template) => true,
+                        Expression::UnaryExpression(unary) => {
+                            matches!(
+                                unary.operator,
+                                UnaryOperator::UnaryPlus | UnaryOperator::UnaryNegation
+                            )
+                        }
+                        _ => false,
+                    }
+            }
+            ExpressionLeftSide::AssignmentTarget(assignment) => {
+                matches!(
+                    assignment.as_ref(),
+                    AssignmentTarget::ArrayAssignmentTarget(_)
+                        | AssignmentTarget::TSTypeAssertion(_)
+                )
+            }
+            ExpressionLeftSide::SimpleAssignmentTarget(assignment) => {
+                matches!(
+                    assignment.as_ref(),
+                        | SimpleAssignmentTarget::TSTypeAssertion(_)
+                )
+            }
+            _ => false,
+        };
+
+        if needs_semi {
+            return true;
+        }
+
+        if let Some(next) = current.left_expression() { current = next } else { return false }
+    }
+}
+
 impl<'a> FormatWrite<'a> for AstNode<'a, ExpressionStatement<'a>> {
     fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
+        // Check if we need a leading semicolon to prevent ASI issues
+        if f.options().semicolons == Semicolons::AsNeeded
+            && expression_statement_needs_semicolon(self, f)
+        {
+            write!(f, ";")?;
+        }
+
         write!(f, [self.expression(), OptionalSemicolon])
     }
 }
@@ -932,7 +1021,7 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ArrayPattern<'a>> {
                 group(&soft_block_indent(&format_once(|f| {
                     if !self.elements.is_empty() {
                         write_array_node(
-                            self.elements.len(),
+                            self.elements.len() + usize::from(self.rest.is_some()),
                             self.elements().iter().map(AstNode::as_ref),
                             f,
                         )?;

crates/oxc_formatter/src/write/return_or_throw_statement.rs

@@ -122,7 +122,7 @@ impl<'a> Format<'a> for FormatReturnOrThrowArgument<'a, '_> {
 ///
 /// Traversing the left nodes is necessary in case the first node is parenthesized because
 /// parentheses will be removed (and be re-added by the return statement, but only if the argument breaks)
-fn has_argument_leading_comments(argument: &Expression, f: &Formatter<'_, '_>) -> bool {
+fn has_argument_leading_comments(argument: &AstNode<Expression>, f: &Formatter<'_, '_>) -> bool {
     let source_text = f.source_text();
 
     let mut current = Some(ExpressionLeftSide::from(argument));
@@ -147,7 +147,7 @@ fn has_argument_leading_comments(argument: &Expression, f: &Formatter<'_, '_>) -
         // This check is based on
         // <https://github.com/prettier/prettier/blob/7584432401a47a26943dd7a9ca9a8e032ead7285/src/language-js/comments/handle-comments.js#L335-L349>
         if let ExpressionLeftSide::Expression(left_side) = left_side {
-            let has_leading_own_line_comment = match left_side {
+            let has_leading_own_line_comment = match left_side.as_ref() {
                 Expression::ChainExpression(chain) => {
                     if let ChainElement::StaticMemberExpression(member) = &chain.expression {
                         is_line_comment_or_multi_line_comment(