From bc2afcaa15a774bd30c58c1d9ff36277c665ab30 Mon Sep 17 00:00:00 2001 From: Jon Egeland Date: Mon, 27 Nov 2023 18:57:26 +0000 Subject: [PATCH 1/5] attempting to match prettier's IR better --- crates/biome_formatter/src/formatter.rs | 17 ++- .../src/js/bindings/constructor_parameters.rs | 8 +- .../src/js/bindings/formal_parameter.rs | 20 ++- .../src/js/bindings/parameters.rs | 139 ++++++++++++++---- .../expressions/arrow_function_expression.rs | 37 +++-- .../src/js/lists/parameter_list.rs | 76 ++++++---- .../src/utils/object_pattern_like.rs | 18 ++- crates/biome_js_formatter/tests/quick_test.rs | 27 ++-- 8 files changed, 242 insertions(+), 100 deletions(-) diff --git a/crates/biome_formatter/src/formatter.rs b/crates/biome_formatter/src/formatter.rs index d4638fe8c5d9..735185486dbd 100644 --- a/crates/biome_formatter/src/formatter.rs +++ b/crates/biome_formatter/src/formatter.rs @@ -114,7 +114,7 @@ impl<'buf, Context> Formatter<'buf, Context> { /// Specialized version of [crate::Formatter::join_with] for joining SyntaxNodes separated by a space, soft /// line break or empty line depending on the input file. /// - /// This functions inspects the input source and separates consecutive elements with either + /// This function inspects the input source and separates consecutive elements with either /// a [crate::builders::soft_line_break_or_space] or [crate::builders::empty_line] depending on how many line breaks were /// separating the elements in the original file. pub fn join_nodes_with_soft_line<'a>( @@ -126,13 +126,26 @@ impl<'buf, Context> Formatter<'buf, Context> { /// Specialized version of [crate::Formatter::join_with] for joining SyntaxNodes separated by one or more /// line breaks depending on the input file. /// - /// This functions inspects the input source and separates consecutive elements with either + /// This function inspects the input source and separates consecutive elements with either /// a [crate::builders::hard_line_break] or [crate::builders::empty_line] depending on how many line breaks were separating the /// elements in the original file. pub fn join_nodes_with_hardline<'a>(&'a mut self) -> JoinNodesBuilder<'a, 'buf, Line, Context> { JoinNodesBuilder::new(hard_line_break(), self) } + /// Specialized version of [crate::Formatter::join_with] for joining SyntaxNodes separated by a simple space. + /// + /// This function *disregards* the input source and always separates consecutive elements with a plain + /// [crate::builders::space], forcing a flat layout regardless of any line breaks or spaces were separating + /// the elements in the original file. + /// + /// This function should likely only be used in a `best_fitting!` context, where one variant attempts to + /// force a list of nodes onto a single line without any possible breaks, then falls back to a broken + /// out variant if the content does not fit. + pub fn join_nodes_with_space<'a>(&'a mut self) -> JoinNodesBuilder<'a, 'buf, Space, Context> { + JoinNodesBuilder::new(space(), self) + } + /// Concatenates a list of [crate::Format] objects with spaces and line breaks to fit /// them on as few lines as possible. Each element introduces a conceptual group. The printer /// first tries to print the item in flat mode but then prints it in expanded mode if it doesn't fit. diff --git a/crates/biome_js_formatter/src/js/bindings/constructor_parameters.rs b/crates/biome_js_formatter/src/js/bindings/constructor_parameters.rs index 8bf4dbe9e5d1..d4e4d1994611 100644 --- a/crates/biome_js_formatter/src/js/bindings/constructor_parameters.rs +++ b/crates/biome_js_formatter/src/js/bindings/constructor_parameters.rs @@ -3,12 +3,18 @@ use crate::prelude::*; use crate::js::bindings::parameters::FormatAnyJsParameters; use biome_js_syntax::JsConstructorParameters; +use super::parameters::{AnyJsParameters, FormatJsParametersOptions}; + #[derive(Debug, Clone, Default)] pub(crate) struct FormatJsConstructorParameters; impl FormatNodeRule for FormatJsConstructorParameters { fn fmt_fields(&self, node: &JsConstructorParameters, f: &mut JsFormatter) -> FormatResult<()> { - FormatAnyJsParameters::from(node.clone()).fmt(f) + FormatAnyJsParameters::new( + AnyJsParameters::JsConstructorParameters(node.clone()), + FormatJsParametersOptions::default(), + ) + .fmt(f) } fn fmt_dangling_comments( diff --git a/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs b/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs index e873da1ee4c8..70807fb95472 100644 --- a/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs +++ b/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs @@ -3,9 +3,12 @@ use biome_formatter::write; use crate::utils::FormatInitializerClause; -use crate::js::bindings::parameters::{should_hug_function_parameters, FormatAnyJsParameters}; -use biome_js_syntax::JsFormalParameter; +use crate::js::bindings::parameters::{ + should_hug_function_parameters, AnyJsParameters, FormatAnyJsParameters, + FormatJsParametersOptions, +}; use biome_js_syntax::JsFormalParameterFields; +use biome_js_syntax::{JsFormalParameter, JsParameters}; #[derive(Debug, Clone, Default)] pub(crate) struct FormatJsFormalParameter; @@ -34,15 +37,22 @@ impl FormatNodeRule for FormatJsFormalParameter { let is_hug_parameter = node .syntax() .grand_parent() - .and_then(FormatAnyJsParameters::cast) + .and_then(JsParameters::cast) .map_or(false, |parameters| { - should_hug_function_parameters(¶meters, f.comments()).unwrap_or(false) + should_hug_function_parameters( + &FormatAnyJsParameters::new( + AnyJsParameters::JsParameters(parameters), + FormatJsParametersOptions::default(), + ), + f.comments(), + ) + .unwrap_or(false) }); if is_hug_parameter && decorators.is_empty() { write![f, [decorators.format(), content]]?; } else if decorators.is_empty() { - write![f, [decorators.format(), group(&content)]]?; + write![f, [decorators.format(), content]]?; } else { write![f, [group(&decorators.format()), group(&content)]]?; } diff --git a/crates/biome_js_formatter/src/js/bindings/parameters.rs b/crates/biome_js_formatter/src/js/bindings/parameters.rs index e32a2406d31e..8d0e9f4199fe 100644 --- a/crates/biome_js_formatter/src/js/bindings/parameters.rs +++ b/crates/biome_js_formatter/src/js/bindings/parameters.rs @@ -1,5 +1,5 @@ use crate::prelude::*; -use biome_formatter::{write, CstFormatContext}; +use biome_formatter::{write, CstFormatContext, FormatRuleWithOptions}; use crate::js::expressions::arrow_function_expression::can_avoid_parentheses; use crate::js::lists::parameter_list::FormatJsAnyParameterList; @@ -8,16 +8,40 @@ use biome_js_syntax::parameter_ext::{AnyJsParameterList, AnyParameter}; use biome_js_syntax::{ AnyJsBinding, AnyJsBindingPattern, AnyJsConstructorParameter, AnyJsExpression, AnyJsFormalParameter, AnyJsParameter, AnyTsType, JsArrowFunctionExpression, - JsConstructorParameters, JsParameters, JsSyntaxToken, + JsConstructorParameters, JsParameters, JsSyntaxNode, JsSyntaxToken, }; -use biome_rowan::{declare_node_union, SyntaxResult}; +use biome_rowan::{AstNode, SyntaxResult}; -#[derive(Debug, Clone, Default)] -pub(crate) struct FormatJsParameters; +#[derive(Debug, Copy, Clone, Default)] +pub(crate) struct FormatJsParameters { + options: FormatJsParametersOptions, +} + +#[derive(Debug, Clone, Copy, Default)] +pub(crate) struct FormatJsParametersOptions { + /// Whether the parameters should include soft line breaks to allow them to + /// break naturally over multiple lines when they can't fit on one line. + /// + /// This is particularly important for arrow chains passed as arguments in + /// call expressions, where it must be set to false to avoid having the + /// parameters break onto lines before the entire expression expands. + /// + /// When `true`, parameters will _not_ include any soft line break points. + pub prevent_soft_line_breaks: bool, +} + +impl FormatRuleWithOptions for FormatJsParameters { + type Options = FormatJsParametersOptions; + + fn with_options(mut self, options: Self::Options) -> Self { + self.options = options; + self + } +} impl FormatNodeRule for FormatJsParameters { fn fmt_fields(&self, node: &JsParameters, f: &mut JsFormatter) -> FormatResult<()> { - FormatAnyJsParameters::from(node.clone()).fmt(f) + FormatAnyJsParameters::new(AnyJsParameters::JsParameters(node.clone()), self.options).fmt(f) } fn fmt_dangling_comments(&self, _: &JsParameters, _: &mut JsFormatter) -> FormatResult<()> { @@ -26,8 +50,23 @@ impl FormatNodeRule for FormatJsParameters { } } -declare_node_union! { - pub(crate) FormatAnyJsParameters = JsParameters | JsConstructorParameters +pub(crate) enum AnyJsParameters { + JsParameters(JsParameters), + JsConstructorParameters(JsConstructorParameters), +} + +pub(crate) struct FormatAnyJsParameters { + pub(crate) parameters: AnyJsParameters, + pub(crate) options: FormatJsParametersOptions, +} + +impl FormatAnyJsParameters { + pub(crate) fn new(parameters: AnyJsParameters, options: FormatJsParametersOptions) -> Self { + Self { + parameters, + options, + } + } } impl Format for FormatAnyJsParameters { @@ -43,6 +82,8 @@ impl Format for FormatAnyJsParameters { ParameterLayout::NoParameters } else if can_hug || self.is_in_test_call()? { ParameterLayout::Hug + } else if self.options.prevent_soft_line_breaks { + ParameterLayout::Compact } else { ParameterLayout::Default }; @@ -60,7 +101,7 @@ impl Format for FormatAnyJsParameters { f, [ l_paren_token.format(), - format_dangling_comments(self.syntax()).with_soft_block_indent(), + format_dangling_comments(&self.inner_syntax()).with_soft_block_indent(), r_paren_token.format() ] ) @@ -99,7 +140,7 @@ impl Format for FormatAnyJsParameters { f, [soft_block_indent(&FormatJsAnyParameterList::with_layout( &list, - ParameterLayout::Default + ParameterLayout::Default, ))] )?; @@ -109,6 +150,29 @@ impl Format for FormatAnyJsParameters { write!(f, [format_removed(&r_paren_token)])?; } + Ok(()) + } + ParameterLayout::Compact => { + if !parentheses_not_needed { + write!(f, [l_paren_token.format()])?; + } else { + write!(f, [format_removed(&l_paren_token)])?; + } + + write!( + f, + [FormatJsAnyParameterList::with_layout( + &list, + ParameterLayout::Compact + )] + )?; + + if !parentheses_not_needed { + write!(f, [r_paren_token.format()])?; + } else { + write!(f, [format_removed(&r_paren_token)])?; + } + Ok(()) } } @@ -117,54 +181,57 @@ impl Format for FormatAnyJsParameters { impl FormatAnyJsParameters { fn l_paren_token(&self) -> SyntaxResult { - match self { - FormatAnyJsParameters::JsParameters(parameters) => parameters.l_paren_token(), - FormatAnyJsParameters::JsConstructorParameters(parameters) => { - parameters.l_paren_token() - } + match &self.parameters { + AnyJsParameters::JsParameters(parameters) => parameters.l_paren_token(), + AnyJsParameters::JsConstructorParameters(parameters) => parameters.l_paren_token(), } } fn list(&self) -> AnyJsParameterList { - match self { - FormatAnyJsParameters::JsParameters(parameters) => { + match &self.parameters { + AnyJsParameters::JsParameters(parameters) => { AnyJsParameterList::from(parameters.items()) } - FormatAnyJsParameters::JsConstructorParameters(parameters) => { + AnyJsParameters::JsConstructorParameters(parameters) => { AnyJsParameterList::from(parameters.parameters()) } } } fn r_paren_token(&self) -> SyntaxResult { - match self { - FormatAnyJsParameters::JsParameters(parameters) => parameters.r_paren_token(), - FormatAnyJsParameters::JsConstructorParameters(parameters) => { - parameters.r_paren_token() - } + match &self.parameters { + AnyJsParameters::JsParameters(parameters) => parameters.r_paren_token(), + AnyJsParameters::JsConstructorParameters(parameters) => parameters.r_paren_token(), } } /// Returns `true` for function parameters if the function is an argument of a [test `CallExpression`](is_test_call_expression). fn is_in_test_call(&self) -> SyntaxResult { - let result = match self { - FormatAnyJsParameters::JsParameters(parameters) => match parameters.syntax().parent() { + let result = match &self.parameters { + AnyJsParameters::JsParameters(parameters) => match parameters.syntax().parent() { Some(function) => is_test_call_argument(&function)?, None => false, }, - FormatAnyJsParameters::JsConstructorParameters(_) => false, + AnyJsParameters::JsConstructorParameters(_) => false, }; Ok(result) } fn as_arrow_function_expression(&self) -> Option { - match self { - FormatAnyJsParameters::JsParameters(parameters) => parameters + match &self.parameters { + AnyJsParameters::JsParameters(parameters) => parameters .syntax() .parent() .and_then(JsArrowFunctionExpression::cast), - FormatAnyJsParameters::JsConstructorParameters(_) => None, + AnyJsParameters::JsConstructorParameters(_) => None, + } + } + + fn inner_syntax(&self) -> &JsSyntaxNode { + match &self.parameters { + AnyJsParameters::JsParameters(parameters) => parameters.syntax(), + AnyJsParameters::JsConstructorParameters(parameters) => parameters.syntax(), } } } @@ -190,7 +257,8 @@ pub enum ParameterLayout { Hug, /// The default layout formats all parameters on the same line if they fit or breaks after the `(` - /// and before the `(`. + /// and before the `)`. + /// /// ```javascript /// function test( /// firstParameter, @@ -199,6 +267,17 @@ pub enum ParameterLayout { /// ) {} /// ``` Default, + + /// Compact layout forces all parameters to try to render on the same line, + /// with no breaks added around the brackets. This should likely only be + /// used in a `best_fitting!` context where one variant attempts to fit the + /// parameters on a single line, and a default expanded version that is + /// used in case that does not fit. + /// + /// ```javascript + /// function test(firstParameter, secondParameter, thirdParameter, evenOverlyLong) {} + /// ``` + Compact, } pub(crate) fn should_hug_function_parameters( diff --git a/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs b/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs index eb859a76a92a..bfb360f6bca1 100644 --- a/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs +++ b/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs @@ -1,3 +1,4 @@ +use crate::js::bindings::parameters::FormatJsParametersOptions; use crate::prelude::*; use biome_formatter::{ format_args, write, CstFormatContext, FormatRuleWithOptions, RemoveSoftLinesBuffer, @@ -64,7 +65,12 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi write!( f, [ - format_signature(&arrow, self.options.call_arg_layout.is_some(), false), + format_signature( + &arrow, + self.options.call_arg_layout.is_some(), + false, + true + ), space(), arrow.fat_arrow_token().format() ] @@ -206,6 +212,7 @@ fn format_signature( arrow: &JsArrowFunctionExpression, is_first_or_last_call_argument: bool, ancestor_call_expr_or_logical_expr: bool, + is_first_in_chain: bool, ) -> impl Format + '_ { format_with(move |f| { if let Some(async_token) = arrow.async_token() { @@ -242,10 +249,15 @@ fn format_signature( } } AnyJsArrowFunctionParameters::JsParameters(params) => { - if ancestor_call_expr_or_logical_expr { - write!(f, [dedent(¶ms.format())])?; + let formatted_params = + params.format().with_options(FormatJsParametersOptions { + prevent_soft_line_breaks: is_first_or_last_call_argument, + }); + + if ancestor_call_expr_or_logical_expr && !is_first_or_last_call_argument { + write!(f, [group(&dedent(&formatted_params))])?; } else { - write!(f, [params.format()])?; + write!(f, [formatted_params])?; } } }; @@ -260,6 +272,7 @@ fn format_signature( write!( recording, [group(&format_args![ + (!is_first_in_chain).then_some(space()), group(&format_parameters), group(&arrow.return_type_annotation().format()) ])] @@ -272,6 +285,7 @@ fn format_signature( write!( f, [group(&format_args![ + (!is_first_in_chain).then_some(soft_line_break_or_space()), format_parameters, arrow.return_type_annotation().format() ])] @@ -510,6 +524,7 @@ impl Format for ArrowChain { } let join_signatures = format_with(|f| { + let mut is_first_in_chain = true; for arrow in self.arrows() { write!( f, @@ -518,22 +533,18 @@ impl Format for ArrowChain { format_signature( arrow, self.options.call_arg_layout.is_some(), - ancestor_call_expr_or_logical_expr + ancestor_call_expr_or_logical_expr, + is_first_in_chain, ) ] )?; + is_first_in_chain = false; + // The arrow of the tail is formatted outside of the group to ensure it never // breaks from the body if arrow != tail { - write!( - f, - [ - space(), - arrow.fat_arrow_token().format(), - soft_line_break_or_space() - ] - )?; + write!(f, [space(), arrow.fat_arrow_token().format()])?; } } diff --git a/crates/biome_js_formatter/src/js/lists/parameter_list.rs b/crates/biome_js_formatter/src/js/lists/parameter_list.rs index 0b989c078a7a..f774a1b58a92 100644 --- a/crates/biome_js_formatter/src/js/lists/parameter_list.rs +++ b/crates/biome_js_formatter/src/js/lists/parameter_list.rs @@ -38,7 +38,10 @@ impl<'a> FormatJsAnyParameterList<'a> { impl Format for FormatJsAnyParameterList<'_> { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { match self.layout { - None | Some(ParameterLayout::Default | ParameterLayout::NoParameters) => { + None + | Some( + ParameterLayout::Default | ParameterLayout::Compact | ParameterLayout::NoParameters, + ) => { // The trailing separator is disallowed if the last element in the list is a rest parameter let has_trailing_rest = match self.list.last() { Some(elem) => matches!( @@ -51,38 +54,23 @@ impl Format for FormatJsAnyParameterList<'_> { None => false, }; - let trailing_separator = if has_trailing_rest { + let is_compact = matches!(self.layout, Some(ParameterLayout::Compact)); + + let trailing_separator = if is_compact || has_trailing_rest { TrailingSeparator::Disallowed } else { FormatTrailingComma::All.trailing_separator(f.options()) }; - let mut join = f.join_nodes_with_soft_line(); - - match self.list { - AnyJsParameterList::JsParameterList(list) => { - let entries = list - .format_separated(",") - .with_trailing_separator(trailing_separator) - .zip(list.iter()); - - for (format_entry, node) in entries { - join.entry(node?.syntax(), &format_entry); - } - } - AnyJsParameterList::JsConstructorParameterList(list) => { - let entries = list - .format_separated(",") - .with_trailing_separator(trailing_separator) - .zip(list.iter()); - - for (format_entry, node) in entries { - join.entry(node?.syntax(), &format_entry); - } - } + if is_compact { + let mut joiner = f.join_nodes_with_space(); + join_parameter_list(&mut joiner, &self.list, trailing_separator)?; + joiner.finish() + } else { + let mut joiner = f.join_nodes_with_soft_line(); + join_parameter_list(&mut joiner, &self.list, trailing_separator)?; + joiner.finish() } - - join.finish() } Some(ParameterLayout::Hug) => { let mut join = f.join_with(space()); @@ -103,3 +91,37 @@ impl Format for FormatJsAnyParameterList<'_> { } } } + +fn join_parameter_list( + joiner: &mut JoinNodesBuilder<'_, '_, S, JsFormatContext>, + list: &AnyJsParameterList, + trailing_separator: TrailingSeparator, +) -> FormatResult<()> +where + S: Format, +{ + match list { + AnyJsParameterList::JsParameterList(list) => { + let entries = list + .format_separated(",") + .with_trailing_separator(trailing_separator) + .zip(list.iter()); + + for (format_entry, node) in entries { + joiner.entry(node?.syntax(), &format_entry); + } + } + AnyJsParameterList::JsConstructorParameterList(list) => { + let entries = list + .format_separated(",") + .with_trailing_separator(trailing_separator) + .zip(list.iter()); + + for (format_entry, node) in entries { + joiner.entry(node?.syntax(), &format_entry); + } + } + } + + Ok(()) +} diff --git a/crates/biome_js_formatter/src/utils/object_pattern_like.rs b/crates/biome_js_formatter/src/utils/object_pattern_like.rs index e6647303bc7a..ba1b5f6d6b90 100644 --- a/crates/biome_js_formatter/src/utils/object_pattern_like.rs +++ b/crates/biome_js_formatter/src/utils/object_pattern_like.rs @@ -1,4 +1,7 @@ -use crate::js::bindings::parameters::{should_hug_function_parameters, FormatAnyJsParameters}; +use crate::js::bindings::parameters::{ + should_hug_function_parameters, AnyJsParameters, FormatAnyJsParameters, + FormatJsParametersOptions, +}; use crate::prelude::*; use crate::JsFormatContext; use biome_formatter::formatter::Formatter; @@ -7,7 +10,7 @@ use biome_formatter::{Format, FormatResult}; use biome_js_syntax::{ AnyJsAssignmentPattern, AnyJsBindingPattern, AnyJsFormalParameter, AnyJsObjectAssignmentPatternMember, AnyJsObjectBindingPatternMember, JsObjectAssignmentPattern, - JsObjectBindingPattern, JsSyntaxKind, JsSyntaxToken, + JsObjectBindingPattern, JsParameters, JsSyntaxKind, JsSyntaxToken, }; use biome_rowan::{declare_node_union, AstNode, SyntaxNodeOptionExt, SyntaxResult}; @@ -129,9 +132,16 @@ impl JsObjectPatternLike { JsObjectPatternLike::JsObjectBindingPattern(binding) => binding .parent::() .and_then(|parameter| parameter.syntax().grand_parent()) - .and_then(FormatAnyJsParameters::cast) + .and_then(JsParameters::cast) .map_or(false, |parameters| { - should_hug_function_parameters(¶meters, comments).unwrap_or(false) + should_hug_function_parameters( + &FormatAnyJsParameters::new( + AnyJsParameters::JsParameters(parameters), + FormatJsParametersOptions::default(), + ), + comments, + ) + .unwrap_or(false) }), } } diff --git a/crates/biome_js_formatter/tests/quick_test.rs b/crates/biome_js_formatter/tests/quick_test.rs index 20769bf5b359..8b1f9c315b64 100644 --- a/crates/biome_js_formatter/tests/quick_test.rs +++ b/crates/biome_js_formatter/tests/quick_test.rs @@ -1,5 +1,4 @@ use biome_formatter::IndentStyle; -use biome_formatter_test::check_reformat::CheckReformat; use biome_js_formatter::context::{ArrowParentheses, JsFormatOptions, QuoteStyle, Semicolons}; use biome_js_formatter::format_node; use biome_js_parser::{parse, JsParserOptions}; @@ -14,13 +13,11 @@ mod language { // use this test check if your snippet prints as you wish, without using a snapshot fn quick_test() { let src = r#" -type a= { - [ - A in - // prettier-ignore - B - ]: C | D - } +a( + (long1ArgumentNamfffe1AndEvongferSothat,ff) => ( + long3ArgumentName3ItafsafBreaks22 + ) => [a], + ); "#; let syntax = JsFileSource::tsx(); let tree = parse( @@ -33,18 +30,12 @@ type a= { .with_semicolons(Semicolons::Always) .with_quote_style(QuoteStyle::Double) .with_jsx_quote_style(QuoteStyle::Single) - .with_arrow_parentheses(ArrowParentheses::AsNeeded); - let result = format_node(options.clone(), &tree.syntax()) - .unwrap() - .print() - .unwrap(); + .with_arrow_parentheses(ArrowParentheses::Always); + let doc = format_node(options.clone(), &tree.syntax()).unwrap(); - let root = &tree.syntax(); - let language = language::JsTestFormatLanguage::new(JsFileSource::tsx()); - let check_reformat = - CheckReformat::new(root, result.as_code(), "quick_test", &language, options); - check_reformat.check_reformat(); + let result = doc.print().unwrap(); + println!("{}", doc.into_document()); eprintln!("{}", result.as_code()); // I don't know why semicolons are added there, but it's not related to my code changes so ¯\_(ツ)_/¯ assert_eq!( From 7184a451be78c0b17bff036d65ae990f90c04d1b Mon Sep 17 00:00:00 2001 From: Jon Egeland Date: Mon, 27 Nov 2023 23:33:29 +0000 Subject: [PATCH 2/5] fix soft lines breaking parameter lists --- .../src/js/bindings/formal_parameter.rs | 2 +- .../expressions/arrow_function_expression.rs | 41 +- .../src/js/lists/parameter_list.rs | 11 +- crates/biome_js_formatter/tests/quick_test.rs | 32 +- .../specs/prettier/js/arrows/curried.js.snap | 595 ------------------ 5 files changed, 50 insertions(+), 631 deletions(-) delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/arrows/curried.js.snap diff --git a/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs b/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs index 70807fb95472..0df8765c6f64 100644 --- a/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs +++ b/crates/biome_js_formatter/src/js/bindings/formal_parameter.rs @@ -52,7 +52,7 @@ impl FormatNodeRule for FormatJsFormalParameter { if is_hug_parameter && decorators.is_empty() { write![f, [decorators.format(), content]]?; } else if decorators.is_empty() { - write![f, [decorators.format(), content]]?; + write![f, [decorators.format(), group(&content)]]?; } else { write![f, [group(&decorators.format()), group(&content)]]?; } diff --git a/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs b/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs index bfb360f6bca1..1d2665c7ee38 100644 --- a/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs +++ b/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs @@ -61,7 +61,7 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi let body = arrow.body()?; - let format_signature = format_with(|f| { + let formatted_signature = format_with(|f| { write!( f, [ @@ -114,7 +114,7 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi return write!( f, [group(&format_args![ - format_signature, + formatted_signature, group(&format_args![indent(&format_args![ hard_line_break(), text("("), @@ -127,7 +127,7 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi return write!( f, [group(&format_args![ - format_signature, + formatted_signature, group(&format_args![ space(), text("("), @@ -141,7 +141,7 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi }; if body_has_soft_line_break { - write![f, [format_signature, space(), format_body]] + write![f, [formatted_signature, space(), format_body]] } else { let should_add_parens = should_add_parens(&body); @@ -158,7 +158,7 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi write!( f, [ - format_signature, + formatted_signature, group(&format_args![ soft_line_indent_or_space(&format_with(|f| { if should_add_parens { @@ -215,11 +215,16 @@ fn format_signature( is_first_in_chain: bool, ) -> impl Format + '_ { format_with(move |f| { - if let Some(async_token) = arrow.async_token() { - write!(f, [async_token.format(), space()])?; - } + let formatted_async_token = format_with(|f: &mut JsFormatter| { + if let Some(async_token) = arrow.async_token() { + write!(f, [async_token.format(), space()])?; + Ok(()) + } else { + Ok(()) + } + }); - let format_parameters = format_with(|f: &mut JsFormatter| { + let formatted_parameters = format_with(|f: &mut JsFormatter| { write!(f, [arrow.type_parameters().format()])?; match arrow.parameters()? { @@ -273,7 +278,8 @@ fn format_signature( recording, [group(&format_args![ (!is_first_in_chain).then_some(space()), - group(&format_parameters), + formatted_async_token, + group(&formatted_parameters), group(&arrow.return_type_annotation().format()) ])] )?; @@ -284,11 +290,18 @@ fn format_signature( } else { write!( f, - [group(&format_args![ + [ + // This soft break is placed outside of the group to ensure + // that the parameter group only tries to write on a single + // line and can't break pre-emptively without also causing + // the parent (i.e., this ArrowChain) to break first. (!is_first_in_chain).then_some(soft_line_break_or_space()), - format_parameters, - arrow.return_type_annotation().format() - ])] + group(&format_args![ + formatted_async_token, + formatted_parameters, + arrow.return_type_annotation().format() + ]) + ] )?; } diff --git a/crates/biome_js_formatter/src/js/lists/parameter_list.rs b/crates/biome_js_formatter/src/js/lists/parameter_list.rs index f774a1b58a92..1c6d65a7abf8 100644 --- a/crates/biome_js_formatter/src/js/lists/parameter_list.rs +++ b/crates/biome_js_formatter/src/js/lists/parameter_list.rs @@ -42,7 +42,7 @@ impl Format for FormatJsAnyParameterList<'_> { | Some( ParameterLayout::Default | ParameterLayout::Compact | ParameterLayout::NoParameters, ) => { - // The trailing separator is disallowed if the last element in the list is a rest parameter + let is_compact = matches!(self.layout, Some(ParameterLayout::Compact)); let has_trailing_rest = match self.list.last() { Some(elem) => matches!( elem?, @@ -54,8 +54,13 @@ impl Format for FormatJsAnyParameterList<'_> { None => false, }; - let is_compact = matches!(self.layout, Some(ParameterLayout::Compact)); - + // If the list is being printed compactly, then the closing + // bracket _must_ appear immediately next to the last element, + // so a trailing separator would appear incorrect. + // + // If it's a rest parameter, the assumption is no more + // parameters could be added afterward, so no separator is + // added there either. let trailing_separator = if is_compact || has_trailing_rest { TrailingSeparator::Disallowed } else { diff --git a/crates/biome_js_formatter/tests/quick_test.rs b/crates/biome_js_formatter/tests/quick_test.rs index 8b1f9c315b64..d429a98fb5b4 100644 --- a/crates/biome_js_formatter/tests/quick_test.rs +++ b/crates/biome_js_formatter/tests/quick_test.rs @@ -1,4 +1,5 @@ use biome_formatter::IndentStyle; +use biome_formatter_test::check_reformat::CheckReformat; use biome_js_formatter::context::{ArrowParentheses, JsFormatOptions, QuoteStyle, Semicolons}; use biome_js_formatter::format_node; use biome_js_parser::{parse, JsParserOptions}; @@ -13,11 +14,9 @@ mod language { // use this test check if your snippet prints as you wish, without using a snapshot fn quick_test() { let src = r#" -a( - (long1ArgumentNamfffe1AndEvongferSothat,ff) => ( - long3ArgumentName3ItafsafBreaks22 - ) => [a], - ); + request.get('https://preview-9992--prettier.netlify.app', head => body => { + console.log(head, body); + }); "#; let syntax = JsFileSource::tsx(); let tree = parse( @@ -31,22 +30,19 @@ a( .with_quote_style(QuoteStyle::Double) .with_jsx_quote_style(QuoteStyle::Single) .with_arrow_parentheses(ArrowParentheses::Always); - let doc = format_node(options.clone(), &tree.syntax()).unwrap(); + let doc = format_node(options.clone(), &tree.syntax()).unwrap(); let result = doc.print().unwrap(); + let source_type = JsFileSource::js_module(); - println!("{}", doc.into_document()); eprintln!("{}", result.as_code()); - // I don't know why semicolons are added there, but it's not related to my code changes so ¯\_(ツ)_/¯ - assert_eq!( + + CheckReformat::new( + &tree.syntax(), result.as_code(), - r#"const fooo: SomeThingWithLongMappedType<{ - [P in - | AAAAAAAAAAAAAAAAA - | BBBBBBBBBBBB - | CCCCCCCCCCCCCCCCCCCCC - | DDDDDDDDDDDDDDDDDDDDDDDDDDDDD]: number; -}> = {}; -"# - ); + "testing", + &language::JsTestFormatLanguage::new(source_type), + options, + ) + .check_reformat(); } diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/arrows/curried.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/arrows/curried.js.snap deleted file mode 100644 index 1bac8db7063c..000000000000 --- a/crates/biome_js_formatter/tests/specs/prettier/js/arrows/curried.js.snap +++ /dev/null @@ -1,595 +0,0 @@ ---- -source: crates/biome_formatter_test/src/snapshot_builder.rs -info: js/arrows/curried.js ---- - -# Input - -```js -const fn1 = a => 3; -const fn2 = a => b => 3; -const fn3 = a => b => c => 3; -const fn4 = a => b => c => d => 3; -const fn5 = a => b => c => d => e => 3; -const fn6 = a => b => c => d => e => g => 3; -const fn7 = a => b => c => d => e => g => f => 3; - -const fn8 = a => ({ foo: bar, bar: baz, baz: foo }); -const fn9 = a => b => ({ foo: bar, bar: baz, baz: foo }); -const fn10 = a => b => c => ({ foo: bar, bar: baz, baz: foo }); -const fn11 = a => b => c => d => ({ foo: bar, bar: baz, baz: foo }); -const fn12 = a => b => c => d => e => ({ foo: bar, bar: baz, baz: foo }); -const fn13 = a => b => c => d => e => g => ({ foo: bar, bar: baz, baz: foo }); -const fn14 = a => b => c => d => e => g => f => ({ foo: bar, bar: baz, baz: foo }); - -const curryTest = - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => - ({ - foo: argument1, - bar: argument2, - }); - -let curryTest2 = - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - }; - -curryTest2 = - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - }; - -throw (argument1) => -(argument2) => -(argument3) => -(argument4) => -(argument5) => -(argument6) => -(argument7) => -(argument8) => -(argument9) => -(argument10) => -(argument11) => -(argument12) => { - const foo = "foo"; - return foo + "bar"; -}; - -foo((argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => 3); - -foo((argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => ({ - foo: bar, - bar: baz, - baz: foo - })); - -foo( - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - } -); - -((argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => 3)(3); - -bar( - foo( - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => ({ - foo: bar, - bar: baz, - }) - ) -); - -const baaaz = (aaaaa1, bbbbb1) => (aaaaa2, bbbbb2) => (aaaaa3, bbbbb3) => (aaaaa4, bbbbb4) => ({ - foo: bar -}); - -new Fooooooooooooooooooooooooooooooooooooooooooooooooooo( - (action) => - (next) => - (next) => - (next) => - (next) => - (next) => - (next) => - dispatch(action) -); - -foo?.Fooooooooooooooooooooooooooooooooooooooooooooooooooo( - (action) => - (next) => - (next) => - (next) => - (next) => - (next) => - (next) => - dispatch(action) -); - -foo(action => action => action); - -import( (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - }); - -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Biome -@@ -109,42 +109,38 @@ - 3, - ); - --foo( -- (argument1) => -- (argument2) => -- (argument3) => -- (argument4) => -- (argument5) => -- (argument6) => -- (argument7) => -- (argument8) => -- (argument9) => -- (argument10) => -- (argument11) => -- (argument12) => ({ -- foo: bar, -- bar: baz, -- baz: foo, -- }), --); -+foo((argument1) => -+ (argument2) => -+ (argument3) => -+ (argument4) => -+ (argument5) => -+ (argument6) => -+ (argument7) => -+ (argument8) => -+ (argument9) => -+ (argument10) => -+ (argument11) => -+ (argument12) => ({ -+ foo: bar, -+ bar: baz, -+ baz: foo, -+ })); - --foo( -- (argument1) => -- (argument2) => -- (argument3) => -- (argument4) => -- (argument5) => -- (argument6) => -- (argument7) => -- (argument8) => -- (argument9) => -- (argument10) => -- (argument11) => -- (argument12) => { -- const foo = "foo"; -- return foo + "bar"; -- }, --); -+foo((argument1) => -+ (argument2) => -+ (argument3) => -+ (argument4) => -+ (argument5) => -+ (argument6) => -+ (argument7) => -+ (argument8) => -+ (argument9) => -+ (argument10) => -+ (argument11) => -+ (argument12) => { -+ const foo = "foo"; -+ return foo + "bar"; -+ }); - - ( - (argument1) => -@@ -163,23 +159,21 @@ - )(3); - - bar( -- foo( -- (argument1) => -- (argument2) => -- (argument3) => -- (argument4) => -- (argument5) => -- (argument6) => -- (argument7) => -- (argument8) => -- (argument9) => -- (argument10) => -- (argument11) => -- (argument12) => ({ -- foo: bar, -- bar: baz, -- }), -- ), -+ foo((argument1) => -+ (argument2) => -+ (argument3) => -+ (argument4) => -+ (argument5) => -+ (argument6) => -+ (argument7) => -+ (argument8) => -+ (argument9) => -+ (argument10) => -+ (argument11) => -+ (argument12) => ({ -+ foo: bar, -+ bar: baz, -+ })), - ); - - const baaaz = -@@ -202,20 +196,18 @@ - - foo((action) => (action) => action); - --import( -- (argument1) => -- (argument2) => -- (argument3) => -- (argument4) => -- (argument5) => -- (argument6) => -- (argument7) => -- (argument8) => -- (argument9) => -- (argument10) => -- (argument11) => -- (argument12) => { -- const foo = "foo"; -- return foo + "bar"; -- } --); -+import((argument1) => -+ (argument2) => -+ (argument3) => -+ (argument4) => -+ (argument5) => -+ (argument6) => -+ (argument7) => -+ (argument8) => -+ (argument9) => -+ (argument10) => -+ (argument11) => -+ (argument12) => { -+ const foo = "foo"; -+ return foo + "bar"; -+ }); -``` - -# Output - -```js -const fn1 = (a) => 3; -const fn2 = (a) => (b) => 3; -const fn3 = (a) => (b) => (c) => 3; -const fn4 = (a) => (b) => (c) => (d) => 3; -const fn5 = (a) => (b) => (c) => (d) => (e) => 3; -const fn6 = (a) => (b) => (c) => (d) => (e) => (g) => 3; -const fn7 = (a) => (b) => (c) => (d) => (e) => (g) => (f) => 3; - -const fn8 = (a) => ({ foo: bar, bar: baz, baz: foo }); -const fn9 = (a) => (b) => ({ foo: bar, bar: baz, baz: foo }); -const fn10 = (a) => (b) => (c) => ({ foo: bar, bar: baz, baz: foo }); -const fn11 = (a) => (b) => (c) => (d) => ({ foo: bar, bar: baz, baz: foo }); -const fn12 = (a) => (b) => (c) => (d) => (e) => ({ - foo: bar, - bar: baz, - baz: foo, -}); -const fn13 = (a) => (b) => (c) => (d) => (e) => (g) => ({ - foo: bar, - bar: baz, - baz: foo, -}); -const fn14 = (a) => (b) => (c) => (d) => (e) => (g) => (f) => ({ - foo: bar, - bar: baz, - baz: foo, -}); - -const curryTest = - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => ({ - foo: argument1, - bar: argument2, - }); - -let curryTest2 = - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - }; - -curryTest2 = - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - }; - -throw (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - }; - -foo( - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => - 3, -); - -foo((argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => ({ - foo: bar, - bar: baz, - baz: foo, - })); - -foo((argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - }); - -( - (argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => - 3 -)(3); - -bar( - foo((argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => ({ - foo: bar, - bar: baz, - })), -); - -const baaaz = - (aaaaa1, bbbbb1) => - (aaaaa2, bbbbb2) => - (aaaaa3, bbbbb3) => - (aaaaa4, bbbbb4) => ({ - foo: bar, - }); - -new Fooooooooooooooooooooooooooooooooooooooooooooooooooo( - (action) => (next) => (next) => (next) => (next) => (next) => (next) => - dispatch(action), -); - -foo?.Fooooooooooooooooooooooooooooooooooooooooooooooooooo( - (action) => (next) => (next) => (next) => (next) => (next) => (next) => - dispatch(action), -); - -foo((action) => (action) => action); - -import((argument1) => - (argument2) => - (argument3) => - (argument4) => - (argument5) => - (argument6) => - (argument7) => - (argument8) => - (argument9) => - (argument10) => - (argument11) => - (argument12) => { - const foo = "foo"; - return foo + "bar"; - }); -``` - - From 63a2e7632ca4b5548ba632f62163744b0f151526 Mon Sep 17 00:00:00 2001 From: Jon Egeland Date: Tue, 28 Nov 2023 04:42:03 +0000 Subject: [PATCH 3/5] handle remaining arrow chain layout diffs --- .../expressions/arrow_function_expression.rs | 50 ++++++++--- .../src/js/expressions/call_arguments.rs | 15 ++-- crates/biome_js_formatter/tests/quick_test.rs | 28 +++++- .../prettier/js/arrows/currying-2.js.snap | 85 ------------------- .../function-body-in-mode-break.js.snap | 66 -------------- 5 files changed, 71 insertions(+), 173 deletions(-) delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/arrows/currying-2.js.snap delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/last-argument-expansion/function-body-in-mode-break.js.snap diff --git a/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs b/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs index 1d2665c7ee38..504e5b3424fa 100644 --- a/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs +++ b/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs @@ -229,7 +229,8 @@ fn format_signature( match arrow.parameters()? { AnyJsArrowFunctionParameters::AnyJsBinding(binding) => { - let should_hug = is_test_call_argument(arrow.syntax())?; + let should_hug = + is_test_call_argument(arrow.syntax())? || is_first_or_last_call_argument; let parentheses_not_needed = can_avoid_parentheses(arrow, f); @@ -277,7 +278,7 @@ fn format_signature( write!( recording, [group(&format_args![ - (!is_first_in_chain).then_some(space()), + maybe_space(!is_first_in_chain), formatted_async_token, group(&formatted_parameters), group(&arrow.return_type_annotation().format()) @@ -536,20 +537,45 @@ impl Format for ArrowChain { write!(f, [soft_line_break()])?; } - let join_signatures = format_with(|f| { + let is_grouped_call_arg_layout = self.options.call_arg_layout.is_some(); + + let join_signatures = format_with(|f: &mut JsFormatter| { let mut is_first_in_chain = true; for arrow in self.arrows() { + if f.context().comments().has_leading_comments(arrow.syntax()) { + // A grouped layout implies that the arrow chain is trying to be rendered + // in a condensend, single-line format (at least the signatures, not + // necessarily the body). In that case, we _need_ to prevent the leading + // comments from inserting line breaks. But if it's _not_ a grouped layout, + // then we want to _force_ the line break so that the leading comments + // don't inadvertently end up on the previous line after the fat arrow. + if is_grouped_call_arg_layout { + write!( + f, + [ + maybe_space(!is_first_in_chain), + format_leading_comments(arrow.syntax()) + ] + )?; + } else { + write!( + f, + [ + (!is_first_in_chain).then_some(soft_line_break_or_space()), + format_leading_comments(arrow.syntax()) + ] + )?; + } + } + write!( f, - [ - format_leading_comments(arrow.syntax()), - format_signature( - arrow, - self.options.call_arg_layout.is_some(), - ancestor_call_expr_or_logical_expr, - is_first_in_chain, - ) - ] + [format_signature( + arrow, + is_grouped_call_arg_layout, + ancestor_call_expr_or_logical_expr, + is_first_in_chain, + )] )?; is_first_in_chain = false; diff --git a/crates/biome_js_formatter/src/js/expressions/call_arguments.rs b/crates/biome_js_formatter/src/js/expressions/call_arguments.rs index bbd0c9fd93d5..1e96f0ddfd19 100644 --- a/crates/biome_js_formatter/src/js/expressions/call_arguments.rs +++ b/crates/biome_js_formatter/src/js/expressions/call_arguments.rs @@ -508,9 +508,14 @@ fn write_grouped_arguments( buffer.into_vec().into_boxed_slice() }; - if grouped_breaks { + // If the grouped content breaks, then we can skip the most_flat variant, + // since we already know that it won't be fitting on a single line. + let variants = if grouped_breaks { write!(f, [expand_parent()])?; - } + vec![middle_variant, most_expanded.into_boxed_slice()] + } else { + vec![most_flat, middle_variant, most_expanded.into_boxed_slice()] + }; // SAFETY: Safe because variants is guaranteed to contain exactly 3 entries: // * most flat @@ -519,11 +524,7 @@ fn write_grouped_arguments( // ... and best fitting only requires the most flat/and expanded. unsafe { f.write_element(FormatElement::BestFitting( - format_element::BestFittingElement::from_vec_unchecked(vec![ - most_flat, - middle_variant, - most_expanded.into_boxed_slice(), - ]), + format_element::BestFittingElement::from_vec_unchecked(variants), )) } } diff --git a/crates/biome_js_formatter/tests/quick_test.rs b/crates/biome_js_formatter/tests/quick_test.rs index d429a98fb5b4..e65e5ed2f0d6 100644 --- a/crates/biome_js_formatter/tests/quick_test.rs +++ b/crates/biome_js_formatter/tests/quick_test.rs @@ -14,9 +14,30 @@ mod language { // use this test check if your snippet prints as you wish, without using a snapshot fn quick_test() { let src = r#" - request.get('https://preview-9992--prettier.netlify.app', head => body => { - console.log(head, body); - }); + /** + * Curried function that ends with a BEM CSS Selector + * + * @param {String} block - the BEM Block you'd like to select. + * @returns {Function} + */ +export const bem = + (block) => + /** + * @param {String} [element] - the BEM Element within that block; if undefined, selects the block itself. + * @returns {Function} + */ + (element) => + /** + * @param {?String} [modifier] - the BEM Modifier for the Block or Element; if undefined, selects the Block or Element unmodified. + * @returns {String} + */ + (modifier) => + [ + ".", + css(block), + element ? `__${css(element)}` : "", + modifier ? `--${css(modifier)}` : "", + ].join(""); "#; let syntax = JsFileSource::tsx(); let tree = parse( @@ -35,6 +56,7 @@ fn quick_test() { let result = doc.print().unwrap(); let source_type = JsFileSource::js_module(); + println!("{}", doc.into_document()); eprintln!("{}", result.as_code()); CheckReformat::new( diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/arrows/currying-2.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/arrows/currying-2.js.snap deleted file mode 100644 index 07de4ece1041..000000000000 --- a/crates/biome_js_formatter/tests/specs/prettier/js/arrows/currying-2.js.snap +++ /dev/null @@ -1,85 +0,0 @@ ---- -source: crates/biome_formatter_test/src/snapshot_builder.rs -info: js/arrows/currying-2.js ---- - -# Input - -```js -const a = - (x) => (y) => (z) => - x / 0.123456789 + (y * calculateSomething(z)) / Math.PI; - -request.get('https://preview-9992--prettier.netlify.app', head => body => { - console.log(head, body); -}); - -request.get('https://preview-9992--prettier.netlify.app', head => body => mody => { - console.log(head, body); -}); - -request.get('https://preview-9992--prettier.netlify.app', head => body => modyLoremIpsumDolorAbstractProviderFactoryServiceModule => { - console.log(head, body); -}); -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Biome -@@ -5,18 +5,14 @@ - console.log(head, body); - }); - --request.get( -- "https://preview-9992--prettier.netlify.app", -- (head) => (body) => (mody) => { -+request.get("https://preview-9992--prettier.netlify.app", (head) => -+ (body) => -+ (mody) => { - console.log(head, body); -- }, --); -+ }); - --request.get( -- "https://preview-9992--prettier.netlify.app", -- (head) => -- (body) => -- (modyLoremIpsumDolorAbstractProviderFactoryServiceModule) => { -- console.log(head, body); -- }, --); -+request.get("https://preview-9992--prettier.netlify.app", (head) => -+ (body) => -+ (modyLoremIpsumDolorAbstractProviderFactoryServiceModule) => { -+ console.log(head, body); -+ }); -``` - -# Output - -```js -const a = (x) => (y) => (z) => - x / 0.123456789 + (y * calculateSomething(z)) / Math.PI; - -request.get("https://preview-9992--prettier.netlify.app", (head) => (body) => { - console.log(head, body); -}); - -request.get("https://preview-9992--prettier.netlify.app", (head) => - (body) => - (mody) => { - console.log(head, body); - }); - -request.get("https://preview-9992--prettier.netlify.app", (head) => - (body) => - (modyLoremIpsumDolorAbstractProviderFactoryServiceModule) => { - console.log(head, body); - }); -``` - - diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/last-argument-expansion/function-body-in-mode-break.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/last-argument-expansion/function-body-in-mode-break.js.snap deleted file mode 100644 index 1e72c52def49..000000000000 --- a/crates/biome_js_formatter/tests/specs/prettier/js/last-argument-expansion/function-body-in-mode-break.js.snap +++ /dev/null @@ -1,66 +0,0 @@ ---- -source: crates/biome_formatter_test/src/snapshot_builder.rs -info: js/last-argument-expansion/function-body-in-mode-break.js ---- - -# Input - -```js -fs.readdirSync(suiteLoc).forEach(function (testName) { - (skip ? it.skip : it)( - testName, - buildTest(binName, testName, opts), - 2_000_000, - ); -}); - -{ - (skip ? it.skip : it)( - testName, - buildTest(binName, testName, opts), - 2_000_000, - ); -} - -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Biome -@@ -1,9 +1,7 @@ - fs.readdirSync(suiteLoc).forEach(function (testName) { -- (skip ? it.skip : it)( -- testName, -- buildTest(binName, testName, opts), -- 2_000_000, -- ); -+ (skip -+ ? it.skip -+ : it)(testName, buildTest(binName, testName, opts), 2_000_000); - }); - - { -``` - -# Output - -```js -fs.readdirSync(suiteLoc).forEach(function (testName) { - (skip - ? it.skip - : it)(testName, buildTest(binName, testName, opts), 2_000_000); -}); - -{ - (skip ? it.skip : it)( - testName, - buildTest(binName, testName, opts), - 2_000_000, - ); -} -``` - - From b4eee45ee136633c35dfdc18e114eb6f25ec2134 Mon Sep 17 00:00:00 2001 From: Jon Egeland Date: Tue, 28 Nov 2023 05:30:11 +0000 Subject: [PATCH 4/5] clippy? --- crates/biome_js_formatter/src/js/bindings/parameters.rs | 2 +- crates/biome_js_formatter/src/js/lists/parameter_list.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/biome_js_formatter/src/js/bindings/parameters.rs b/crates/biome_js_formatter/src/js/bindings/parameters.rs index 8d0e9f4199fe..e706e1c54af2 100644 --- a/crates/biome_js_formatter/src/js/bindings/parameters.rs +++ b/crates/biome_js_formatter/src/js/bindings/parameters.rs @@ -101,7 +101,7 @@ impl Format for FormatAnyJsParameters { f, [ l_paren_token.format(), - format_dangling_comments(&self.inner_syntax()).with_soft_block_indent(), + format_dangling_comments(self.inner_syntax()).with_soft_block_indent(), r_paren_token.format() ] ) diff --git a/crates/biome_js_formatter/src/js/lists/parameter_list.rs b/crates/biome_js_formatter/src/js/lists/parameter_list.rs index 1c6d65a7abf8..50b6608fcde1 100644 --- a/crates/biome_js_formatter/src/js/lists/parameter_list.rs +++ b/crates/biome_js_formatter/src/js/lists/parameter_list.rs @@ -69,11 +69,11 @@ impl Format for FormatJsAnyParameterList<'_> { if is_compact { let mut joiner = f.join_nodes_with_space(); - join_parameter_list(&mut joiner, &self.list, trailing_separator)?; + join_parameter_list(&mut joiner, self.list, trailing_separator)?; joiner.finish() } else { let mut joiner = f.join_nodes_with_soft_line(); - join_parameter_list(&mut joiner, &self.list, trailing_separator)?; + join_parameter_list(&mut joiner, self.list, trailing_separator)?; joiner.finish() } } From a1d7a08fc55911ed83e8da1d4aee0e7da014fca3 Mon Sep 17 00:00:00 2001 From: Jon Egeland Date: Tue, 28 Nov 2023 19:41:53 +0000 Subject: [PATCH 5/5] pr feedback 1 --- .../biome_js_formatter/src/js/bindings/parameters.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/biome_js_formatter/src/js/bindings/parameters.rs b/crates/biome_js_formatter/src/js/bindings/parameters.rs index e706e1c54af2..536f99ff6b98 100644 --- a/crates/biome_js_formatter/src/js/bindings/parameters.rs +++ b/crates/biome_js_formatter/src/js/bindings/parameters.rs @@ -10,7 +10,7 @@ use biome_js_syntax::{ AnyJsFormalParameter, AnyJsParameter, AnyTsType, JsArrowFunctionExpression, JsConstructorParameters, JsParameters, JsSyntaxNode, JsSyntaxToken, }; -use biome_rowan::{AstNode, SyntaxResult}; +use biome_rowan::{declare_node_union, AstNode, SyntaxResult}; #[derive(Debug, Copy, Clone, Default)] pub(crate) struct FormatJsParameters { @@ -50,9 +50,8 @@ impl FormatNodeRule for FormatJsParameters { } } -pub(crate) enum AnyJsParameters { - JsParameters(JsParameters), - JsConstructorParameters(JsConstructorParameters), +declare_node_union! { + pub(crate) AnyJsParameters = JsParameters | JsConstructorParameters } pub(crate) struct FormatAnyJsParameters { @@ -101,7 +100,7 @@ impl Format for FormatAnyJsParameters { f, [ l_paren_token.format(), - format_dangling_comments(self.inner_syntax()).with_soft_block_indent(), + format_dangling_comments(self.parameters_syntax()).with_soft_block_indent(), r_paren_token.format() ] ) @@ -228,7 +227,7 @@ impl FormatAnyJsParameters { } } - fn inner_syntax(&self) -> &JsSyntaxNode { + fn parameters_syntax(&self) -> &JsSyntaxNode { match &self.parameters { AnyJsParameters::JsParameters(parameters) => parameters.syntax(), AnyJsParameters::JsConstructorParameters(parameters) => parameters.syntax(),