From d7bff1ca25eec5d7d0beb1059a5eb90511969859 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 518f06b05344..7e796d60179c 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 74b61caa4d70..3651f4147f0a 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 648017d30eb9..045efcc74288 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 7185fc932680..c47712053e74 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 4ae87cd4518c..ab8906207043 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 17dbf1892ed5..81e886c0820e 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 2684adf20ce1..51b0ecc730cd 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, } } @@ -58,6 +66,7 @@ impl FormatFunction { } FormatFunction::JsFunctionExpression(expression) => Ok(expression.id()), FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => Ok(declaration.id()), + FormatFunction::TsDeclareFunctionDeclaration(member) => member.id().map(|id| Some(id)), } } @@ -68,6 +77,7 @@ impl FormatFunction { FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { declaration.type_parameters() } + FormatFunction::TsDeclareFunctionDeclaration(member) => member.type_parameters(), } } @@ -78,6 +88,7 @@ impl FormatFunction { FormatFunction::JsFunctionExportDefaultDeclaration(declaration) => { declaration.parameters() } + FormatFunction::TsDeclareFunctionDeclaration(member) => member.parameters(), } } @@ -90,15 +101,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, + }) } } @@ -122,22 +137,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 7aa765990768..a87391481df0 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 b5dd46afbe98..8dc8416cc05c 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 c6249e861fcf..c7c28934fafd 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 1d1b6da62027..7b06e1f736da 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 b38a1cbe39ce..b33ca46707f5 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 379f260e85a8..27955d243db7 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 b77af812ee00..0240fca7d493 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 90389c1befa2..458914df22c8 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 c676d10814d4..b77ce3cf89bf 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 e275c2e8ddd1..107a769f01ac 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 a2e0d7ad67fe..000000000000 --- 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 70c413aa0150..d4ee7cbfe0b6 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 = {