From 5d204923195038e12fe4dc430090073b4dc1743c Mon Sep 17 00:00:00 2001 From: Denis Bezrukov <6227442+denbezrukov@users.noreply.github.com> Date: Sat, 4 Mar 2023 23:21:24 +0200 Subject: [PATCH] fix(rome_js_formatter): Parity with prettier for many function arguments #4229 --- .../src/js/bindings/formal_parameter.rs | 17 +- .../src/ts/expressions/type_arguments.rs | 23 ++- .../ts/function/function_parameters.ts.snap | 72 ------- .../{ => parameters}/function_parameters.ts | 14 ++ .../parameters/function_parameters.ts.snap | 185 ++++++++++++++++++ .../specs/ts/function/parameters/options.json | 10 + 6 files changed, 237 insertions(+), 84 deletions(-) delete mode 100644 crates/rome_js_formatter/tests/specs/ts/function/function_parameters.ts.snap rename crates/rome_js_formatter/tests/specs/ts/function/{ => parameters}/function_parameters.ts (58%) create mode 100644 crates/rome_js_formatter/tests/specs/ts/function/parameters/function_parameters.ts.snap create mode 100644 crates/rome_js_formatter/tests/specs/ts/function/parameters/options.json diff --git a/crates/rome_js_formatter/src/js/bindings/formal_parameter.rs b/crates/rome_js_formatter/src/js/bindings/formal_parameter.rs index 443a138b305..ca7ee522efb 100644 --- a/crates/rome_js_formatter/src/js/bindings/formal_parameter.rs +++ b/crates/rome_js_formatter/src/js/bindings/formal_parameter.rs @@ -3,8 +3,9 @@ use rome_formatter::write; use crate::utils::FormatInitializerClause; +use crate::js::bindings::parameters::{should_hug_function_parameters, FormatAnyJsParameters}; +use rome_js_syntax::JsFormalParameter; use rome_js_syntax::JsFormalParameterFields; -use rome_js_syntax::{AnyJsBindingPattern, JsFormalParameter}; #[derive(Debug, Clone, Default)] pub(crate) struct FormatJsFormalParameter; @@ -29,10 +30,18 @@ impl FormatNodeRule for FormatJsFormalParameter { ] }); - if let AnyJsBindingPattern::JsObjectBindingPattern(_) = node.binding()? { - write![f, [group(&content)]]?; - } else { + let is_hug_parameter = node + .syntax() + .grand_parent() + .and_then(FormatAnyJsParameters::cast) + .map_or(false, |parameters| { + should_hug_function_parameters(¶meters, f.comments()).unwrap_or(false) + }); + + if is_hug_parameter { write![f, [content]]?; + } else { + write![f, [group(&content)]]?; } write![f, [FormatInitializerClause::new(initializer.as_ref())]] diff --git a/crates/rome_js_formatter/src/ts/expressions/type_arguments.rs b/crates/rome_js_formatter/src/ts/expressions/type_arguments.rs index d174a7b602c..1b3e10f66db 100644 --- a/crates/rome_js_formatter/src/ts/expressions/type_arguments.rs +++ b/crates/rome_js_formatter/src/ts/expressions/type_arguments.rs @@ -1,7 +1,7 @@ use crate::utils::should_hug_type; use crate::{prelude::*, utils::is_object_like_type}; -use rome_formatter::write; use rome_formatter::FormatError::SyntaxError; +use rome_formatter::{format_args, write}; use rome_js_syntax::{ AnyJsExpression, AnyTsType, JsSyntaxKind, JsVariableDeclarator, TsTypeArguments, TsTypeArgumentsFields, @@ -75,17 +75,24 @@ impl FormatNodeRule for FormatTsTypeArguments { let should_inline = !is_arrow_function_variables && (ts_type_argument_list.len() == 0 || first_argument_can_be_hugged_or_is_null_type); - write!(f, [l_angle_token.format(),])?; - if should_inline { - write!(f, [ts_type_argument_list.format()])?; + write!( + f, + [ + l_angle_token.format(), + ts_type_argument_list.format(), + r_angle_token.format() + ] + ) } else { write!( f, - [group(&soft_block_indent(&ts_type_argument_list.format()))] - )?; + [group(&format_args![ + l_angle_token.format(), + soft_block_indent(&ts_type_argument_list.format()), + r_angle_token.format() + ])] + ) } - - write!(f, [r_angle_token.format()]) } } diff --git a/crates/rome_js_formatter/tests/specs/ts/function/function_parameters.ts.snap b/crates/rome_js_formatter/tests/specs/ts/function/function_parameters.ts.snap deleted file mode 100644 index 7f7ad4c17a1..00000000000 --- a/crates/rome_js_formatter/tests/specs/ts/function/function_parameters.ts.snap +++ /dev/null @@ -1,72 +0,0 @@ ---- -source: crates/rome_formatter_test/src/snapshot_builder.rs -info: - test_file: ts/function/function_parameters.ts ---- - -# Input - -```ts -export function formatNumber1( - value: string, - { - a, - b, - c, - formatNumber, - ...props - }: Omit & { - useGrouping?: boolean; - } -): string {} - -export function formatNumber2( - value: string, - { a }: Omit & { - useGrouping?: boolean; - } -): string {} - -``` - - -============================= - -# Outputs - -## Output 1 - ------ -Indent style: Tab -Line width: 80 -Quote style: Double Quotes -Quote properties: As needed -Trailing comma: All -Semicolons: Always ------ - -```ts -export function formatNumber1( - value: string, - { - a, - b, - c, - formatNumber, - ...props - }: Omit & { - useGrouping?: boolean; - }, -): string {} - -export function formatNumber2( - value: string, - { - a, - }: Omit & { - useGrouping?: boolean; - }, -): string {} -``` - - diff --git a/crates/rome_js_formatter/tests/specs/ts/function/function_parameters.ts b/crates/rome_js_formatter/tests/specs/ts/function/parameters/function_parameters.ts similarity index 58% rename from crates/rome_js_formatter/tests/specs/ts/function/function_parameters.ts rename to crates/rome_js_formatter/tests/specs/ts/function/parameters/function_parameters.ts index 1379ea5f3ae..43b3ca4c38e 100644 --- a/crates/rome_js_formatter/tests/specs/ts/function/function_parameters.ts +++ b/crates/rome_js_formatter/tests/specs/ts/function/parameters/function_parameters.ts @@ -17,3 +17,17 @@ export function formatNumber2( useGrouping?: boolean; } ): string {} + + +export const findByDatefindByDatefindByDatefindByDate = + (_, { date }, { req } ) => findByDatefindByDatefindByDatefindByDate; + +export const queryAuditLog = async ({ + startDate, + endDate, + jobId, + src, + type, + }: Filter): Promise => { + +}; diff --git a/crates/rome_js_formatter/tests/specs/ts/function/parameters/function_parameters.ts.snap b/crates/rome_js_formatter/tests/specs/ts/function/parameters/function_parameters.ts.snap new file mode 100644 index 00000000000..da2c6e2ab2d --- /dev/null +++ b/crates/rome_js_formatter/tests/specs/ts/function/parameters/function_parameters.ts.snap @@ -0,0 +1,185 @@ +--- +source: crates/rome_formatter_test/src/snapshot_builder.rs +info: ts/function/parameters/function_parameters.ts +--- + +# Input + +```ts +export function formatNumber1( + value: string, + { + a, + b, + c, + formatNumber, + ...props + }: Omit & { + useGrouping?: boolean; + } +): string {} + +export function formatNumber2( + value: string, + { a }: Omit & { + useGrouping?: boolean; + } +): string {} + + +export const findByDatefindByDatefindByDatefindByDate = + (_, { date }, { req } ) => findByDatefindByDatefindByDatefindByDate; + +export const queryAuditLog = async ({ + startDate, + endDate, + jobId, + src, + type, + }: Filter): Promise => { + +}; + +``` + + +============================= + +# Outputs + +## Output 1 + +----- +Indent style: Tab +Line width: 80 +Quote style: Double Quotes +Quote properties: As needed +Trailing comma: All +Semicolons: Always +----- + +```ts +export function formatNumber1( + value: string, + { + a, + b, + c, + formatNumber, + ...props + }: Omit & { + useGrouping?: boolean; + }, +): string {} + +export function formatNumber2( + value: string, + { + a, + }: Omit & { + useGrouping?: boolean; + }, +): string {} + +export const findByDatefindByDatefindByDatefindByDate = ( + _, + { date }, + { req }, +) => findByDatefindByDatefindByDatefindByDate; + +export const queryAuditLog = async ({ + startDate, + endDate, + jobId, + src, + type, +}: Filter): Promise => {}; +``` + +## Output 2 + +----- +Indent style: Tab +Line width: 100 +Quote style: Double Quotes +Quote properties: As needed +Trailing comma: All +Semicolons: Always +----- + +```ts +export function formatNumber1( + value: string, + { + a, + b, + c, + formatNumber, + ...props + }: Omit & { + useGrouping?: boolean; + }, +): string {} + +export function formatNumber2( + value: string, + { + a, + }: Omit & { + useGrouping?: boolean; + }, +): string {} + +export const findByDatefindByDatefindByDatefindByDate = (_, { date }, { req }) => + findByDatefindByDatefindByDatefindByDate; + +export const queryAuditLog = async ({ + startDate, + endDate, + jobId, + src, + type, +}: Filter): Promise => {}; +``` + +## Output 3 + +----- +Indent style: Tab +Line width: 120 +Quote style: Double Quotes +Quote properties: As needed +Trailing comma: All +Semicolons: Always +----- + +```ts +export function formatNumber1( + value: string, + { + a, + b, + c, + formatNumber, + ...props + }: Omit & { + useGrouping?: boolean; + }, +): string {} + +export function formatNumber2( + value: string, + { + a, + }: Omit & { + useGrouping?: boolean; + }, +): string {} + +export const findByDatefindByDatefindByDatefindByDate = (_, { date }, { req }) => + findByDatefindByDatefindByDatefindByDate; + +export const queryAuditLog = async ({ startDate, endDate, jobId, src, type }: Filter): Promise => {}; +``` + + diff --git a/crates/rome_js_formatter/tests/specs/ts/function/parameters/options.json b/crates/rome_js_formatter/tests/specs/ts/function/parameters/options.json new file mode 100644 index 00000000000..2f6c376d90a --- /dev/null +++ b/crates/rome_js_formatter/tests/specs/ts/function/parameters/options.json @@ -0,0 +1,10 @@ +{ + "cases": [ + { + "line_width": 100 + }, + { + "line_width": 120 + } + ] +}