From 0c199914716af68f72a7c03d9c722cd34847e305 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Sun, 10 Dec 2023 15:59:13 +0800 Subject: [PATCH] feat(prettier): print CallExpression arguments correctly (#1631) --- crates/oxc_ast/src/ast/js.rs | 9 + crates/oxc_prettier/src/doc.rs | 5 + .../src/format/call_expression.rs | 335 +++++++++++++++++- .../src/format/function_parameters.rs | 8 +- crates/oxc_prettier/src/lib.rs | 9 + crates/oxc_prettier/src/printer/mod.rs | 15 +- crates/oxc_prettier/src/utils/document.rs | 29 ++ crates/oxc_prettier/src/utils/mod.rs | 3 + tasks/prettier_conformance/prettier.snap.md | 17 +- tasks/prettier_conformance/src/lib.rs | 2 +- 10 files changed, 402 insertions(+), 30 deletions(-) create mode 100644 crates/oxc_prettier/src/utils/document.rs create mode 100644 crates/oxc_prettier/src/utils/mod.rs diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index 081bfac215828..0536b6087bcde 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -225,6 +225,15 @@ impl<'a> Expression<'a> { matches!(self, Expression::FunctionExpression(_) | Expression::ArrowExpression(_)) } + pub fn is_call_expression(&self) -> bool { + matches!(self, Expression::CallExpression(_)) + } + + pub fn is_call_like_expression(&self) -> bool { + self.is_call_expression() + && matches!(self, Expression::NewExpression(_) | Expression::ImportExpression(_)) + } + pub fn is_binaryish(&self) -> bool { matches!(self, Expression::BinaryExpression(_) | Expression::LogicalExpression(_)) } diff --git a/crates/oxc_prettier/src/doc.rs b/crates/oxc_prettier/src/doc.rs index 155c22e791e16..f9557b68992c2 100644 --- a/crates/oxc_prettier/src/doc.rs +++ b/crates/oxc_prettier/src/doc.rs @@ -170,6 +170,11 @@ pub trait DocBuilder<'a> { fn vec(&self) -> Vec<'a, T> { Vec::new_in(self.allocator()) } + fn vec_single(&self, value: T) -> Vec<'a, T> { + let mut vec = Vec::with_capacity_in(1, self.allocator()); + vec.push(value); + vec + } #[inline] fn str(&self, s: &str) -> Doc<'a> { diff --git a/crates/oxc_prettier/src/format/call_expression.rs b/crates/oxc_prettier/src/format/call_expression.rs index c96bc3d850587..ea0c080c525d9 100644 --- a/crates/oxc_prettier/src/format/call_expression.rs +++ b/crates/oxc_prettier/src/format/call_expression.rs @@ -2,10 +2,14 @@ use super::misc; use oxc_allocator::Vec; use oxc_ast::ast::*; use oxc_span::{GetSpan, Span}; +use oxc_syntax::operator::UnaryOperator; use crate::{ + array, conditional_group, doc::{Doc, DocBuilder, Group}, - if_break, line, softline, ss, Format, Prettier, + group_break, hardline, if_break, indent, line, softline, ss, + utils::will_break, + Format, Prettier, }; pub(super) enum CallExpressionLike<'a, 'b> { @@ -88,24 +92,132 @@ fn print_call_expression_arguments<'a>( return Doc::Array(parts); } - let mut parts_inner = p.vec(); + #[allow(clippy::cast_sign_loss)] + let get_printed_arguments = |p: &mut Prettier<'a>, skip_index: isize| { + let mut printed_arguments = p.vec(); + let mut len = arguments.len(); + let arguments: Box> = match skip_index { + _ if skip_index > 0 => { + len -= skip_index as usize; + Box::new(arguments.iter().skip(skip_index as usize).enumerate()) + } + _ if skip_index < 0 => { + len -= (-skip_index) as usize; + Box::new( + arguments.iter().take(arguments.len() - (-skip_index) as usize).enumerate(), + ) + } + _ => Box::new(arguments.iter().enumerate()), + }; - for (i, element) in arguments.iter().enumerate() { - let doc = element.format(p); - parts_inner.push(doc); + for (i, element) in arguments { + let doc = element.format(p); + let mut arg = p.vec(); + arg.push(doc); - if i < arguments.len() - 1 { - parts_inner.push(ss!(",")); - parts_inner.push(line!()); + if i < len - 1 { + arg.push(ss!(",")); + if p.is_next_line_empty(element.span()) { + arg.extend(hardline!()); + arg.extend(hardline!()); + } else { + arg.push(line!()); + } + } + printed_arguments.push(Doc::Array(arg)); + } + printed_arguments + }; + + let all_args_broken_out = |p: &mut Prettier<'a>| { + let mut parts = p.vec(); + parts.push(ss!("(")); + parts.push(indent!( + p, + line!(), + Doc::Array(get_printed_arguments(p, 0)), + if p.should_print_all_comma() { ss!(",") } else { ss!("") } + )); + parts.push(line!()); + parts.push(ss!(")")); + Doc::Group(Group::new(parts, true)) + }; + + if should_expand_first_arg(arguments) { + p.args.expand_first_arg = true; + let mut first_doc = arguments[0].format(p); + p.args.expand_first_arg = false; + + if will_break(&mut first_doc) { + let last_doc = get_printed_arguments(p, 1).pop().unwrap(); + return array![ + p, + Doc::BreakParent, + conditional_group!( + p, + array!(p, ss!("("), group_break!(p, first_doc), ss!(", "), last_doc, ss!(")")), + all_args_broken_out(p) + ) + ]; } } + + if should_expand_last_arg(arguments) { + let mut printed_arguments = get_printed_arguments(p, -1); + if printed_arguments.iter_mut().any(will_break) { + return all_args_broken_out(p); + } + + let get_last_doc = |p: &mut Prettier<'a>| { + p.args.expand_last_arg = true; + let last_doc = arguments.last().unwrap().format(p); + p.args.expand_last_arg = false; + last_doc + }; + + let mut last_doc = get_last_doc(p); + + if will_break(&mut last_doc) { + return array![ + p, + Doc::BreakParent, + conditional_group!( + p, + array!( + p, + ss!("("), + Doc::Array(printed_arguments), + group_break!(p, last_doc), + ss!(")") + ), + all_args_broken_out(p) + ), + ]; + } + + return conditional_group!( + p, + array!(p, ss!("("), Doc::Array(printed_arguments), last_doc, ss!(")")), + array!( + p, + ss!("("), + Doc::Array(get_printed_arguments(p, -1)), + group_break!(p, get_last_doc(p)), + ss!(")") + ), + all_args_broken_out(p) + ); + } + + let mut printed_arguments = get_printed_arguments(p, 0); + if should_break { - parts_inner.insert(0, softline!()); - parts.push(Doc::Indent(parts_inner)); + printed_arguments.insert(0, softline!()); + parts.push(Doc::Indent(printed_arguments)); parts.push(if_break!(p, ",", "", None)); parts.push(softline!()); } else { - parts.extend(parts_inner); + parts.extend(printed_arguments); } parts.push(ss!(")")); @@ -113,6 +225,7 @@ fn print_call_expression_arguments<'a>( && arguments.iter().any(|arg| { misc::has_new_line_in_range(p.source_text, arg.span().start, arg.span().end) }); + Doc::Group(Group::new(parts, should_break)) } @@ -145,3 +258,203 @@ fn is_commons_js_or_amd_call<'a>( } false } + +/// * Reference https://github.com/prettier/prettier/blob/main/src/language-js/print/call-arguments.js#L247-L272 +fn should_expand_first_arg<'a>(arguments: &Vec<'a, Argument<'a>>) -> bool { + if arguments.len() != 2 { + return false; + } + + let Argument::Expression(first_arg) = &arguments[0] else { return false }; + let Argument::Expression(second_arg) = &arguments[1] else { return false }; + + let first_check = match first_arg { + Expression::FunctionExpression(_) => true, + Expression::ArrowExpression(arrow) => !arrow.expression, + _ => false, + }; + + first_check + && !matches!( + second_arg, + Expression::FunctionExpression(_) + | Expression::ArrowExpression(_) + | Expression::ConditionalExpression(_) + ) + && is_hopefully_short_call_argument(second_arg) + && !could_expand_arg(second_arg, false) +} + +fn should_expand_last_arg(args: &Vec<'_, Argument<'_>>) -> bool { + let Some(Argument::Expression(last_arg)) = args.last() else { return false }; + + let penultimate_arg = if args.len() >= 2 { Some(&args[args.len() - 2]) } else { None }; + + could_expand_arg(last_arg, false) + && (penultimate_arg.is_none() || matches!(last_arg, arg)) + && (args.len() != 2 + || !matches!( + penultimate_arg, + Some(Argument::Expression(Expression::ArrowExpression(_))) + ) + || !matches!(last_arg, Expression::ArrayExpression(_))) +} + +fn is_hopefully_short_call_argument(node: &Expression) -> bool { + if let Expression::ParenthesizedExpression(expr) = node { + return is_hopefully_short_call_argument(&expr.expression); + } + + if node.is_call_like_expression() { + return !match node { + Expression::CallExpression(call) => call.arguments.len() > 1, + Expression::NewExpression(call) => call.arguments.len() > 1, + Expression::ImportExpression(call) => call.arguments.len() > 0, + _ => false, + }; + } + + if let Expression::BinaryExpression(expr) = node { + return is_simple_call_argument(&expr.left, 1) && is_simple_call_argument(&expr.right, 1); + } + + matches!(node, Expression::RegExpLiteral(_)) || is_simple_call_argument(node, 2) +} + +fn is_simple_call_argument(node: &Expression, depth: usize) -> bool { + if let Expression::RegExpLiteral(literal) = node { + return literal.regex.pattern.len() <= 5; + } + + if node.is_literal() || is_string_word_type(node) { + return true; + } + + let is_child_simple = |node: &Expression| { + if depth <= 1 { + return false; + } + is_simple_call_argument(node, depth - 1) + }; + + if let Expression::TemplateLiteral(literal) = node { + return literal.quasis.iter().all(|element| !element.value.raw.contains('\n')) + && literal.expressions.iter().all(|expr| is_child_simple(expr)); + } + + if let Expression::ObjectExpression(expr) = node { + return expr.properties.iter().all(|p| { + if let ObjectPropertyKind::ObjectProperty(property) = p { + !property.computed && (property.shorthand || is_child_simple(&property.value)) + } else { + false + } + }); + } + + if let Expression::ArrayExpression(expr) = node { + return expr.elements.iter().all( + |x| matches!(x, ArrayExpressionElement::Expression(expr) if is_child_simple(expr)), + ); + } + + if node.is_call_expression() { + if let Expression::ImportExpression(expr) = node { + return expr.arguments.len() <= depth && expr.arguments.iter().all(is_child_simple); + } else if let Expression::CallExpression(expr) = node { + if is_simple_call_argument(&expr.callee, depth) { + return expr.arguments.len() <= depth + && expr.arguments.iter().all(|arg| { + if let Argument::Expression(expr) = arg { + is_child_simple(expr) + } else { + false + } + }); + } + } else if let Expression::NewExpression(expr) = node { + if is_simple_call_argument(&expr.callee, depth) { + return expr.arguments.len() <= depth + && expr.arguments.iter().all(|arg| { + if let Argument::Expression(expr) = arg { + is_child_simple(expr) + } else { + false + } + }); + } + } + return false; + } + + let check_member_expression = |expr: &MemberExpression<'_>| { + if let MemberExpression::StaticMemberExpression(expr) = expr { + return is_simple_call_argument(&expr.object, depth); + } + if let MemberExpression::ComputedMemberExpression(expr) = expr { + return is_simple_call_argument(&expr.object, depth) + && is_simple_call_argument(&expr.expression, depth); + } + if let MemberExpression::PrivateFieldExpression(expr) = expr { + return is_simple_call_argument(&expr.object, depth); + } + false + }; + + if let Expression::MemberExpression(expr) = node { + return check_member_expression(expr); + } + + if let Expression::UnaryExpression(expr) = node { + return matches!( + expr.operator, + UnaryOperator::LogicalNot + | UnaryOperator::UnaryNegation + | UnaryOperator::UnaryPlus + | UnaryOperator::BitwiseNot + ) && is_simple_call_argument(&expr.argument, depth); + } + + if let Expression::UpdateExpression(expr) = node { + return match &expr.argument { + SimpleAssignmentTarget::AssignmentTargetIdentifier(target) => true, + SimpleAssignmentTarget::MemberAssignmentTarget(target) => { + check_member_expression(target) + } + _ => return false, + }; + } + + false +} + +fn could_expand_arg(arg: &Expression, arrow_chain_recursion: bool) -> bool { + match arg { + Expression::ObjectExpression(expr) => expr.properties.len() > 0, + Expression::ArrayExpression(expr) => expr.elements.len() > 0, + Expression::BinaryExpression(expr) => could_expand_arg(&expr.left, arrow_chain_recursion), + Expression::FunctionExpression(_) => true, + Expression::ArrowExpression(expr) => { + if !expr.expression { + return true; + } + let Statement::ExpressionStatement(statement) = &expr.body.statements[0] else { + return false; + }; + + match &statement.expression { + Expression::ArrayExpression(expr) => could_expand_arg(&statement.expression, true), + Expression::ObjectExpression(_) => true, + Expression::CallExpression(_) | Expression::ConditionalExpression(_) => { + !arrow_chain_recursion + } + _ => false, + } + } + _ => false, + } +} + +fn is_string_word_type(node: &Expression) -> bool { + matches!(node, Expression::Identifier(_) | Expression::ThisExpression(_) | Expression::Super(_)) +} diff --git a/crates/oxc_prettier/src/format/function_parameters.rs b/crates/oxc_prettier/src/format/function_parameters.rs index 12f596dc42e5e..3a3dc90bff53e 100644 --- a/crates/oxc_prettier/src/format/function_parameters.rs +++ b/crates/oxc_prettier/src/format/function_parameters.rs @@ -2,7 +2,7 @@ use oxc_ast::{ast::*, AstKind}; use crate::{ comments::CommentFlags, - doc::{Doc, DocBuilder}, + doc::{Doc, DocBuilder, Group}, hardline, if_break, indent, line, softline, ss, Format, Prettier, }; @@ -105,7 +105,11 @@ pub(super) fn print_function_parameters<'a>( parts.push(ss!(")")); } - Doc::Array(parts) + if p.args.expand_first_arg { + Doc::Group(Group::new(parts, false)) + } else { + Doc::Array(parts) + } } pub(super) fn should_group_function_parameters(func: &Function) -> bool { diff --git a/crates/oxc_prettier/src/lib.rs b/crates/oxc_prettier/src/lib.rs index e851eeda73bf0..2bdfc4c66d6d8 100644 --- a/crates/oxc_prettier/src/lib.rs +++ b/crates/oxc_prettier/src/lib.rs @@ -12,6 +12,7 @@ mod macros; mod needs_parens; mod options; mod printer; +mod utils; use std::{iter::Peekable, vec}; @@ -37,6 +38,12 @@ impl GroupIdBuilder { } } +#[derive(Debug, Default)] +pub struct PrettierArgs { + expand_first_arg: bool, + expand_last_arg: bool, +} + pub struct Prettier<'a> { allocator: &'a Allocator, @@ -52,6 +59,7 @@ pub struct Prettier<'a> { nodes: Vec>, group_id_builder: GroupIdBuilder, + args: PrettierArgs, } impl<'a> DocBuilder<'a> for Prettier<'a> { @@ -75,6 +83,7 @@ impl<'a> Prettier<'a> { trivias: trivias.into_iter().peekable(), nodes: vec![], group_id_builder: GroupIdBuilder::default(), + args: PrettierArgs::default(), } } diff --git a/crates/oxc_prettier/src/printer/mod.rs b/crates/oxc_prettier/src/printer/mod.rs index 66024e26d42c8..7da1729b7444f 100644 --- a/crates/oxc_prettier/src/printer/mod.rs +++ b/crates/oxc_prettier/src/printer/mod.rs @@ -122,7 +122,9 @@ impl<'a> Printer<'a> { fn handle_group(&mut self, indent: Indent, mode: Mode, doc: Doc<'a>) { match mode { Mode::Flat => { - let Doc::Group(group) = doc else { unreachable!() }; + let Doc::Group(group) = doc else { + unreachable!(); + }; self.cmds.extend(group.contents.into_iter().rev().map(|doc| { Command::new(indent, if group.should_break { Mode::Break } else { mode }, doc) })); @@ -132,15 +134,18 @@ impl<'a> Printer<'a> { Mode::Break => { #[allow(clippy::cast_possible_wrap)] let remaining_width = self.remaining_width(); - let Doc::Group(group) = &doc else { unreachable!() }; + let Doc::Group(group) = &doc else { + unreachable!(); + }; let should_break = group.should_break; let group_id = group.id; let cmd = Command::new(indent, Mode::Flat, doc); if !should_break && self.fits(&cmd, remaining_width) { self.cmds.push(Command::new(indent, Mode::Flat, cmd.doc)); } else { - let Doc::Group(group) = cmd.doc else { unreachable!() }; - + let Doc::Group(group) = cmd.doc else { + unreachable!(); + }; if let Some(mut expanded_states) = group.expanded_states { let most_expanded = expanded_states.pop().unwrap(); if group.should_break { @@ -449,7 +454,7 @@ impl<'a> Printer<'a> { if !should_break { should_break = check_array(&mut group.contents); } - if should_break { + if group.expanded_states.is_none() && should_break { group.should_break = should_break; } group.should_break diff --git a/crates/oxc_prettier/src/utils/document.rs b/crates/oxc_prettier/src/utils/document.rs new file mode 100644 index 0000000000000..a147dca1c7225 --- /dev/null +++ b/crates/oxc_prettier/src/utils/document.rs @@ -0,0 +1,29 @@ +use crate::{doc::IndentIfBreak, Doc}; + +pub fn will_break(doc: &mut Doc<'_>) -> bool { + let check_array = + |arr: &mut oxc_allocator::Vec<'_, Doc<'_>>| arr.iter_mut().rev().any(|doc| will_break(doc)); + + match doc { + Doc::BreakParent => true, + Doc::Group(group) => { + if group.should_break { + return true; + } + if let Some(expanded_states) = &mut group.expanded_states { + if expanded_states.iter_mut().rev().any(will_break) { + return true; + } + } + check_array(&mut group.contents) + } + Doc::IfBreak(d) => will_break(&mut d.break_contents), + Doc::Array(arr) + | Doc::Indent(arr) + | Doc::LineSuffix(arr) + | Doc::IndentIfBreak(IndentIfBreak { contents: arr, .. }) => check_array(arr), + Doc::Fill(doc) => check_array(&mut doc.parts), + Doc::Line(doc) => doc.hard, + Doc::Str(_) => false, + } +} diff --git a/crates/oxc_prettier/src/utils/mod.rs b/crates/oxc_prettier/src/utils/mod.rs new file mode 100644 index 0000000000000..0bcb5212be8b4 --- /dev/null +++ b/crates/oxc_prettier/src/utils/mod.rs @@ -0,0 +1,3 @@ +mod document; + +pub use self::document::*; diff --git a/tasks/prettier_conformance/prettier.snap.md b/tasks/prettier_conformance/prettier.snap.md index 55bf11a039238..1da39942c449f 100644 --- a/tasks/prettier_conformance/prettier.snap.md +++ b/tasks/prettier_conformance/prettier.snap.md @@ -1,10 +1,9 @@ -Compatibility: 228/550 (41.45%) +Compatibility: 231/550 (42.00%) # Failed ### arrays * arrays/empty.js -* arrays/numbers-in-args.js * arrays/numbers-negative-comment-after-minus.js * arrays/numbers-negative.js * arrays/numbers-with-holes.js @@ -31,7 +30,7 @@ Compatibility: 228/550 (41.45%) * arrows/currying-4.js * arrows/currying.js * arrows/issue-1389-curry.js -* arrows/long-contents.js +* arrows/long-call-no-args.js * arrows/parens.js ### arrows/newline-before-arrow @@ -44,6 +43,7 @@ Compatibility: 228/550 (41.45%) * assignment/chain.js * assignment/discussion-15196.js * assignment/issue-10218.js +* assignment/issue-2540.js * assignment/issue-6922.js * assignment/issue-7572.js * assignment/lone-arg.js @@ -61,7 +61,6 @@ Compatibility: 228/550 (41.45%) * async/conditional-expression.js * async/inline-await.js * async/nested.js -* async/nested2.js ### binary-expressions * binary-expressions/arrow.js @@ -173,7 +172,6 @@ Compatibility: 228/550 (41.45%) * comments-closure-typecast/issue-9358.js * comments-closure-typecast/member.js * comments-closure-typecast/nested.js -* comments-closure-typecast/non-casts.js * comments-closure-typecast/object-with-comment.js * comments-closure-typecast/satisfies.js * comments-closure-typecast/styled-components.js @@ -269,8 +267,10 @@ Compatibility: 228/550 (41.45%) * last-argument-expansion/dangling-comment-in-arrow-function.js * last-argument-expansion/edge_case.js * last-argument-expansion/embed.js +* last-argument-expansion/empty-lines.js * last-argument-expansion/empty-object.js -* last-argument-expansion/function-body-in-mode-break.js +* last-argument-expansion/function-expression-issue-2239.js +* last-argument-expansion/function-expression.js * last-argument-expansion/issue-10708.js * last-argument-expansion/issue-7518.js * last-argument-expansion/jsx.js @@ -291,7 +291,6 @@ Compatibility: 228/550 (41.45%) ### method-chain * method-chain/bracket_0-1.js -* method-chain/bracket_0.js * method-chain/break-last-call.js * method-chain/break-last-member.js * method-chain/comment.js @@ -346,9 +345,6 @@ Compatibility: 228/550 (41.45%) * objects/expand.js * objects/expression.js -### objects/assignment-expression -* objects/assignment-expression/object-value.js - ### optional-chaining * optional-chaining/chaining.js * optional-chaining/comments.js @@ -361,7 +357,6 @@ Compatibility: 228/550 (41.45%) ### performance * performance/nested-real.js -* performance/nested.js ### preserve-line * preserve-line/argument-list.js diff --git a/tasks/prettier_conformance/src/lib.rs b/tasks/prettier_conformance/src/lib.rs index c15728dc92ebd..bbb2897cdbaac 100644 --- a/tasks/prettier_conformance/src/lib.rs +++ b/tasks/prettier_conformance/src/lib.rs @@ -313,7 +313,7 @@ impl TestRunner { ); // put it here but not in below if-statement to help detect no matched input cases. - let expected = Self::get_expect(snap_content, &snapshot_without_output).unwrap(); + let expected = Self::get_expect(snap_content, &snapshot_without_output).unwrap_or_default(); if self.options.filter.is_some() { println!("Input path: {}", path.to_string_lossy());