diff --git a/crates/oxc_formatter/src/write/arrow_function_expression.rs b/crates/oxc_formatter/src/write/arrow_function_expression.rs index eb945db8a6bc5..98f57df25e49e 100644 --- a/crates/oxc_formatter/src/write/arrow_function_expression.rs +++ b/crates/oxc_formatter/src/write/arrow_function_expression.rs @@ -15,9 +15,11 @@ use crate::{ options::FormatTrailingCommas, utils::assignment_like::AssignmentLikeLayout, write, - write::parameter_list::has_only_simple_parameters, + write::function::FormatContentWithCacheMode, }; +use super::parameters::has_only_simple_parameters; + #[derive(Clone, Copy)] pub struct FormatJsArrowFunctionExpression<'a, 'b> { arrow: &'b AstNode<'a, ArrowFunctionExpression<'a>>, @@ -29,7 +31,7 @@ pub struct FormatJsArrowFunctionExpressionOptions { pub assignment_layout: Option, pub call_arg_layout: Option, // Determine whether the signature and body should be cached. - pub cache_mode: FunctionBodyCacheMode, + pub cache_mode: FunctionCacheMode, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -52,7 +54,7 @@ impl GroupedCallArgumentLayout { } #[derive(Default, Debug, Clone, Copy)] -pub enum FunctionBodyCacheMode { +pub enum FunctionCacheMode { /// Format the body without caching it or retrieving it from the cache. #[default] NoCache, @@ -852,43 +854,32 @@ fn format_signature<'a, 'b>( arrow: &'b AstNode<'a, ArrowFunctionExpression<'a>>, is_first_or_last_call_argument: bool, is_first_in_chain: bool, - cache_mode: FunctionBodyCacheMode, + cache_mode: FunctionCacheMode, ) -> impl Format<'a> + 'b { format_with(move |f| { - let signatures = format_once(|f| { - write!( - f, - [group(&format_args!( - maybe_space(!is_first_in_chain), - arrow.r#async().then_some("async "), - arrow.type_parameters(), - arrow.params(), - group(&arrow.return_type()) - ))] - ) - }); - - // The [`call_arguments`] will format the argument that can be grouped in different ways until - // find the best layout. So we have to cache the parameters because it never be broken. - let cached_signature = format_once(|f| { - if matches!(cache_mode, FunctionBodyCacheMode::NoCache) { - signatures.fmt(f) - } else if let Some(grouped) = f.context().get_cached_element(&arrow.params.span) { - f.write_element(grouped) - } else { - if let Ok(Some(grouped)) = f.intern(&signatures) { - f.context_mut().cache_element(&arrow.params.span, grouped.clone()); - f.write_element(grouped.clone()); - } - Ok(()) - } + let content = format_once(|f| { + group(&format_args!( + maybe_space(!is_first_in_chain), + arrow.r#async().then_some("async "), + arrow.type_parameters(), + arrow.params(), + format_once(|f| { + let needs_space = arrow.return_type.as_ref().is_some_and(|return_type| { + f.context().comments().has_comment_before(return_type.span.start) + }); + maybe_space(needs_space).fmt(f) + }), + group(&arrow.return_type()) + )) + .fmt(f) }); + let format_head = FormatContentWithCacheMode::new(arrow.params.span, content, cache_mode); if is_first_or_last_call_argument { let mut buffer = RemoveSoftLinesBuffer::new(f); let mut recording = buffer.start_recording(); - write!(recording, cached_signature)?; + write!(recording, format_head)?; if recording.stop().will_break() { return Err(FormatError::PoorLayout); @@ -902,7 +893,7 @@ fn format_signature<'a, 'b>( // 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()), - cached_signature + format_head ] )?; } @@ -925,38 +916,20 @@ pub struct FormatMaybeCachedFunctionBody<'a, 'b> { pub expression: bool, /// If the body should be cached or if the formatter should try to retrieve it from the cache. - pub mode: FunctionBodyCacheMode, -} - -impl<'a> FormatMaybeCachedFunctionBody<'a, '_> { - fn format(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - if self.expression - && let AstNodes::ExpressionStatement(s) = - &self.body.statements().first().unwrap().as_ast_nodes() - { - return s.expression().fmt(f); - } - self.body.fmt(f) - } + pub mode: FunctionCacheMode, } impl<'a> Format<'a> for FormatMaybeCachedFunctionBody<'a, '_> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - match self.mode { - FunctionBodyCacheMode::NoCache => self.format(f), - FunctionBodyCacheMode::Cache => { - if let Some(cached) = f.context().get_cached_element(&self.body.span) { - f.write_element(cached) - } else { - match f.intern(&format_once(|f| self.format(f)))? { - Some(interned) => { - f.context_mut().cache_element(&self.body.span, interned.clone()); - f.write_element(interned) - } - None => Ok(()), - } - } + let content = format_once(|f| { + if self.expression + && let AstNodes::ExpressionStatement(s) = + &self.body.statements().first().unwrap().as_ast_nodes() + { + return s.expression().fmt(f); } - } + self.body.fmt(f) + }); + FormatContentWithCacheMode::new(self.body.span, content, self.mode).fmt(f) } } diff --git a/crates/oxc_formatter/src/write/call_arguments.rs b/crates/oxc_formatter/src/write/call_arguments.rs index 8b96aad77cd51..563bc8b76bb1b 100644 --- a/crates/oxc_formatter/src/write/call_arguments.rs +++ b/crates/oxc_formatter/src/write/call_arguments.rs @@ -25,17 +25,17 @@ use crate::{ write::{ FormatFunctionOptions, arrow_function_expression::is_multiline_template_starting_on_same_line, - parameter_list::has_only_simple_parameters, }, }; use super::{ array_element_list::can_concisely_print_array_list, arrow_function_expression::{ - FormatJsArrowFunctionExpression, FormatJsArrowFunctionExpressionOptions, - FunctionBodyCacheMode, GroupedCallArgumentLayout, + FormatJsArrowFunctionExpression, FormatJsArrowFunctionExpressionOptions, FunctionCacheMode, + GroupedCallArgumentLayout, }, function, + parameters::has_only_simple_parameters, }; impl<'a> Format<'a> for AstNode<'a, ArenaVec<'a, Argument<'a>>> { @@ -573,7 +573,7 @@ fn write_grouped_arguments<'a>( has_cached = true; return function.fmt_with_options( FormatFunctionOptions { - cache_mode: FunctionBodyCacheMode::Cache, + cache_mode: FunctionCacheMode::Cache, ..Default::default() }, f, @@ -583,7 +583,7 @@ fn write_grouped_arguments<'a>( has_cached = true; return arrow.fmt_with_options( FormatJsArrowFunctionExpressionOptions { - cache_mode: FunctionBodyCacheMode::Cache, + cache_mode: FunctionCacheMode::Cache, ..FormatJsArrowFunctionExpressionOptions::default() }, f, @@ -801,7 +801,7 @@ impl<'a> Format<'a> for FormatGroupedFirstArgument<'a, '_> { AstNodes::ArrowFunctionExpression(arrow) => with_token_tracking_disabled(f, |f| { arrow.fmt_with_options( FormatJsArrowFunctionExpressionOptions { - cache_mode: FunctionBodyCacheMode::Cache, + cache_mode: FunctionCacheMode::Cache, call_arg_layout: Some(GroupedCallArgumentLayout::GroupedFirstArgument), ..FormatJsArrowFunctionExpressionOptions::default() }, @@ -835,7 +835,7 @@ impl<'a> Format<'a> for FormatGroupedLastArgument<'a, '_> { with_token_tracking_disabled(f, |f| { function.fmt_with_options( FormatFunctionOptions { - cache_mode: FunctionBodyCacheMode::Cache, + cache_mode: FunctionCacheMode::Cache, call_argument_layout: Some( GroupedCallArgumentLayout::GroupedLastArgument, ), @@ -848,7 +848,7 @@ impl<'a> Format<'a> for FormatGroupedLastArgument<'a, '_> { AstNodes::ArrowFunctionExpression(arrow) => with_token_tracking_disabled(f, |f| { arrow.fmt_with_options( FormatJsArrowFunctionExpressionOptions { - cache_mode: FunctionBodyCacheMode::Cache, + cache_mode: FunctionCacheMode::Cache, call_arg_layout: Some(GroupedCallArgumentLayout::GroupedLastArgument), ..FormatJsArrowFunctionExpressionOptions::default() }, diff --git a/crates/oxc_formatter/src/write/function.rs b/crates/oxc_formatter/src/write/function.rs index 2261c97e8da2c..7e40f827f5c9d 100644 --- a/crates/oxc_formatter/src/write/function.rs +++ b/crates/oxc_formatter/src/write/function.rs @@ -4,7 +4,7 @@ use oxc_ast::ast::*; use super::{ FormatWrite, - arrow_function_expression::{FunctionBodyCacheMode, GroupedCallArgumentLayout}, + arrow_function_expression::{FunctionCacheMode, GroupedCallArgumentLayout}, block_statement::is_empty_block, }; use crate::{ @@ -26,7 +26,7 @@ use crate::{ pub struct FormatFunctionOptions { pub call_argument_layout: Option, // Determine whether the signature and body should be cached. - pub cache_mode: FunctionBodyCacheMode, + pub cache_mode: FunctionCacheMode, } pub struct FormatFunction<'a, 'b> { @@ -44,47 +44,35 @@ impl<'a> Deref for FormatFunction<'a, '_> { impl<'a> FormatWrite<'a> for FormatFunction<'a, '_> { fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - if self.declare() { - write!(f, ["declare", space()])?; - } - - if self.r#async() { - write!(f, ["async", space()])?; - } - - write!( - f, - [ - "function", - self.generator().then_some("*"), - space(), - self.id(), - group(&self.type_parameters()), - ] - ); - - // The [`call_arguments`] will format the argument that can be grouped in different ways until - // find the best layout. So we have to cache the parameters because it never be broken. - let cached_signature = format_once(|f| { - if matches!(self.options.cache_mode, FunctionBodyCacheMode::NoCache) { - self.params().fmt(f) - } else if let Some(grouped) = f.context().get_cached_element(&self.params.span) { - f.write_element(grouped) - } else { - if let Ok(Some(grouped)) = f.intern(&self.params()) { - f.context_mut().cache_element(&self.params.span, grouped.clone()); - f.write_element(grouped.clone()); - } - Ok(()) - } + let head = format_once(|f| { + write!( + f, + [ + self.declare.then_some("declare "), + self.r#async.then_some("async "), + "function", + self.generator().then_some("*"), + space(), + self.id(), + group(&self.type_parameters()), + ] + ) }); + FormatContentWithCacheMode::new(self.span, head, self.options.cache_mode).fmt(f)?; - let format_parameters = format_with(|f: &mut Formatter<'_, 'a>| { + let format_parameters = FormatContentWithCacheMode::new( + self.params.span, + self.params(), + self.options.cache_mode, + ) + .memoized(); + + let mut format_parameters = format_once(|f: &mut Formatter<'_, 'a>| { if self.options.call_argument_layout.is_some() { let mut buffer = RemoveSoftLinesBuffer::new(f); let mut recording = buffer.start_recording(); - write!(recording, cached_signature)?; + write!(recording, format_parameters)?; let recorded = recording.stop(); if recorded.will_break() { @@ -93,20 +81,38 @@ impl<'a> FormatWrite<'a> for FormatFunction<'a, '_> { Ok(()) } else { - cached_signature.fmt(f) + format_parameters.fmt(f) } - }); + }) + .memoized(); + + let mut format_return_type = self + .return_type() + .map(|return_type| { + let content = format_once(move |f| { + let needs_space = + f.context().comments().has_comment_before(return_type.span.start); + write!(f, [maybe_space(needs_space), return_type]) + }); + FormatContentWithCacheMode::new(return_type.span, content, self.options.cache_mode) + }) + .memoized(); write!( f, - [group(&format_with(|f| { + [group(&format_once(|f| { let params = &self.params; - let mut format_return_type_annotation = self.return_type().memoized(); + // Inspect early, in case the `return_type` is formatted before `parameters` + // in `should_group_function_parameters`. + format_parameters.inspect(f)?; + let group_parameters = should_group_function_parameters( self.type_parameters.as_deref(), - params.items.len() + usize::from(params.rest.is_some()), + params.items.len() + + usize::from(params.rest.is_some()) + + usize::from(self.this_param.is_some()), self.return_type.as_deref(), - &mut format_return_type_annotation, + &mut format_return_type, f, )?; @@ -116,7 +122,7 @@ impl<'a> FormatWrite<'a> for FormatFunction<'a, '_> { write!(f, [format_parameters])?; } - write!(f, [format_return_type_annotation]) + write!(f, [format_return_type]) }))] )?; @@ -180,16 +186,11 @@ pub fn should_group_function_parameters<'a>( formatted_return_type: &mut Memoized<'a, impl Format<'a>>, f: &mut Formatter<'_, 'a>, ) -> FormatResult { - let return_type = match return_type { - Some(return_type) => &return_type.type_annotation, - None => return Ok(false), - }; - if let Some(type_parameters) = type_parameters { - let mut params = type_parameters.params.iter(); - match params.next() { - None => {} // fall through - Some(first) if params.count() == 0 => { + match type_parameters.params.len() { + 0 => {} // fall through + 1 => { + let first = type_parameters.params.iter().next().unwrap(); if first.constraint.is_some() || first.default.is_some() { return Ok(false); } @@ -198,7 +199,52 @@ pub fn should_group_function_parameters<'a>( } } - Ok(parameter_count != 1 - && (matches!(return_type, TSType::TSLiteralType(_) | TSType::TSMappedType(_)) + let return_type = match return_type { + Some(return_type) => &return_type.type_annotation, + None => return Ok(false), + }; + + Ok(parameter_count == 1 + && (matches!(return_type, TSType::TSTypeLiteral(_) | TSType::TSMappedType(_)) || formatted_return_type.inspect(f)?.will_break())) } + +/// A wrapper that formats content and caches the result based on the given cache mode. +/// +/// It is useful in cases like in [`super::call_arguments`] because it allows printing a node +/// a few times to find a proper layout. +/// However, the current architecture of the formatter isn't able to do things like this, +/// because it will cause the comments printed after the first printing to be lost in the +/// subsequent printing, because comments only can be printed once. +/// This wrapper solves this problem by caching the result of the first printing +/// and reusing it in the subsequent printing. +pub struct FormatContentWithCacheMode { + key: Span, + content: T, + cache_mode: FunctionCacheMode, +} + +impl FormatContentWithCacheMode { + pub fn new(key: Span, content: T, cache_mode: FunctionCacheMode) -> Self { + Self { key, content, cache_mode } + } +} + +impl<'a, T> Format<'a> for FormatContentWithCacheMode +where + T: Format<'a>, +{ + fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { + if matches!(self.cache_mode, FunctionCacheMode::NoCache) { + self.content.fmt(f) + } else if let Some(grouped) = f.context().get_cached_element(&self.key) { + f.write_element(grouped) + } else { + if let Ok(Some(grouped)) = f.intern(&self.content) { + f.context_mut().cache_element(&self.key, grouped.clone()); + f.write_element(grouped.clone()); + } + Ok(()) + } + } +} diff --git a/crates/oxc_formatter/src/write/mod.rs b/crates/oxc_formatter/src/write/mod.rs index bbb25258a69fb..407f3f9ed5ae7 100644 --- a/crates/oxc_formatter/src/write/mod.rs +++ b/crates/oxc_formatter/src/write/mod.rs @@ -19,7 +19,7 @@ mod mapped_type; mod member_expression; mod object_like; mod object_pattern_like; -mod parameter_list; +mod parameters; mod program; mod return_or_throw_statement; mod semicolon; @@ -72,14 +72,14 @@ use crate::{ suppressed::FormatSuppressedNode, }, write, - write::parameter_list::{can_avoid_parentheses, should_hug_function_parameters}, + write::parameters::{can_avoid_parentheses, should_hug_function_parameters}, }; use self::{ array_expression::FormatArrayExpression, object_like::ObjectLike, object_pattern_like::ObjectPatternLike, - parameter_list::{ParameterLayout, ParameterList}, + parameters::{ParameterLayout, ParameterList}, semicolon::OptionalSemicolon, type_parameters::{FormatTSTypeParameters, FormatTSTypeParametersOptions}, utils::{ @@ -941,102 +941,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, BindingRestElement<'a>> { } } -impl<'a> FormatWrite<'a> for AstNode<'a, FormalParameters<'a>> { - fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - let comments = f.context().comments().comments_before(self.span.start); - if !comments.is_empty() { - write!(f, [space(), FormatTrailingComments::Comments(comments)])?; - } - - let parentheses_not_needed = if let AstNodes::ArrowFunctionExpression(arrow) = self.parent { - can_avoid_parentheses(arrow, f) - } else { - false - }; - - let has_any_decorated_parameter = - self.items.iter().any(|param| !param.decorators.is_empty()); - - let can_hug = should_hug_function_parameters(self, parentheses_not_needed, f) - && !has_any_decorated_parameter; - - let layout = if !self.has_parameter() { - ParameterLayout::NoParameters - } else if can_hug || { - // `self.parent`: Function - // `self.parent.parent()`: Argument - // `self.parent.parent().parent()` CallExpression - if let AstNodes::CallExpression(call) = self.parent.parent().parent() { - is_test_call_expression(call) - } else { - false - } - } { - ParameterLayout::Hug - } else { - ParameterLayout::Default - }; - - if !parentheses_not_needed { - write!(f, "(")?; - } - - match layout { - ParameterLayout::NoParameters => { - write!(f, format_dangling_comments(self.span()).with_soft_block_indent())?; - } - ParameterLayout::Hug => { - write!(f, ParameterList::with_layout(self, layout))?; - } - ParameterLayout::Default => { - write!(f, soft_block_indent(&ParameterList::with_layout(self, layout)))?; - } - } - - if !parentheses_not_needed { - write!(f, ")")?; - } - - Ok(()) - } -} - -impl<'a> FormatWrite<'a> for AstNode<'a, FormalParameter<'a>> { - fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - let content = format_with(|f| { - if let Some(accessibility) = self.accessibility() { - write!(f, [accessibility.as_str(), space()])?; - } - if self.r#override() { - write!(f, ["override", space()])?; - } - if self.readonly() { - write!(f, ["readonly", space()])?; - } - write!(f, self.pattern()) - }); - - // TODO - let is_hug_parameter = false; - // let is_hug_parameter = node - // .syntax() - // .grand_parent() - // .and_then(FormatAnyJsParameters::cast) - // .is_some_and(|parameters| { - // should_hug_function_parameters(¶meters, f.comments(), false).unwrap_or(false) - // }); - - let decorators = self.decorators(); - if is_hug_parameter && decorators.is_empty() { - write!(f, [decorators, content]) - } else if decorators.is_empty() { - write!(f, [decorators, group(&content)]) - } else { - write!(f, [group(&decorators), group(&content)]) - } - } -} - impl<'a> FormatWrite<'a, FormatJsArrowFunctionExpressionOptions> for AstNode<'a, ArrowFunctionExpression<'a>> { @@ -1133,16 +1037,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, RegExpLiteral<'a>> { } } -impl<'a> FormatWrite<'a> for AstNode<'a, TSThisParameter<'a>> { - fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - write!(f, "this")?; - if let Some(type_annotation) = self.type_annotation() { - type_annotation.fmt(f); - } - Ok(()) - } -} - impl<'a> FormatWrite<'a> for AstNode<'a, TSEnumDeclaration<'a>> { fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { if self.declare() { diff --git a/crates/oxc_formatter/src/write/object_like.rs b/crates/oxc_formatter/src/write/object_like.rs index 3c306a6be92b6..4efdee383cdc8 100644 --- a/crates/oxc_formatter/src/write/object_like.rs +++ b/crates/oxc_formatter/src/write/object_like.rs @@ -10,6 +10,7 @@ use crate::{ generated::ast_nodes::{AstNode, AstNodes}, options::Expand, write, + write::parameters::should_hug_function_parameters, }; #[derive(Clone, Copy)] @@ -26,25 +27,35 @@ impl<'a> ObjectLike<'a, '_> { } } - fn should_hug(&self) -> bool { + fn should_hug(&self, f: &Formatter<'_, 'a>) -> bool { // Check if the object type is the type annotation of the only parameter in a function. // This prevents breaking object properties in cases like: // const fn = ({ foo }: { foo: string }) => { ... }; - match self { - Self::TSTypeLiteral(node) => { - // Check if parent is TSTypeAnnotation - matches!(node.parent, AstNodes::TSTypeAnnotation(type_ann) if { - // Check if that parent is FormalParameter - matches!(type_ann.parent, AstNodes::FormalParameter(param) if { - // Check if that parent is FormalParameters with only one item - matches!(param.parent, AstNodes::FormalParameters(params) if { - params.items.len() == 1 + matches!(self, Self::TSTypeLiteral(node) if { + // Check if parent is TSTypeAnnotation + matches!(node.parent, AstNodes::TSTypeAnnotation(type_ann) if { + match &type_ann.parent { + AstNodes::FormalParameter(param) => { + let AstNodes::FormalParameters(parameters) = ¶m.parent else { + unreachable!() + }; + let this_param = if let AstNodes::Function(function) = parameters.parent { + function.this_param() + } else { + None + }; + should_hug_function_parameters(parameters, this_param, false, f) + + } + AstNodes::TSThisParameter(param) => { + matches!(param.parent, AstNodes::Function(func) if { + should_hug_function_parameters(func.params(), Some(param), false, f) }) - }) - }) - } - Self::ObjectExpression(node) => false, - } + }, + _ => false, + } + }) + }) } fn members_have_leading_newline(&self, f: &Formatter<'_, 'a>) -> bool { @@ -95,7 +106,7 @@ impl<'a> Format<'a> for ObjectLike<'a, '_> { // const fn = ({ foo }: { foo: string }) => { ... }; // ^ do not break properties here // ``` - let should_hug = self.should_hug(); + let should_hug = self.should_hug(f); let inner = soft_block_indent_with_maybe_space(&members, should_insert_space_around_brackets); diff --git a/crates/oxc_formatter/src/write/object_pattern_like.rs b/crates/oxc_formatter/src/write/object_pattern_like.rs index d2de82c85bb64..58590344f535b 100644 --- a/crates/oxc_formatter/src/write/object_pattern_like.rs +++ b/crates/oxc_formatter/src/write/object_pattern_like.rs @@ -8,7 +8,7 @@ use crate::{ }, generated::ast_nodes::{AstNode, AstNodes}, write, - write::parameter_list::should_hug_function_parameters, + write::parameters::should_hug_function_parameters, }; use super::{ @@ -38,12 +38,8 @@ impl<'a> ObjectPatternLike<'a, '_> { fn is_inline(&self, f: &Formatter<'_, 'a>) -> bool { match self { - Self::ObjectPattern(node) => { - matches!(node.parent, AstNodes::FormalParameter(_)) || self.is_hug_parameter(f) - } - Self::ObjectAssignmentTarget(node) => { - matches!(node.parent, AstNodes::FormalParameter(_)) - } + Self::ObjectPattern(node) => self.is_hug_parameter(f), + Self::ObjectAssignmentTarget(node) => false, } } @@ -53,9 +49,7 @@ impl<'a> ObjectPatternLike<'a, '_> { Self::ObjectPattern(node) => { matches!(node.parent, AstNodes::CatchParameter(_) | AstNodes::FormalParameter(_)) } - Self::ObjectAssignmentTarget(node) => { - matches!(node.parent, AstNodes::CatchParameter(_) | AstNodes::FormalParameter(_)) - } + Self::ObjectAssignmentTarget(node) => false, }; if parent_is_catch_or_parameter { @@ -104,16 +98,18 @@ impl<'a> ObjectPatternLike<'a, '_> { } fn is_hug_parameter(&self, f: &Formatter<'_, 'a>) -> bool { - match self { - Self::ObjectAssignmentTarget(_) => false, - Self::ObjectPattern(node) => { - matches!(node.parent, AstNodes::FormalParameter(param) if { - matches!(param.parent, AstNodes::FormalParameters(params) if { - should_hug_function_parameters(params, false, f) - }) + matches!(self, Self::ObjectPattern(node) if { + matches!(node.parent, AstNodes::FormalParameter(param) if { + matches!(param.parent, AstNodes::FormalParameters(params) if { + let this_param = if let AstNodes::Function(function) = params.parent { + function.this_param() + } else { + None + }; + should_hug_function_parameters(params, this_param, false, f) }) - } - } + }) + }) } fn layout(&self, f: &Formatter<'_, 'a>) -> ObjectPatternLayout { diff --git a/crates/oxc_formatter/src/write/parameter_list.rs b/crates/oxc_formatter/src/write/parameters.rs similarity index 55% rename from crates/oxc_formatter/src/write/parameter_list.rs rename to crates/oxc_formatter/src/write/parameters.rs index a6e02e73b1bf3..60680560719fd 100644 --- a/crates/oxc_formatter/src/write/parameter_list.rs +++ b/crates/oxc_formatter/src/write/parameters.rs @@ -2,12 +2,126 @@ use oxc_ast::ast::*; use oxc_span::GetSpan; use crate::{ - formatter::{Format, FormatResult, Formatter, prelude::*, separated::FormatSeparatedIter}, - generated::ast_nodes::{AstNode, AstNodeIterator}, + format_args, + formatter::{ + Format, FormatResult, Formatter, prelude::*, separated::FormatSeparatedIter, + trivia::FormatTrailingComments, + }, + generated::ast_nodes::{AstNode, AstNodeIterator, AstNodes}, options::{FormatTrailingCommas, TrailingSeparator}, + utils::call_expression::is_test_call_expression, + write, }; +use super::FormatWrite; + +impl<'a> FormatWrite<'a> for AstNode<'a, FormalParameters<'a>> { + fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { + let comments = f.context().comments().comments_before(self.span.start); + if !comments.is_empty() { + write!(f, [space(), FormatTrailingComments::Comments(comments)])?; + } + + let parentheses_not_needed = if let AstNodes::ArrowFunctionExpression(arrow) = self.parent { + can_avoid_parentheses(arrow, f) + } else { + false + }; + + let this_param = if let AstNodes::Function(function) = self.parent { + function.this_param() + } else { + None + }; + + let has_any_decorated_parameter = + self.items.iter().any(|param| !param.decorators.is_empty()); + + let can_hug = should_hug_function_parameters(self, this_param, parentheses_not_needed, f) + && !has_any_decorated_parameter; + + let layout = if !self.has_parameter() && this_param.is_none() { + ParameterLayout::NoParameters + } else if can_hug || { + // `self.parent`: Function + // `self.parent.parent()`: Argument + // `self.parent.parent().parent()` CallExpression + if let AstNodes::CallExpression(call) = self.parent.parent().parent() { + is_test_call_expression(call) + } else { + false + } + } { + ParameterLayout::Hug + } else { + ParameterLayout::Default + }; + + if !parentheses_not_needed { + write!(f, "(")?; + } + + match layout { + ParameterLayout::NoParameters => { + write!(f, format_dangling_comments(self.span()).with_soft_block_indent())?; + } + ParameterLayout::Hug => { + write!(f, ParameterList::with_layout(self, this_param, layout))?; + } + ParameterLayout::Default => { + write!( + f, + soft_block_indent(&format_args!( + &ParameterList::with_layout(self, this_param, layout), + format_once(|f| { + let comments = f.context().comments().comments_before(self.span.end); + write!(f, [FormatTrailingComments::Comments(comments)]) + }) + )) + ); + } + } + + if !parentheses_not_needed { + write!(f, [")"])?; + } + + Ok(()) + } +} + +impl<'a> FormatWrite<'a> for AstNode<'a, FormalParameter<'a>> { + fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { + let content = format_with(|f| { + if let Some(accessibility) = self.accessibility() { + write!(f, [accessibility.as_str(), space()])?; + } + if self.r#override() { + write!(f, ["override", space()])?; + } + if self.readonly() { + write!(f, ["readonly", space()])?; + } + write!(f, self.pattern()) + }); + + let decorators = self.decorators(); + if decorators.is_empty() { + write!(f, [decorators, content]) + } else { + write!(f, [group(&decorators), group(&content)]) + } + } +} + +impl<'a> FormatWrite<'a> for AstNode<'a, TSThisParameter<'a>> { + fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { + write!(f, ["this", self.type_annotation()]) + } +} + enum Parameter<'a, 'b> { + This(&'b AstNode<'a, TSThisParameter<'a>>), FormalParameter(&'b AstNode<'a, FormalParameter<'a>>), Rest(&'b AstNode<'a, BindingRestElement<'a>>), } @@ -15,6 +129,7 @@ enum Parameter<'a, 'b> { impl GetSpan for Parameter<'_, '_> { fn span(&self) -> Span { match self { + Self::This(param) => param.span(), Self::FormalParameter(param) => param.span(), Self::Rest(e) => e.span(), } @@ -24,6 +139,7 @@ impl GetSpan for Parameter<'_, '_> { impl<'a> Format<'a> for Parameter<'a, '_> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { match self { + Self::This(param) => param.fmt(f), Self::FormalParameter(param) => param.fmt(f), Self::Rest(e) => e.fmt(f), } @@ -31,13 +147,14 @@ impl<'a> Format<'a> for Parameter<'a, '_> { } struct FormalParametersIter<'a, 'b> { + this: Option<&'b AstNode<'a, TSThisParameter<'a>>>, params: AstNodeIterator<'a, FormalParameter<'a>>, rest: Option<&'b AstNode<'a, BindingRestElement<'a>>>, } -impl<'a, 'b> From<&'b AstNode<'a, FormalParameters<'a>>> for FormalParametersIter<'a, 'b> { - fn from(value: &'b AstNode<'a, FormalParameters<'a>>) -> Self { - Self { params: value.items().iter(), rest: value.rest() } +impl<'a, 'b> From<&'b ParameterList<'a, 'b>> for FormalParametersIter<'a, 'b> { + fn from(value: &'b ParameterList<'a, 'b>) -> Self { + Self { this: value.this, params: value.list.items().iter(), rest: value.list.rest() } } } @@ -45,15 +162,18 @@ impl<'a, 'b> Iterator for FormalParametersIter<'a, 'b> { type Item = Parameter<'a, 'b>; fn next(&mut self) -> Option { - self.params - .next() - .map(Parameter::FormalParameter) - .or_else(|| self.rest.take().map(Parameter::Rest)) + self.this.take().map(Parameter::This).or_else(|| { + self.params + .next() + .map(Parameter::FormalParameter) + .or_else(|| self.rest.take().map(Parameter::Rest)) + }) } } pub struct ParameterList<'a, 'b> { list: &'b AstNode<'a, FormalParameters<'a>>, + this: Option<&'b AstNode<'a, TSThisParameter<'a>>>, layout: Option, } @@ -93,9 +213,10 @@ pub enum ParameterLayout { impl<'a, 'b> ParameterList<'a, 'b> { pub fn with_layout( list: &'b AstNode<'a, FormalParameters<'a>>, + this: Option<&'b AstNode<'a, TSThisParameter<'a>>>, layout: ParameterLayout, ) -> Self { - Self { list, layout: Some(layout) } + Self { list, this, layout: Some(layout) } } } @@ -123,7 +244,7 @@ impl<'a> Format<'a> for ParameterList<'a, '_> { }; joiner .entries_with_trailing_separator( - FormalParametersIter::from(self.list), + FormalParametersIter::from(self), ",", trailing_separator, ) @@ -132,7 +253,7 @@ impl<'a> Format<'a> for ParameterList<'a, '_> { Some(ParameterLayout::Hug) => { let mut join = f.join_with(space()); join.entries_with_trailing_separator( - self.list.items().iter(), + FormalParametersIter::from(self), ",", TrailingSeparator::Omit, ); @@ -163,16 +284,35 @@ pub fn can_avoid_parentheses( pub fn should_hug_function_parameters<'a>( parameters: &AstNode<'a, FormalParameters<'a>>, + this_param: Option<&AstNode<'a, TSThisParameter<'a>>>, parentheses_not_needed: bool, f: &Formatter<'_, 'a>, ) -> bool { let list = ¶meters.items(); - if list.len() != 1 || parameters.rest.is_some() { + + if list.len() > 1 || parameters.rest.is_some() { return false; } + if let Some(this_param) = this_param { + // `(/* comment before */ this /* comment after */)` + // Checker whether there are comments around the only parameter. + + if f.comments().has_comment_in_range(parameters.span.start, this_param.span.start) + || f.comments().has_comment_in_range(this_param.span.end, parameters.span.end) + { + return false; + } + + return list.is_empty() + && this_param + .type_annotation + .as_ref() + .is_none_or(|ty| matches!(ty.type_annotation, TSType::TSTypeLiteral(_))); + } + // Safe because of the length check above - let only_parameter = list.first().unwrap(); + let Some(only_parameter) = list.first() else { return false }; if only_parameter.has_modifier() { return false; diff --git a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md index 8230839a7b936..1172d4ff9e6ea 100644 --- a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md @@ -1,4 +1,4 @@ -ts compatibility: 520/573 (90.75%) +ts compatibility: 521/573 (90.92%) # Failed @@ -33,7 +33,6 @@ ts compatibility: 520/573 (90.75%) | typescript/conditional-types/parentheses.ts | 💥✨ | 15.22% | | typescript/conformance/types/functions/functionOverloadErrorsSyntax.ts | 💥 | 0.00% | | typescript/custom/computedProperties/string.ts | 💥 | 73.33% | -| typescript/declare/object-type-in-declare-function.ts | 💥 | 56.25% | | typescript/decorators/comments.ts | 💥 | 60.00% | | typescript/decorators/decorators-comments.ts | 💥 | 65.71% | | typescript/decorators-ts/angular.ts | 💥 | 87.50% |