From 7e0a4e789c9717418a674968fceccd72d4177dbe Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 11 Oct 2022 13:57:50 +0200 Subject: [PATCH 1/2] fix(rome_js_formatter): Fix arrow binding parameter in test call Rome has a special formatting for test calls that ensures that callbacks remain on the same line as the `it` or `describe` even if exceeding the configured print width ``` it('should have the default duration when using the onClose arguments', done => { expect(true); done(); }); ``` This logic has been missing for arrow functions with a single binding parameter (not using parameters). This PR adds the additional check. ## Tests I added a new snapshot test and verified that formatting the ant-design project twice yields no more formatting changes. --- .../expressions/arrow_function_expression.rs | 39 +++++++++++++------ .../js/module/arrow/arrow_test_callback.js | 4 ++ .../module/arrow/arrow_test_callback.js.snap | 29 ++++++++++++++ 3 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 crates/rome_js_formatter/tests/specs/js/module/arrow/arrow_test_callback.js create mode 100644 crates/rome_js_formatter/tests/specs/js/module/arrow/arrow_test_callback.js.snap diff --git a/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs b/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs index ead14a5d7e3..5271155bc9c 100644 --- a/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs @@ -10,13 +10,14 @@ use crate::parentheses::{ update_or_lower_expression_needs_parentheses, NeedsParentheses, }; use crate::utils::function_body::{FormatMaybeCachedFunctionBody, FunctionBodyCacheMode}; +use crate::utils::test_call::is_test_call_expression; use crate::utils::{ resolve_left_most_expression, AssignmentLikeLayout, JsAnyBinaryLikeLeftExpression, }; use rome_js_syntax::{ JsAnyArrowFunctionParameters, JsAnyBindingPattern, JsAnyExpression, JsAnyFormalParameter, JsAnyFunctionBody, JsAnyParameter, JsAnyTemplateElement, JsArrowFunctionExpression, - JsSyntaxKind, JsSyntaxNode, JsTemplate, + JsCallArgumentList, JsCallExpression, JsSyntaxKind, JsSyntaxNode, JsTemplate, }; use rome_rowan::SyntaxResult; @@ -222,17 +223,31 @@ fn format_signature( write!(f, [arrow.type_parameters().format()])?; match arrow.parameters()? { - JsAnyArrowFunctionParameters::JsAnyBinding(binding) => write!( - f, - [ - text("("), - &soft_block_indent(&format_args![ - binding.format(), - if_group_breaks(&text(",")) - ]), - text(")") - ] - )?, + JsAnyArrowFunctionParameters::JsAnyBinding(binding) => { + let call_expression = arrow + .parent::() + .and_then(|args| args.syntax().grand_parent()) + .and_then(JsCallExpression::cast); + + let should_hug = call_expression + .map_or(false, |call| is_test_call_expression(&call) == Ok(true)); + + write!(f, [text("(")])?; + + if should_hug { + write!(f, [binding.format()])?; + } else { + write!( + f, + [&soft_block_indent(&format_args![ + binding.format(), + if_group_breaks(&text(",")) + ])] + )? + } + + write!(f, [text(")")])?; + } JsAnyArrowFunctionParameters::JsParameters(params) => { write!(f, [params.format()])?; } diff --git a/crates/rome_js_formatter/tests/specs/js/module/arrow/arrow_test_callback.js b/crates/rome_js_formatter/tests/specs/js/module/arrow/arrow_test_callback.js new file mode 100644 index 00000000000..ce3698ec647 --- /dev/null +++ b/crates/rome_js_formatter/tests/specs/js/module/arrow/arrow_test_callback.js @@ -0,0 +1,4 @@ +it('should have the default duration when using the onClose arguments', done => { + expect(true); + done(); +}); diff --git a/crates/rome_js_formatter/tests/specs/js/module/arrow/arrow_test_callback.js.snap b/crates/rome_js_formatter/tests/specs/js/module/arrow/arrow_test_callback.js.snap new file mode 100644 index 00000000000..f0b9249daf7 --- /dev/null +++ b/crates/rome_js_formatter/tests/specs/js/module/arrow/arrow_test_callback.js.snap @@ -0,0 +1,29 @@ +--- +source: crates/rome_js_formatter/tests/spec_test.rs +expression: arrow_test_callback.js +--- +# Input +it('should have the default duration when using the onClose arguments', done => { + expect(true); + done(); +}); + +============================= +# Outputs +## Output 1 +----- +Indent style: Tab +Line width: 80 +Quote style: Double Quotes +Quote properties: As needed +----- +it("should have the default duration when using the onClose arguments", (done) => { + expect(true); + done(); +}); + + +## Lines exceeding width of 80 characters + + 1: it("should have the default duration when using the onClose arguments", (done) => { + From c2a593b436f4b35ef08e12402b4a15a5b7695912 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 11 Oct 2022 14:03:22 +0200 Subject: [PATCH 2/2] Extract helper for testing if inside a test call expression --- .../src/js/bindings/parameters.rs | 33 ++++--------------- .../expressions/arrow_function_expression.rs | 12 ++----- .../rome_js_formatter/src/utils/test_call.rs | 12 +++++++ 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/crates/rome_js_formatter/src/js/bindings/parameters.rs b/crates/rome_js_formatter/src/js/bindings/parameters.rs index e2725a21760..138f65d547b 100644 --- a/crates/rome_js_formatter/src/js/bindings/parameters.rs +++ b/crates/rome_js_formatter/src/js/bindings/parameters.rs @@ -5,10 +5,10 @@ use crate::js::lists::parameter_list::{ AnyParameter, FormatJsAnyParameterList, JsAnyParameterList, }; -use crate::utils::test_call::is_test_call_expression; +use crate::utils::test_call::is_test_call_argument; use rome_js_syntax::{ - JsAnyConstructorParameter, JsAnyFormalParameter, JsCallExpression, JsConstructorParameters, - JsParameters, JsSyntaxKind, JsSyntaxToken, TsType, + JsAnyConstructorParameter, JsAnyFormalParameter, JsConstructorParameters, JsParameters, + JsSyntaxToken, TsType, }; use rome_rowan::{declare_node_union, SyntaxResult}; @@ -129,29 +129,10 @@ impl FormatJsAnyParameters { /// 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 { - FormatJsAnyParameters::JsParameters(parameters) => { - match parameters.syntax().grand_parent() { - Some(function_parent) => match function_parent.kind() { - JsSyntaxKind::JS_CALL_ARGUMENT_LIST => { - let arguments = function_parent.parent(); - let call_expression = arguments.and_then(|args| args.parent()); - - match call_expression { - Some(call_expression) - if JsCallExpression::can_cast(call_expression.kind()) => - { - is_test_call_expression(&JsCallExpression::unwrap_cast( - call_expression, - ))? - } - _ => false, - } - } - _ => false, - }, - None => false, - } - } + FormatJsAnyParameters::JsParameters(parameters) => match parameters.syntax().parent() { + Some(function) => is_test_call_argument(&function)?, + None => false, + }, FormatJsAnyParameters::JsConstructorParameters(_) => false, }; diff --git a/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs b/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs index 5271155bc9c..6d046e5069a 100644 --- a/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/arrow_function_expression.rs @@ -10,14 +10,14 @@ use crate::parentheses::{ update_or_lower_expression_needs_parentheses, NeedsParentheses, }; use crate::utils::function_body::{FormatMaybeCachedFunctionBody, FunctionBodyCacheMode}; -use crate::utils::test_call::is_test_call_expression; +use crate::utils::test_call::is_test_call_argument; use crate::utils::{ resolve_left_most_expression, AssignmentLikeLayout, JsAnyBinaryLikeLeftExpression, }; use rome_js_syntax::{ JsAnyArrowFunctionParameters, JsAnyBindingPattern, JsAnyExpression, JsAnyFormalParameter, JsAnyFunctionBody, JsAnyParameter, JsAnyTemplateElement, JsArrowFunctionExpression, - JsCallArgumentList, JsCallExpression, JsSyntaxKind, JsSyntaxNode, JsTemplate, + JsSyntaxKind, JsSyntaxNode, JsTemplate, }; use rome_rowan::SyntaxResult; @@ -224,13 +224,7 @@ fn format_signature( match arrow.parameters()? { JsAnyArrowFunctionParameters::JsAnyBinding(binding) => { - let call_expression = arrow - .parent::() - .and_then(|args| args.syntax().grand_parent()) - .and_then(JsCallExpression::cast); - - let should_hug = call_expression - .map_or(false, |call| is_test_call_expression(&call) == Ok(true)); + let should_hug = is_test_call_argument(arrow.syntax())?; write!(f, [text("(")])?; diff --git a/crates/rome_js_formatter/src/utils/test_call.rs b/crates/rome_js_formatter/src/utils/test_call.rs index 04c8e9f6e5e..d514dc0bf1c 100644 --- a/crates/rome_js_formatter/src/utils/test_call.rs +++ b/crates/rome_js_formatter/src/utils/test_call.rs @@ -2,9 +2,21 @@ use crate::prelude::*; use rome_js_syntax::{ JsAnyArrowFunctionParameters, JsAnyCallArgument, JsAnyExpression, JsAnyFunctionBody, JsAnyLiteralExpression, JsAnyName, JsCallArgumentList, JsCallArguments, JsCallExpression, + JsSyntaxNode, }; use rome_rowan::{SyntaxResult, SyntaxTokenText}; +/// Returns `Ok(true)` if `maybe_argument` is an argument of a [test call expression](is_test_call_expression). +pub(crate) fn is_test_call_argument(maybe_argument: &JsSyntaxNode) -> SyntaxResult { + let call_expression = maybe_argument + .parent() + .and_then(JsCallArgumentList::cast) + .and_then(|args| args.syntax().grand_parent()) + .and_then(JsCallExpression::cast); + + call_expression.map_or(Ok(false), |call| is_test_call_expression(&call)) +} + /// This is a specialised function that checks if the current [call expression] /// resembles a call expression usually used by a testing frameworks. ///