From 777f5966e0f71126b4449da833e28c37c23f5876 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 2 Aug 2022 15:20:37 +0200 Subject: [PATCH] feat(rome_js_formatter): Group return type with parameters Implement the same heuristic as Prettier for grouping parameters with the return type annotation. This ensures that a return type will break FIRST before the parameters and the parameters only break if they don't fit on the same line together with the expanded return type annotation. This PR further unifies the formatting of methods and functions to achieve better code reuse. --- crates/rome_formatter/src/prelude.rs | 2 +- crates/rome_js_formatter/src/builders.rs | 12 +- .../src/js/bindings/constructor_parameters.rs | 12 +- .../src/js/bindings/parameters.rs | 12 +- .../js/classes/constructor_class_member.rs | 16 +- .../src/js/classes/method_class_member.rs | 223 ++++++++++++++++-- .../js/declarations/function_declaration.rs | 119 ++++++++-- .../expressions/arrow_function_expression.rs | 4 +- .../src/js/objects/method_object_member.rs | 30 +-- .../auxiliary/method_signature_type_member.rs | 20 +- .../constructor_signature_class_member.rs | 2 +- .../classes/method_signature_class_member.rs | 39 ++- .../declare_function_declaration.rs | 34 +-- .../src/ts/types/function_type.rs | 40 +++- .../typescript/comments/method_types.ts.snap | 12 +- .../comments/type-parameters.ts.snap | 23 +- .../classes/mixinClassesAnnotated.ts.snap | 19 +- .../method/issue-10352-consistency.ts.snap | 97 -------- .../specs/ts/expression/type_member.ts.snap | 6 +- 19 files changed, 401 insertions(+), 321 deletions(-) delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/typescript/method/issue-10352-consistency.ts.snap diff --git a/crates/rome_formatter/src/prelude.rs b/crates/rome_formatter/src/prelude.rs index 518f06b0534..7e796d60179 100644 --- a/crates/rome_formatter/src/prelude.rs +++ b/crates/rome_formatter/src/prelude.rs @@ -1,6 +1,6 @@ pub use crate::builders::*; pub use crate::format_element::*; -pub use crate::format_extensions::{FormatOptional as _, MemoizeFormat}; +pub use crate::format_extensions::{FormatOptional as _, MemoizeFormat, Memoized}; pub use crate::formatter::Formatter; pub use crate::printer::PrinterOptions; pub use crate::token::{ diff --git a/crates/rome_js_formatter/src/builders.rs b/crates/rome_js_formatter/src/builders.rs index 74b61caa4d7..3651f4147f0 100644 --- a/crates/rome_js_formatter/src/builders.rs +++ b/crates/rome_js_formatter/src/builders.rs @@ -320,6 +320,7 @@ pub fn format_delimited<'a, 'content>( content: Argument::new(content), close_token, mode: DelimitedMode::SoftBlockIndent(None), + grouped: true, } } @@ -329,6 +330,7 @@ pub struct FormatDelimited<'a, 'content> { content: Argument<'content, JsFormatContext>, close_token: &'a JsSyntaxToken, mode: DelimitedMode, + grouped: bool, } impl FormatDelimited<'_, '_> { @@ -359,6 +361,12 @@ impl FormatDelimited<'_, '_> { pub fn soft_block_indent_with_group_id(self, group_id: Option) -> Self { self.with_mode(DelimitedMode::SoftBlockIndent(group_id)) } + + /// Prevents the formatter from grouping the content even in soft block or soft block spaces mode. + pub fn ungrouped(mut self) -> Self { + self.grouped = false; + self + } } impl Format for FormatDelimited<'_, '_> { @@ -368,6 +376,7 @@ impl Format for FormatDelimited<'_, '_> { close_token, content, mode, + grouped, } = self; let open_delimiter = format_open_delimiter(open_token); @@ -427,8 +436,8 @@ impl Format for FormatDelimited<'_, '_> { }); match mode { + _ if !grouped => write!(f, [delimited])?, // Group is useless, the block indent would expand it right anyway - DelimitedMode::BlockIndent => write!(f, [delimited])?, DelimitedMode::SoftBlockIndent(group_id) | DelimitedMode::SoftBlockSpaces(group_id) => { match group_id { None => write!(f, [group(&delimited)])?, @@ -437,6 +446,7 @@ impl Format for FormatDelimited<'_, '_> { } } } + DelimitedMode::BlockIndent => write!(f, [delimited])?, }; write!(f, [format_trailing_trivia(close_token)]) diff --git a/crates/rome_js_formatter/src/js/bindings/constructor_parameters.rs b/crates/rome_js_formatter/src/js/bindings/constructor_parameters.rs index 648017d30eb..045efcc7428 100644 --- a/crates/rome_js_formatter/src/js/bindings/constructor_parameters.rs +++ b/crates/rome_js_formatter/src/js/bindings/constructor_parameters.rs @@ -1,6 +1,5 @@ use crate::prelude::*; -use rome_formatter::write; use rome_js_syntax::JsConstructorParameters; use rome_js_syntax::JsConstructorParametersFields; @@ -15,12 +14,9 @@ impl FormatNodeRule for FormatJsConstructorParameters { r_paren_token, } = node.as_fields(); - write!( - f, - [ - format_delimited(&l_paren_token?, ¶meters.format(), &r_paren_token?,) - .soft_block_indent() - ] - ) + format_delimited(&l_paren_token?, ¶meters.format(), &r_paren_token?) + .soft_block_indent() + .ungrouped() + .fmt(f) } } diff --git a/crates/rome_js_formatter/src/js/bindings/parameters.rs b/crates/rome_js_formatter/src/js/bindings/parameters.rs index 7185fc93268..c47712053e7 100644 --- a/crates/rome_js_formatter/src/js/bindings/parameters.rs +++ b/crates/rome_js_formatter/src/js/bindings/parameters.rs @@ -1,6 +1,5 @@ use crate::prelude::*; -use rome_formatter::write; use rome_js_syntax::JsParameters; use rome_js_syntax::JsParametersFields; @@ -15,12 +14,9 @@ impl FormatNodeRule for FormatJsParameters { r_paren_token, } = node.as_fields(); - write!( - f, - [ - format_delimited(&l_paren_token?, &items.format(), &r_paren_token?,) - .soft_block_indent() - ] - ) + format_delimited(&l_paren_token?, &items.format(), &r_paren_token?) + .soft_block_indent() + .ungrouped() + .fmt(f) } } diff --git a/crates/rome_js_formatter/src/js/classes/constructor_class_member.rs b/crates/rome_js_formatter/src/js/classes/constructor_class_member.rs index 4ae87cd4518..ab890620704 100644 --- a/crates/rome_js_formatter/src/js/classes/constructor_class_member.rs +++ b/crates/rome_js_formatter/src/js/classes/constructor_class_member.rs @@ -1,30 +1,20 @@ use crate::prelude::*; +use crate::js::classes::method_class_member::FormatMethodMember; use rome_formatter::write; use rome_js_syntax::JsConstructorClassMember; -use rome_js_syntax::JsConstructorClassMemberFields; #[derive(Debug, Clone, Default)] pub struct FormatJsConstructorClassMember; impl FormatNodeRule for FormatJsConstructorClassMember { fn fmt_fields(&self, node: &JsConstructorClassMember, f: &mut JsFormatter) -> FormatResult<()> { - let JsConstructorClassMemberFields { - modifiers, - name, - parameters, - body, - } = node.as_fields(); - write![ f, [ - modifiers.format(), - space(), - name.format(), - parameters.format(), + node.modifiers().format(), space(), - body.format() + FormatMethodMember::from(node.clone()) ] ] } diff --git a/crates/rome_js_formatter/src/js/classes/method_class_member.rs b/crates/rome_js_formatter/src/js/classes/method_class_member.rs index 17dbf1892ed..81e886c0820 100644 --- a/crates/rome_js_formatter/src/js/classes/method_class_member.rs +++ b/crates/rome_js_formatter/src/js/classes/method_class_member.rs @@ -1,44 +1,217 @@ use crate::prelude::*; +use crate::js::declarations::function_declaration::should_group_function_parameters; use rome_formatter::write; -use rome_js_syntax::JsMethodClassMember; -use rome_js_syntax::JsMethodClassMemberFields; +use rome_js_syntax::{ + JsAnyClassMemberName, JsAnyObjectMemberName, JsConstructorClassMember, JsConstructorParameters, + JsFunctionBody, JsParameters, TsMethodSignatureClassMember, TsMethodSignatureTypeMember, + TsReturnTypeAnnotation, TsTypeParameters, +}; +use rome_js_syntax::{JsMethodClassMember, JsMethodObjectMember, JsSyntaxToken}; +use rome_rowan::{declare_node_union, SyntaxResult}; #[derive(Debug, Clone, Default)] pub struct FormatJsMethodClassMember; impl FormatNodeRule for FormatJsMethodClassMember { fn fmt_fields(&self, node: &JsMethodClassMember, f: &mut JsFormatter) -> FormatResult<()> { - let JsMethodClassMemberFields { - modifiers, - async_token, - star_token, - name, - question_mark_token, - type_parameters, - parameters, - return_type_annotation, - body, - } = node.as_fields(); - - write![f, [modifiers.format(), space(),]]?; - - if let Some(async_token) = async_token { + write![ + f, + [ + node.modifiers().format(), + space(), + FormatMethodMember::from(node.clone()) + ] + ] + } +} + +declare_node_union! { + /// Formats the type parameters, parameters, and return type annotation of a method + pub(crate) FormatMethodMember = + JsMethodClassMember | + JsMethodObjectMember | + JsConstructorClassMember | + TsMethodSignatureClassMember | + TsMethodSignatureTypeMember +} + +impl Format for FormatMethodMember { + fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { + if let Some(async_token) = self.async_token() { write!(f, [async_token.format(), space()])?; } + let type_parameters = self.type_parameters(); + write!( f, [ - star_token.format(), - name.format(), - question_mark_token.format(), + self.star_token().format(), + self.name(), + self.question_mark_token().format(), type_parameters.format(), - parameters.format(), - return_type_annotation.format(), - space(), - body.format() ] - ) + )?; + + write!( + f, + [group(&format_with(|f| { + let parameters = self.parameters()?; + let return_type_annotation = self.return_type_annotation(); + let mut format_return_type_annotation = return_type_annotation.format().memoized(); + + if should_group_function_parameters( + type_parameters.as_ref(), + parameters.len(), + return_type_annotation + .as_ref() + .map(|annotation| annotation.ty()), + &mut format_return_type_annotation, + f, + )? { + write!(f, [group(¶meters)])?; + } else { + write!(f, [parameters])?; + } + + write!(f, [format_return_type_annotation]) + }))] + )?; + + if let Some(body) = self.body()? { + write!(f, [space(), body.format()])?; + } + + Ok(()) + } +} + +impl FormatMethodMember { + fn async_token(&self) -> Option { + match self { + FormatMethodMember::JsMethodClassMember(member) => member.async_token(), + FormatMethodMember::JsMethodObjectMember(member) => member.async_token(), + FormatMethodMember::JsConstructorClassMember(_) => None, + FormatMethodMember::TsMethodSignatureClassMember(signature) => signature.async_token(), + FormatMethodMember::TsMethodSignatureTypeMember(_) => None, + } + } + + fn star_token(&self) -> Option { + match self { + FormatMethodMember::JsMethodClassMember(member) => member.star_token(), + FormatMethodMember::JsMethodObjectMember(member) => member.star_token(), + FormatMethodMember::JsConstructorClassMember(_) => None, + FormatMethodMember::TsMethodSignatureClassMember(_) => None, + FormatMethodMember::TsMethodSignatureTypeMember(_) => None, + } + } + + fn name(&self) -> SyntaxResult { + Ok(match self { + FormatMethodMember::JsMethodClassMember(member) => member.name()?.into(), + FormatMethodMember::JsMethodObjectMember(member) => member.name()?.into(), + FormatMethodMember::JsConstructorClassMember(member) => { + AnyMemberName::from(JsAnyClassMemberName::from(member.name()?)) + } + FormatMethodMember::TsMethodSignatureClassMember(signature) => signature.name()?.into(), + FormatMethodMember::TsMethodSignatureTypeMember(member) => member.name()?.into(), + }) + } + + fn type_parameters(&self) -> Option { + match self { + FormatMethodMember::JsMethodClassMember(member) => member.type_parameters(), + FormatMethodMember::JsMethodObjectMember(member) => member.type_parameters(), + FormatMethodMember::JsConstructorClassMember(_) => None, + FormatMethodMember::TsMethodSignatureClassMember(signature) => { + signature.type_parameters() + } + FormatMethodMember::TsMethodSignatureTypeMember(member) => member.type_parameters(), + } + } + + fn parameters(&self) -> SyntaxResult { + Ok(match self { + FormatMethodMember::JsMethodClassMember(member) => member.parameters()?.into(), + FormatMethodMember::JsMethodObjectMember(member) => member.parameters()?.into(), + FormatMethodMember::JsConstructorClassMember(member) => member.parameters()?.into(), + FormatMethodMember::TsMethodSignatureClassMember(signature) => { + signature.parameters()?.into() + } + FormatMethodMember::TsMethodSignatureTypeMember(member) => member.parameters()?.into(), + }) + } + + fn return_type_annotation(&self) -> Option { + match self { + FormatMethodMember::JsMethodClassMember(member) => member.return_type_annotation(), + FormatMethodMember::JsMethodObjectMember(member) => member.return_type_annotation(), + FormatMethodMember::JsConstructorClassMember(_) => None, + FormatMethodMember::TsMethodSignatureClassMember(signature) => { + signature.return_type_annotation() + } + FormatMethodMember::TsMethodSignatureTypeMember(member) => { + member.return_type_annotation() + } + } + } + + fn question_mark_token(&self) -> Option { + match self { + FormatMethodMember::JsMethodClassMember(member) => member.question_mark_token(), + FormatMethodMember::JsMethodObjectMember(_) => None, + FormatMethodMember::JsConstructorClassMember(_) => None, + FormatMethodMember::TsMethodSignatureClassMember(signature) => { + signature.question_mark_token() + } + FormatMethodMember::TsMethodSignatureTypeMember(member) => member.optional_token(), + } + } + + fn body(&self) -> SyntaxResult> { + Ok(match self { + FormatMethodMember::JsMethodClassMember(member) => Some(member.body()?), + FormatMethodMember::JsMethodObjectMember(member) => Some(member.body()?), + FormatMethodMember::JsConstructorClassMember(member) => Some(member.body()?), + FormatMethodMember::TsMethodSignatureClassMember(_) => None, + FormatMethodMember::TsMethodSignatureTypeMember(_) => None, + }) + } +} + +declare_node_union! { + AnyMemberName = JsAnyClassMemberName | JsAnyObjectMemberName +} + +impl Format for AnyMemberName { + fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { + match self { + AnyMemberName::JsAnyClassMemberName(name) => name.format().fmt(f), + AnyMemberName::JsAnyObjectMemberName(name) => name.format().fmt(f), + } + } +} + +declare_node_union! { + MethodParameters = JsParameters | JsConstructorParameters +} + +impl MethodParameters { + pub fn len(&self) -> usize { + match self { + MethodParameters::JsParameters(parameters) => parameters.items().len(), + MethodParameters::JsConstructorParameters(parameters) => parameters.parameters().len(), + } + } +} + +impl Format for MethodParameters { + fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { + match self { + MethodParameters::JsParameters(parameters) => parameters.format().fmt(f), + MethodParameters::JsConstructorParameters(parameters) => parameters.format().fmt(f), + } } } diff --git a/crates/rome_js_formatter/src/js/declarations/function_declaration.rs b/crates/rome_js_formatter/src/js/declarations/function_declaration.rs index 11c09b06a24..e9d2a504aac 100644 --- a/crates/rome_js_formatter/src/js/declarations/function_declaration.rs +++ b/crates/rome_js_formatter/src/js/declarations/function_declaration.rs @@ -3,7 +3,8 @@ use crate::prelude::*; use rome_formatter::write; use rome_js_syntax::{ JsAnyBinding, JsFunctionBody, JsFunctionDeclaration, JsFunctionExportDefaultDeclaration, - JsFunctionExpression, JsParameters, JsSyntaxToken, TsReturnTypeAnnotation, TsTypeParameters, + JsFunctionExpression, JsParameters, JsSyntaxToken, TsAnyReturnType, + TsDeclareFunctionDeclaration, TsReturnTypeAnnotation, TsType, TsTypeParameters, }; use rome_rowan::{declare_node_union, SyntaxResult}; @@ -17,7 +18,11 @@ impl FormatNodeRule for FormatJsFunctionDeclaration { } declare_node_union! { - pub(crate) FormatFunction = JsFunctionDeclaration | JsFunctionExpression | JsFunctionExportDefaultDeclaration + pub(crate) FormatFunction = + JsFunctionDeclaration | + JsFunctionExpression | + JsFunctionExportDefaultDeclaration | + TsDeclareFunctionDeclaration } impl FormatFunction { @@ -28,6 +33,7 @@ impl FormatFunction { FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { declaration.async_token() } + FormatFunction::TsDeclareFunctionDeclaration(member) => member.async_token(), } } @@ -38,6 +44,7 @@ impl FormatFunction { FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { declaration.function_token() } + FormatFunction::TsDeclareFunctionDeclaration(member) => member.function_token(), } } @@ -48,6 +55,7 @@ impl FormatFunction { FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { declaration.star_token() } + FormatFunction::TsDeclareFunctionDeclaration(_) => None, } } @@ -56,6 +64,7 @@ impl FormatFunction { FormatFunction::JsFunctionDeclaration(declaration) => declaration.id().map(Some), FormatFunction::JsFunctionExpression(expression) => Ok(expression.id()), FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => Ok(declaration.id()), + FormatFunction::TsDeclareFunctionDeclaration(member) => member.id().map(Some), } } @@ -66,6 +75,7 @@ impl FormatFunction { FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { declaration.type_parameters() } + FormatFunction::TsDeclareFunctionDeclaration(member) => member.type_parameters(), } } @@ -76,6 +86,7 @@ impl FormatFunction { FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { declaration.parameters() } + FormatFunction::TsDeclareFunctionDeclaration(member) => member.parameters(), } } @@ -88,15 +99,19 @@ impl FormatFunction { FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { declaration.return_type_annotation() } + FormatFunction::TsDeclareFunctionDeclaration(member) => member.return_type_annotation(), } } - fn body(&self) -> SyntaxResult { - match self { - FormatFunction::JsFunctionDeclaration(declaration) => declaration.body(), - FormatFunction::JsFunctionExpression(expression) => expression.body(), - FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => declaration.body(), - } + fn body(&self) -> SyntaxResult> { + Ok(match self { + FormatFunction::JsFunctionDeclaration(declaration) => Some(declaration.body()?), + FormatFunction::JsFunctionExpression(expression) => Some(expression.body()?), + FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { + Some(declaration.body()?) + } + FormatFunction::TsDeclareFunctionDeclaration(_) => None, + }) } } @@ -120,22 +135,92 @@ impl Format for FormatFunction { } } - write!(f, [self.type_parameters().format()])?; + let type_parameters = self.type_parameters(); + let parameters = self.parameters()?; + let return_type_annotation = self.return_type_annotation(); + + write!(f, [type_parameters.format()])?; write!( f, [group(&format_with(|f| { - write![ + let mut format_return_type_annotation = return_type_annotation.format().memoized(); + let group_parameters = should_group_function_parameters( + type_parameters.as_ref(), + parameters.items().len(), + return_type_annotation + .as_ref() + .map(|annotation| annotation.ty()), + &mut format_return_type_annotation, f, - [ - self.parameters().format(), - self.return_type_annotation().format(), - space() - ] - ] + )?; + + if group_parameters { + write!(f, [group(¶meters.format())])?; + } else { + write!(f, [parameters.format()])?; + } + + write![f, [format_return_type_annotation]] }))] )?; - write!(f, [self.body().format()]) + if let Some(body) = self.body()? { + write!(f, [space(), body.format()])?; + } + + Ok(()) } } + +// Grouping the parameters has the effect that the return type will break first. +pub(crate) fn should_group_function_parameters( + type_parameters: Option<&TsTypeParameters>, + parameter_count: usize, + return_type: Option>, + formatted_return_type: &mut Memoized, JsFormatContext>, + f: &mut JsFormatter, +) -> FormatResult { + let return_type = match return_type { + Some(return_type) => return_type?, + None => return Ok(false), + }; + + if let Some(type_parameters) = type_parameters { + match type_parameters.items().len() { + 0 => { + // fall through + } + 1 => { + // SAFETY: Safe because the length is 1 + let first = type_parameters.items().iter().next().unwrap()?; + + if first.constraint().is_none() || first.default().is_some() { + return Ok(false); + } + } + _ => return Ok(false), + } + } + + let result = if parameter_count != 1 { + false + } else { + // THIS is a hack that is necessary to avoid that the formatter doesn't insert a space + // between `)` and the `:` of the return type annotation IF there's a an inline comment + // after the last comment that has been written. This can be deleted once the comments refactor lands. + let is_last_content_inline_comment = f.state().is_last_content_inline_comment(); + f.state_mut().set_last_content_inline_comment(false); + + let group = matches!( + return_type, + TsAnyReturnType::TsType(TsType::TsObjectType(_) | TsType::TsMappedType(_)) + ) || formatted_return_type.inspect(f)?.will_break(); + + f.state_mut() + .set_last_content_inline_comment(is_last_content_inline_comment); + group + }; + + Ok(result) +} 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 7aa76599076..a87391481df 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 @@ -44,7 +44,9 @@ impl FormatNodeRule for FormatJsArrowFunctionExpressi ) .grouped_with_soft_block_indent()] )?, - JsAnyArrowFunctionParameters::JsParameters(params) => write![f, [params.format()]]?, + JsAnyArrowFunctionParameters::JsParameters(params) => { + write![f, [group(¶ms.format())]]? + } } write![ diff --git a/crates/rome_js_formatter/src/js/objects/method_object_member.rs b/crates/rome_js_formatter/src/js/objects/method_object_member.rs index b5dd46afbe9..8dc8416cc05 100644 --- a/crates/rome_js_formatter/src/js/objects/method_object_member.rs +++ b/crates/rome_js_formatter/src/js/objects/method_object_member.rs @@ -1,39 +1,13 @@ use crate::prelude::*; -use rome_formatter::write; +use crate::js::classes::method_class_member::FormatMethodMember; use rome_js_syntax::JsMethodObjectMember; -use rome_js_syntax::JsMethodObjectMemberFields; #[derive(Debug, Clone, Default)] pub struct FormatJsMethodObjectMember; impl FormatNodeRule for FormatJsMethodObjectMember { fn fmt_fields(&self, node: &JsMethodObjectMember, f: &mut JsFormatter) -> FormatResult<()> { - let JsMethodObjectMemberFields { - async_token, - star_token, - name, - type_parameters, - parameters, - return_type_annotation, - body, - } = node.as_fields(); - - if let Some(async_token) = async_token { - write!(f, [async_token.format(), space()])?; - } - - write![ - f, - [ - star_token.format(), - name.format(), - type_parameters.format(), - parameters.format(), - return_type_annotation.format(), - space(), - body.format(), - ] - ] + FormatMethodMember::from(node.clone()).fmt(f) } } diff --git a/crates/rome_js_formatter/src/ts/auxiliary/method_signature_type_member.rs b/crates/rome_js_formatter/src/ts/auxiliary/method_signature_type_member.rs index c6249e861fc..c7c28934faf 100644 --- a/crates/rome_js_formatter/src/ts/auxiliary/method_signature_type_member.rs +++ b/crates/rome_js_formatter/src/ts/auxiliary/method_signature_type_member.rs @@ -1,8 +1,9 @@ use crate::prelude::*; use crate::utils::FormatTypeMemberSeparator; +use crate::js::classes::method_class_member::FormatMethodMember; use rome_formatter::write; -use rome_js_syntax::{TsMethodSignatureTypeMember, TsMethodSignatureTypeMemberFields}; +use rome_js_syntax::TsMethodSignatureTypeMember; #[derive(Debug, Clone, Default)] pub struct FormatTsMethodSignatureTypeMember; @@ -13,24 +14,11 @@ impl FormatNodeRule for FormatTsMethodSignatureType node: &TsMethodSignatureTypeMember, f: &mut JsFormatter, ) -> FormatResult<()> { - let TsMethodSignatureTypeMemberFields { - name, - optional_token, - type_parameters, - parameters, - return_type_annotation, - separator_token, - } = node.as_fields(); - write![ f, [ - name.format(), - optional_token.format(), - type_parameters.format(), - parameters.format(), - return_type_annotation.format(), - FormatTypeMemberSeparator::new(separator_token.as_ref()) + FormatMethodMember::from(node.clone()), + FormatTypeMemberSeparator::new(node.separator_token().as_ref()) ] ] } diff --git a/crates/rome_js_formatter/src/ts/classes/constructor_signature_class_member.rs b/crates/rome_js_formatter/src/ts/classes/constructor_signature_class_member.rs index 1d1b6da6202..7b06e1f736d 100644 --- a/crates/rome_js_formatter/src/ts/classes/constructor_signature_class_member.rs +++ b/crates/rome_js_formatter/src/ts/classes/constructor_signature_class_member.rs @@ -28,7 +28,7 @@ impl FormatNodeRule for FormatTsConstructorSi modifiers.format(), space(), name.format(), - parameters.format(), + group(¶meters.format()), ), semicolon_token.as_ref() )] diff --git a/crates/rome_js_formatter/src/ts/classes/method_signature_class_member.rs b/crates/rome_js_formatter/src/ts/classes/method_signature_class_member.rs index b38a1cbe39c..b33ca46707f 100644 --- a/crates/rome_js_formatter/src/ts/classes/method_signature_class_member.rs +++ b/crates/rome_js_formatter/src/ts/classes/method_signature_class_member.rs @@ -1,8 +1,9 @@ use crate::prelude::*; use crate::utils::FormatWithSemicolon; -use rome_formatter::{format_args, write}; -use rome_js_syntax::{TsMethodSignatureClassMember, TsMethodSignatureClassMemberFields}; +use crate::js::classes::method_class_member::FormatMethodMember; +use rome_formatter::write; +use rome_js_syntax::TsMethodSignatureClassMember; #[derive(Debug, Clone, Default)] pub struct FormatTsMethodSignatureClassMember; @@ -13,33 +14,21 @@ impl FormatNodeRule for FormatTsMethodSignatureCla node: &TsMethodSignatureClassMember, f: &mut JsFormatter, ) -> FormatResult<()> { - let TsMethodSignatureClassMemberFields { - modifiers, - async_token, - name, - question_mark_token, - type_parameters, - parameters, - return_type_annotation, - semicolon_token, - } = node.as_fields(); + let format_inner = format_with(|f| { + let modifiers = node.modifiers(); + + if !modifiers.is_empty() { + write!(f, [modifiers.format(), space()])?; + } + + FormatMethodMember::from(node.clone()).fmt(f) + }); write!( f, [FormatWithSemicolon::new( - &format_args!( - modifiers.format(), - async_token - .format() - .with_or_empty(|token, f| write![f, [token, space()]]), - space(), - name.format(), - question_mark_token.format(), - type_parameters.format(), - parameters.format(), - return_type_annotation.format(), - ), - semicolon_token.as_ref() + &format_inner, + node.semicolon_token().as_ref() )] ) } diff --git a/crates/rome_js_formatter/src/ts/declarations/declare_function_declaration.rs b/crates/rome_js_formatter/src/ts/declarations/declare_function_declaration.rs index 379f260e85a..27955d243db 100644 --- a/crates/rome_js_formatter/src/ts/declarations/declare_function_declaration.rs +++ b/crates/rome_js_formatter/src/ts/declarations/declare_function_declaration.rs @@ -1,9 +1,9 @@ use crate::prelude::*; use crate::utils::FormatWithSemicolon; +use crate::js::declarations::function_declaration::FormatFunction; use rome_formatter::write; use rome_js_syntax::TsDeclareFunctionDeclaration; -use rome_js_syntax::TsDeclareFunctionDeclarationFields; #[derive(Debug, Clone, Default)] pub struct FormatTsDeclareFunctionDeclaration; @@ -14,39 +14,11 @@ impl FormatNodeRule for FormatTsDeclareFunctionDec node: &TsDeclareFunctionDeclaration, f: &mut JsFormatter, ) -> FormatResult<()> { - let TsDeclareFunctionDeclarationFields { - async_token, - function_token, - id, - type_parameters, - parameters, - return_type_annotation, - semicolon_token, - } = node.as_fields(); - - let declaration = format_with(|f| { - if let Some(async_token) = &async_token { - write!(f, [async_token.format(), space()])?; - } - - write!( - f, - [ - function_token.format(), - space(), - id.format(), - type_parameters.format(), - parameters.format(), - return_type_annotation.format(), - ] - ) - }); - write!( f, [FormatWithSemicolon::new( - &declaration, - semicolon_token.as_ref() + &FormatFunction::from(node.clone()), + node.semicolon_token().as_ref() )] ) } diff --git a/crates/rome_js_formatter/src/ts/types/function_type.rs b/crates/rome_js_formatter/src/ts/types/function_type.rs index b77af812ee0..0240fca7d49 100644 --- a/crates/rome_js_formatter/src/ts/types/function_type.rs +++ b/crates/rome_js_formatter/src/ts/types/function_type.rs @@ -1,5 +1,6 @@ use crate::prelude::*; +use crate::js::declarations::function_declaration::should_group_function_parameters; use rome_formatter::write; use rome_js_syntax::TsFunctionType; use rome_js_syntax::TsFunctionTypeFields; @@ -16,16 +17,35 @@ impl FormatNodeRule for FormatTsFunctionType { return_type, } = node.as_fields(); - write![ - f, - [ - type_parameters.format(), - parameters.format(), - space(), - fat_arrow_token.format(), - space(), - return_type.format() + let format_inner = format_with(|f| { + write![f, [type_parameters.format()]]?; + + let mut format_return_type = return_type.format().memoized(); + let should_group_parameters = should_group_function_parameters( + type_parameters.as_ref(), + parameters.as_ref()?.items().len(), + Some(return_type.clone()), + &mut format_return_type, + f, + )?; + + if should_group_parameters { + write!(f, [group(¶meters.format())])?; + } else { + write!(f, [parameters.format()])?; + } + + write![ + f, + [ + space(), + fat_arrow_token.format(), + space(), + format_return_type + ] ] - ] + }); + + write!(f, [group(&format_inner)]) } } diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/comments/method_types.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/comments/method_types.ts.snap index 90389c1befa..458914df22c 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/comments/method_types.ts.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/typescript/comments/method_types.ts.snap @@ -52,12 +52,14 @@ abstract class Test { ```diff --- Prettier +++ Rome -@@ -1,11 +1,11 @@ +@@ -1,11 +1,13 @@ interface foo1 { - bar3 /* foo */(/* baz */); // bat - bar /* foo */ /* bar */?(/* baz */) /* bat */; - bar2 /* foo */(/* baz */) /* bat */; -+ bar3 /* foo */ (/* baz */); // bat ++ bar3 /* foo */ ( ++ /* baz */ ++ ); // bat + bar /* foo */ ? /* bar */ (/* baz */) /* bat */; + bar2 /* foo */ (/* baz */) /* bat */; } @@ -68,7 +70,7 @@ abstract class Test { } interface foo3 { -@@ -17,23 +17,23 @@ +@@ -17,23 +19,23 @@ } interface foo5 { @@ -106,7 +108,9 @@ abstract class Test { ```js interface foo1 { - bar3 /* foo */ (/* baz */); // bat + bar3 /* foo */ ( + /* baz */ + ); // bat bar /* foo */ ? /* bar */ (/* baz */) /* bat */; bar2 /* foo */ (/* baz */) /* bat */; } diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/comments/type-parameters.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/comments/type-parameters.ts.snap index c676d10814d..b77ce3cf89b 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/comments/type-parameters.ts.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/typescript/comments/type-parameters.ts.snap @@ -69,21 +69,6 @@ type T = < >(); function foo< // comment -@@ -20,12 +20,8 @@ - interface Foo { - < - A, // comment -- >( -- arg, -- ): any; -+ >(arg): any; - } - type T = < - // comment -->( -- arg, --) => any; -+>(arg) => any; ``` # Output @@ -111,11 +96,15 @@ function foo< interface Foo { < A, // comment - >(arg): any; + >( + arg, + ): any; } type T = < // comment ->(arg) => any; +>( + arg, +) => any; ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/conformance/classes/mixinClassesAnnotated.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/conformance/classes/mixinClassesAnnotated.ts.snap index e275c2e8ddd..107a769f01a 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/conformance/classes/mixinClassesAnnotated.ts.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/typescript/conformance/classes/mixinClassesAnnotated.ts.snap @@ -87,19 +87,6 @@ class Thing3 extends Thing2 { class extends superClass { static message = "hello"; print() { -@@ -22,9 +23,9 @@ - } - }; - --function Tagged>( -- superClass: T, --): Constructor & T { -+function Tagged>(superClass: T): -+ & Constructor -+ & T { - class C extends superClass { - _tag: string; - constructor(...args: any[]) { ``` # Output @@ -130,9 +117,9 @@ const Printable = >(superClass: T): } }; -function Tagged>(superClass: T): - & Constructor - & T { +function Tagged>( + superClass: T, +): Constructor & T { class C extends superClass { _tag: string; constructor(...args: any[]) { diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/method/issue-10352-consistency.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/method/issue-10352-consistency.ts.snap deleted file mode 100644 index a2e0d7ad67f..00000000000 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/method/issue-10352-consistency.ts.snap +++ /dev/null @@ -1,97 +0,0 @@ ---- -source: crates/rome_js_formatter/tests/prettier_tests.rs ---- - -# Input - -```js -export interface Store { - getRecord(collectionName: string, documentPath: string): TaskEither>; -} - -export default class StoreImpl extends Service implements Store { - getRecord(collectionName: string, documentPath: string): TaskEither> { - // Do some stuff. - } -} - -export function loadPlugin( - name: string, - dirname: string, -): { filepath: string, value: mixed } { - // ... -} -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Rome -@@ -1,22 +1,22 @@ - export interface Store { -- getRecord( -- collectionName: string, -- documentPath: string, -- ): TaskEither>; -+ getRecord(collectionName: string, documentPath: string): TaskEither< -+ Error, -+ Option -+ >; - } - - export default class StoreImpl extends Service implements Store { -- getRecord( -- collectionName: string, -- documentPath: string, -- ): TaskEither> { -+ getRecord(collectionName: string, documentPath: string): TaskEither< -+ Error, -+ Option -+ > { - // Do some stuff. - } - } - --export function loadPlugin( -- name: string, -- dirname: string, --): { filepath: string; value: mixed } { -+export function loadPlugin(name: string, dirname: string): { -+ filepath: string; -+ value: mixed; -+} { - // ... - } -``` - -# Output - -```js -export interface Store { - getRecord(collectionName: string, documentPath: string): TaskEither< - Error, - Option - >; -} - -export default class StoreImpl extends Service implements Store { - getRecord(collectionName: string, documentPath: string): TaskEither< - Error, - Option - > { - // Do some stuff. - } -} - -export function loadPlugin(name: string, dirname: string): { - filepath: string; - value: mixed; -} { - // ... -} -``` - - - diff --git a/crates/rome_js_formatter/tests/specs/ts/expression/type_member.ts.snap b/crates/rome_js_formatter/tests/specs/ts/expression/type_member.ts.snap index 70c413aa015..d4ee7cbfe0b 100644 --- a/crates/rome_js_formatter/tests/specs/ts/expression/type_member.ts.snap +++ b/crates/rome_js_formatter/tests/specs/ts/expression/type_member.ts.snap @@ -1,6 +1,6 @@ --- source: crates/rome_js_formatter/tests/spec_test.rs -assertion_line: 257 +assertion_line: 259 expression: type_member.ts --- # Input @@ -109,7 +109,9 @@ type F = { deeeeeeeeeeeeeee, deeeeeeeeeeeeeeee, deeeeeeeewweeeeee, - >(loreum: string); + >( + loreum: string, + ); }; type G = {