From e55fdc645267e1ca37f9fcd188f23024152030e3 Mon Sep 17 00:00:00 2001 From: Boshen Date: Fri, 24 Nov 2023 15:07:29 +0800 Subject: [PATCH] feat(prettier): add parens to conditional and arrow expr (#1530) --- crates/oxc_ast/src/ast/js.rs | 12 +++++ crates/oxc_prettier/src/needs_parens.rs | 54 +++++++++++++++++++-- tasks/prettier_conformance/prettier.snap.md | 7 +-- 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index 17964d877..9b2dfdc1e 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -1611,6 +1611,18 @@ pub struct ArrowExpression<'a> { pub return_type: Option>>, } +impl<'a> ArrowExpression<'a> { + /// Get expression part of `ArrowExpression`: `() => expression_part`. + pub fn get_expression(&self) -> Option<&Expression<'a>> { + if self.expression { + if let Statement::ExpressionStatement(expr_stmt) = &self.body.statements[0] { + return Some(&expr_stmt.expression); + } + } + None + } +} + /// Generator Function Definitions #[derive(Debug, Hash)] #[cfg_attr(feature = "serde", derive(Serialize), serde(tag = "type"))] diff --git a/crates/oxc_prettier/src/needs_parens.rs b/crates/oxc_prettier/src/needs_parens.rs index eaff779d5..f93d76ccc 100644 --- a/crates/oxc_prettier/src/needs_parens.rs +++ b/crates/oxc_prettier/src/needs_parens.rs @@ -11,7 +11,7 @@ use oxc_ast::{ ast::{ AssignmentTarget, AssignmentTargetPattern, ChainElement, ExportDefaultDeclarationKind, - Expression, ModuleDeclaration, SimpleAssignmentTarget, + Expression, ModuleDeclaration, ObjectExpression, SimpleAssignmentTarget, }, AstKind, }; @@ -40,6 +40,12 @@ impl<'a> Prettier<'a> { let parent_kind = self.parent_kind(); + if let AstKind::ObjectExpression(e) = kind { + if self.check_object_expression(e) { + return true; + } + } + if self.check_parent_kind(kind, parent_kind) { return true; } @@ -72,8 +78,9 @@ impl<'a> Prettier<'a> { AstKind::Class(c) if c.is_expression() => self.check_object_function_class(c.span), AstKind::AssignmentExpression(assign_expr) => match parent_kind { AstKind::ArrowExpression(arrow_expr) - if arrow_expr.expression - && arrow_expr.body.statements[0].span() == assign_expr.span => + if arrow_expr + .get_expression() + .is_some_and(|e| e.span() == assign_expr.span) => { true } @@ -136,6 +143,24 @@ impl<'a> Prettier<'a> { AstKind::TSNonNullExpression(e) => { self.check_member_call_tagged_template_ts_non_null(e.span) } + AstKind::ConditionalExpression(e) => match parent_kind { + AstKind::TaggedTemplateExpression(_) + | AstKind::UnaryExpression(_) + | AstKind::SpreadElement(_) + | AstKind::BinaryExpression(_) + | AstKind::LogicalExpression(_) + | AstKind::ModuleDeclaration(ModuleDeclaration::ExportDefaultDeclaration(_)) + | AstKind::AwaitExpression(_) + | AstKind::JSXSpreadAttribute(_) + | AstKind::TSAsExpression(_) + | AstKind::TSSatisfiesExpression(_) + | AstKind::TSNonNullExpression(_) => true, + AstKind::CallExpression(call_expr) => call_expr.callee.span() == e.span, + AstKind::NewExpression(new_expr) => new_expr.callee.span() == e.span, + AstKind::ConditionalExpression(cond_expr) => cond_expr.test.span() == e.span, + AstKind::MemberExpression(member_expr) => member_expr.object().span() == e.span, + _ => false, + }, AstKind::Function(e) if e.is_expression() => match parent_kind { AstKind::CallExpression(call_expr) => call_expr.callee.span() == e.span, AstKind::NewExpression(new_expr) => new_expr.callee.span() == e.span, @@ -206,6 +231,29 @@ impl<'a> Prettier<'a> { false } + fn check_object_expression(&self, obj_expr: &ObjectExpression<'a>) -> bool { + let mut arrow_expr = None; + for kind in self.nodes.iter().rev() { + if let AstKind::ArrowExpression(e) = kind { + if e.get_expression().is_some_and(|e| e.span() == obj_expr.span) { + arrow_expr = Some(e); + break; + } + } + } + if let Some(arrow_expr) = arrow_expr { + if let Some(e) = arrow_expr.get_expression() { + if !matches!(e, Expression::SequenceExpression(_)) + && !matches!(e, Expression::AssignmentExpression(_)) + && Self::starts_with_no_lookahead_token(e, obj_expr.span) + { + return true; + } + } + } + false + } + fn check_object_function_class(&self, span: Span) -> bool { for ast_kind in self.nodes.iter().rev() { if let AstKind::ExpressionStatement(e) = ast_kind { diff --git a/tasks/prettier_conformance/prettier.snap.md b/tasks/prettier_conformance/prettier.snap.md index d798680a3..e0922e847 100644 --- a/tasks/prettier_conformance/prettier.snap.md +++ b/tasks/prettier_conformance/prettier.snap.md @@ -1,4 +1,4 @@ -Compatibility: 152/597 (25.46%) +Compatibility: 155/597 (25.96%) # Failed @@ -442,7 +442,6 @@ Compatibility: 152/597 (25.46%) * module-blocks/worker.js ### new-expression -* new-expression/new_expression.js * new-expression/with-member-expression.js ### new-target @@ -554,9 +553,6 @@ Compatibility: 152/597 (25.46%) * sequence-expression/ignore.js * sequence-expression/parenthesized.js -### spread -* spread/spread.js - ### strings * strings/escaped.js * strings/multiline-literal.js @@ -577,7 +573,6 @@ Compatibility: 152/597 (25.46%) * template/graphql.js * template/indent.js * template/inline.js -* template/parenthesis.js ### template-align * template-align/indent.js