Skip to content

Commit

Permalink
feat(prettier): Print leading semi for ExpressionStatement (#9194)
Browse files Browse the repository at this point in the history
Part of #5068
  • Loading branch information
leaysgur authored Feb 18, 2025
1 parent 70726e9 commit 2ade29d
Show file tree
Hide file tree
Showing 9 changed files with 244 additions and 82 deletions.
60 changes: 29 additions & 31 deletions crates/oxc_prettier/src/format/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use crate::{
array, dynamic_text,
format::{
print::{
array, arrow_function, assignment, binaryish, block, call_expression, class, function,
function_parameters, literal, member, misc, module, object, property, statement,
template_literal, ternary,
array, arrow_function, assignment, binaryish, block, call_expression, class,
expression_statement, function, function_parameters, literal, member, misc, module,
object, property, statement, template_literal, ternary,
},
Format,
},
Expand Down Expand Up @@ -108,14 +108,7 @@ impl<'a> Format<'a> for Statement<'a> {
impl<'a> Format<'a> for ExpressionStatement<'a> {
fn format(&self, p: &mut Prettier<'a>) -> Doc<'a> {
wrap!(p, self, ExpressionStatement, {
let mut parts = Vec::new_in(p.allocator);

parts.push(self.expression.format(p));
if let Some(semi) = p.semi() {
parts.push(semi);
}

array!(p, parts)
expression_statement::print_expression_statement(p, self)
})
}
}
Expand Down Expand Up @@ -380,30 +373,35 @@ impl<'a> Format<'a> for SwitchStatement<'a> {

impl<'a> Format<'a> for SwitchCase<'a> {
fn format(&self, p: &mut Prettier<'a>) -> Doc<'a> {
let mut parts = Vec::new_in(p.allocator);

if let Some(test) = &self.test {
parts.push(text!("case "));
parts.push(test.format(p));
parts.push(text!(":"));
} else {
parts.push(text!("default:"));
}

let len =
self.consequent.iter().filter(|c| !matches!(c, Statement::EmptyStatement(_))).count();
if len != 0 {
let consequent_parts =
statement::print_statement_sequence(p, self.consequent.as_slice());
wrap!(p, self, SwitchCase, {
let mut parts = Vec::new_in(p.allocator);

if len == 1 && matches!(self.consequent[0], Statement::BlockStatement(_)) {
parts.push(array!(p, [text!(" "), array!(p, consequent_parts)]));
if let Some(test) = &self.test {
parts.push(text!("case "));
parts.push(test.format(p));
parts.push(text!(":"));
} else {
parts.push(indent!(p, [hardline!(p), array!(p, consequent_parts)]));
parts.push(text!("default:"));
}
}

array!(p, parts)
let len = self
.consequent
.iter()
.filter(|c| !matches!(c, Statement::EmptyStatement(_)))
.count();
if len != 0 {
let consequent_parts =
statement::print_statement_sequence(p, self.consequent.as_slice());

if len == 1 && matches!(self.consequent[0], Statement::BlockStatement(_)) {
parts.push(array!(p, [text!(" "), array!(p, consequent_parts)]));
} else {
parts.push(indent!(p, [hardline!(p), array!(p, consequent_parts)]));
}
}

array!(p, parts)
})
}
}

Expand Down
35 changes: 30 additions & 5 deletions crates/oxc_prettier/src/format/print/arrow_function.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
use oxc_allocator::Vec;
use oxc_ast::ast::*;

use crate::{array, group, ir::Doc, text, Format, Prettier};
use crate::{array, group, ir::Doc, text, ArrowParens, Format, Prettier};

pub fn print_arrow_function<'a>(
p: &mut Prettier<'a>,
expr: &ArrowFunctionExpression<'a>,
) -> Doc<'a> {
let mut parts = Vec::new_in(p.allocator);

if !p.options.semi && p.options.arrow_parens.is_always() {
parts.push(text!(";"));
}

if expr.r#async {
parts.push(text!("async "));
}
Expand Down Expand Up @@ -45,3 +41,32 @@ pub fn print_arrow_function<'a>(

array!(p, parts)
}

pub fn should_print_params_without_parens<'a>(
p: &mut Prettier<'a>,
expr: &ArrowFunctionExpression<'a>,
) -> bool {
match p.options.arrow_parens {
ArrowParens::Always => false,
ArrowParens::Avoid => {
// TODO: hasComment(node, CommentCheckFlags.Dangling) &&
let expr_has_dangling_comment = false;
if (expr.type_parameters.is_some() || expr_has_dangling_comment) {
return false;
}

if expr.params.rest.is_some() || expr.params.items.len() != 1 {
return false;
}
let first_param_pat =
&expr.params.items.first().expect("There should be at least one param").pattern;

// TODO: hasComment(firstParam)
let first_param_has_comment = false;
first_param_pat.kind.is_binding_identifier()
&& first_param_pat.type_annotation.is_none()
&& !first_param_pat.optional
&& !first_param_has_comment
}
}
}
92 changes: 92 additions & 0 deletions crates/oxc_prettier/src/format/print/expression_statement.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
use oxc_allocator::Vec;
use oxc_ast::{ast::*, AstKind};

use crate::{
array, format::print::arrow_function, ir::Doc, needs_parens, text, utils, Format, Prettier,
};

pub fn print_expression_statement<'a>(
p: &mut Prettier<'a>,
expression_statement: &ExpressionStatement<'a>,
) -> Doc<'a> {
let mut parts = Vec::new_in(p.allocator);

if should_print_leading_semicolon(p, expression_statement) {
parts.push(text!(";"));
}

parts.push(expression_statement.expression.format(p));

if let Some(semi) = p.semi() {
parts.push(semi);
}

array!(p, parts)
}

fn should_print_leading_semicolon<'a>(
p: &mut Prettier<'a>,
expr_statement: &ExpressionStatement<'a>,
) -> bool {
if p.options.semi {
return false;
}

if !matches!(
p.parent_kind(),
AstKind::Program(_)
| AstKind::BlockStatement(_)
| AstKind::StaticBlock(_)
| AstKind::SwitchCase(_)
| AstKind::TSModuleBlock(_)
) {
return false;
}

expr_needs_asi_protection(p, &expr_statement.expression)
}

fn expr_needs_asi_protection<'a>(p: &mut Prettier<'a>, expr: &Expression<'a>) -> bool {
if match expr {
Expression::ParenthesizedExpression(_)
| Expression::ArrayExpression(_)
// TODO: ArrayPattern?
| Expression::TemplateLiteral(_)
| Expression::RegExpLiteral(_)
| Expression::JSXElement(_) | Expression::JSXFragment(_) => true,
Expression::ArrowFunctionExpression(arrow_expr)
if !arrow_function::should_print_params_without_parens(p, arrow_expr) => true,
Expression::UnaryExpression(unary_expr)
if unary_expr.operator.is_arithmetic() => true,
_ => false,
} {
return true;
}

// PERF: I'm not sure if this is the best way to handle this
let expr = p.alloc(expr);

let expr_kind = AstKind::from_expression(expr);

// TODO: Consider this is a temporary hack or the right way to handle
// The current implementation is:
// - parent-child relationship such as `stack`, `current_node()` result are updated only by the `wrap!` macro
// - and the `wrap!` macro is only used with a `Format` trait for each node
// Therefore, at the time this code is executed, the outermost node in `stack` == the current node is `ExpressionStatement`.
// However, `expr_needs_asi_protection()` should be called for `.expression` of `ExpressionStatement`.
// That is, the code inside `need_parens()` should have that `.expression` as the current node, but it is not!
// To resolve this gap, manually update the `stack` then call `need_parens()`.
p.stack.push(expr_kind);
let need_parens = p.need_parens(expr_kind);
p.stack.pop();
if need_parens {
return true;
}

if !utils::has_naked_left_side(expr_kind) {
return false;
}

// Check left side
utils::get_left_side_expression(expr).is_some_and(|e| expr_needs_asi_protection(p, e))
}
1 change: 1 addition & 0 deletions crates/oxc_prettier/src/format/print/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub mod block;
pub mod call_arguments;
pub mod call_expression;
pub mod class;
pub mod expression_statement;
pub mod function;
pub mod function_parameters;
pub mod literal;
Expand Down
40 changes: 6 additions & 34 deletions crates/oxc_prettier/src/needs_parens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ use oxc_syntax::{
precedence::GetPrecedence,
};

use crate::{binaryish::BinaryishOperator, Prettier};
use crate::{binaryish::BinaryishOperator, utils, Prettier};

impl<'a> Prettier<'a> {
// NOTE: Why this takes `mut`...?
pub(crate) fn need_parens(&mut self, kind: AstKind<'a>) -> bool {
// NOTE: This `mut` is only for `should_wrap_function_for_export_default()`
pub fn need_parens(&mut self, kind: AstKind<'a>) -> bool {
if matches!(kind, AstKind::Program(_)) || kind.is_statement() || kind.is_declaration() {
return false;
}
Expand Down Expand Up @@ -500,46 +500,18 @@ impl<'a> Prettier<'a> {
return b || !self.need_parens(self.current_kind());
}

if !Self::has_naked_left_side(kind) || (!b && self.need_parens(self.current_kind())) {
if !utils::has_naked_left_side(kind) || (!b && self.need_parens(self.current_kind())) {
return false;
}

let lhs = Self::get_left_side_path_name(kind);
// NOTE: This requires `mut self` for `need_parens()`
let lhs = utils::get_left_side_path_name(kind);
self.stack.push(lhs);
let result = self.should_wrap_function_for_export_default();
self.stack.pop();
result
}

fn has_naked_left_side(kind: AstKind<'a>) -> bool {
matches!(
kind,
AstKind::AssignmentExpression(_)
| AstKind::BinaryExpression(_)
| AstKind::LogicalExpression(_)
| AstKind::ConditionalExpression(_)
| AstKind::CallExpression(_)
| AstKind::MemberExpression(_)
| AstKind::SequenceExpression(_)
| AstKind::TaggedTemplateExpression(_)
| AstKind::TSNonNullExpression(_)
| AstKind::ChainExpression(_)
) || matches!(kind, AstKind::UpdateExpression(e) if !e.prefix)
}

fn get_left_side_path_name(kind: AstKind<'a>) -> AstKind<'a> {
match kind {
AstKind::CallExpression(e) => AstKind::from_expression(&e.callee),
AstKind::ConditionalExpression(e) => AstKind::from_expression(&e.test),
AstKind::TaggedTemplateExpression(e) => AstKind::from_expression(&e.tag),
AstKind::AssignmentExpression(e) => AstKind::AssignmentTarget(&e.left),
AstKind::MemberExpression(e) => AstKind::from_expression(e.object()),
AstKind::BinaryExpression(e) => AstKind::from_expression(&e.left),
AstKind::LogicalExpression(e) => AstKind::from_expression(&e.left),
_ => panic!("need to handle {}", kind.debug_name()),
}
}

fn is_binary_cast_expression(&self, _span: Span) -> bool {
false
}
Expand Down
72 changes: 72 additions & 0 deletions crates/oxc_prettier/src/utils/ast.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
use oxc_ast::{ast::*, AstKind};

pub fn has_naked_left_side(kind: AstKind<'_>) -> bool {
matches!(
kind,
AstKind::AssignmentExpression(_)
| AstKind::BinaryExpression(_)
| AstKind::LogicalExpression(_)
| AstKind::ConditionalExpression(_)
| AstKind::CallExpression(_)
| AstKind::MemberExpression(_)
| AstKind::SequenceExpression(_)
| AstKind::TaggedTemplateExpression(_)
| AstKind::TSNonNullExpression(_)
| AstKind::ChainExpression(_)
) || matches!(kind, AstKind::UpdateExpression(e) if !e.prefix)
}

pub fn get_left_side_path_name(kind: AstKind<'_>) -> AstKind<'_> {
match kind {
AstKind::AssignmentExpression(e) => AstKind::AssignmentTarget(&e.left),
AstKind::BinaryExpression(e) => AstKind::from_expression(&e.left),
AstKind::CallExpression(e) => AstKind::from_expression(&e.callee),
AstKind::ChainExpression(e) => match &e.expression {
ChainElement::CallExpression(e) => AstKind::from_expression(&e.callee),
ChainElement::StaticMemberExpression(e) => AstKind::from_expression(&e.object),
ChainElement::ComputedMemberExpression(e) => AstKind::from_expression(&e.object),
ChainElement::PrivateFieldExpression(e) => AstKind::from_expression(&e.object),
ChainElement::TSNonNullExpression(e) => AstKind::from_expression(&e.expression),
},
AstKind::ConditionalExpression(e) => AstKind::from_expression(&e.test),
AstKind::LogicalExpression(e) => AstKind::from_expression(&e.left),
AstKind::MemberExpression(e) => AstKind::from_expression(e.object()),
AstKind::SequenceExpression(e) => AstKind::from_expression(&e.expressions[0]),
AstKind::TaggedTemplateExpression(e) => AstKind::from_expression(&e.tag),
AstKind::UpdateExpression(e) => AstKind::from_expression(
// TODO: If this causes a panic, make sure to handle `Option`
e.argument.get_expression().expect("UpdateExpression.argument is missing"),
),
AstKind::TSAsExpression(e) => AstKind::from_expression(&e.expression),
AstKind::TSNonNullExpression(e) => AstKind::from_expression(&e.expression),
AstKind::TSSatisfiesExpression(e) => AstKind::from_expression(&e.expression),
_ => unreachable!("get_left_side_path_name(): need to handle {}", kind.debug_name()),
}
}

pub fn get_left_side_expression<'a>(expr: &'a Expression<'a>) -> Option<&'a Expression<'a>> {
match expr {
Expression::AssignmentExpression(e) => e.left.get_expression(),
Expression::BinaryExpression(e) => Some(&e.left),
Expression::CallExpression(e) => Some(&e.callee),
Expression::ChainExpression(e) => match &e.expression {
ChainElement::CallExpression(e) => Some(&e.callee),
ChainElement::StaticMemberExpression(e) => Some(&e.object),
ChainElement::ComputedMemberExpression(e) => Some(&e.object),
ChainElement::PrivateFieldExpression(e) => Some(&e.object),
ChainElement::TSNonNullExpression(e) => Some(&e.expression),
},
Expression::ConditionalExpression(e) => Some(&e.test),
Expression::LogicalExpression(e) => Some(&e.left),
Expression::StaticMemberExpression(e) => Some(&e.object),
Expression::ComputedMemberExpression(e) => Some(&e.object),
Expression::PrivateFieldExpression(e) => Some(&e.object),
Expression::SequenceExpression(e) => Some(&e.expressions[0]),
Expression::TaggedTemplateExpression(e) => Some(&e.tag),
Expression::UpdateExpression(e) => e.argument.get_expression(),
Expression::TSAsExpression(e) => Some(&e.expression),
Expression::TSNonNullExpression(e) => Some(&e.expression),
Expression::TSSatisfiesExpression(e) => Some(&e.expression),
_ => unreachable!("get_left_side_expression: need to handle"),
}
}
4 changes: 3 additions & 1 deletion crates/oxc_prettier/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
mod ast;
mod document;

pub use self::document::*;
pub use ast::*;
pub use document::*;
Loading

0 comments on commit 2ade29d

Please sign in to comment.