From 1b0519c755cb4cfad158a37be742ba6730858c5b Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Mon, 29 Sep 2025 09:43:27 +0000 Subject: [PATCH] fix(formatter): correct printing comments within the type annotation of ArrayPattern and `ObjectPattern` (#14077) --- .../oxc_formatter/src/write/array_pattern.rs | 82 +++++++++++++++++++ crates/oxc_formatter/src/write/mod.rs | 37 ++------- .../src/write/object_pattern_like.rs | 22 ++++- .../snapshots/prettier.ts.snap.md | 3 +- 4 files changed, 109 insertions(+), 35 deletions(-) create mode 100644 crates/oxc_formatter/src/write/array_pattern.rs diff --git a/crates/oxc_formatter/src/write/array_pattern.rs b/crates/oxc_formatter/src/write/array_pattern.rs new file mode 100644 index 0000000000000..5f680e883aadd --- /dev/null +++ b/crates/oxc_formatter/src/write/array_pattern.rs @@ -0,0 +1,82 @@ +use std::ops::Deref; + +use oxc_ast::ast::ArrayPattern; +use oxc_span::{GetSpan, Span}; + +use crate::{ + Format, FormatResult, + formatter::{Formatter, prelude::*, trivia::format_dangling_comments}, + generated::ast_nodes::{AstNode, AstNodes}, + utils::format_node_without_trailing_comments::FormatNodeWithoutTrailingComments, + write, + write::utils::array::write_array_node, +}; + +use super::FormatWrite; + +struct FormatArrayPattern<'a, 'b>(&'b AstNode<'a, ArrayPattern<'a>>); + +impl<'a> Deref for FormatArrayPattern<'a, '_> { + type Target = AstNode<'a, ArrayPattern<'a>>; + + fn deref(&self) -> &Self::Target { + self.0 + } +} + +impl GetSpan for FormatArrayPattern<'_, '_> { + fn span(&self) -> Span { + // `[a, b]: [a, b]` + // ^^^^^^^^^^^^^^ ArrayPattern's span covers the type annotation if exists, + // ^^^^^^ but we want the span to cover only the pattern itself, otherwise, + // the comments of type annotation will be treated as dangling comments + // of ArrayPattern. + if let AstNodes::FormalParameter(param) = self.parent + && let Some(ty) = ¶m.pattern.type_annotation + { + Span::new(self.span.start, ty.span.start) + } else { + self.span + } + } +} + +impl<'a> Format<'a> for FormatArrayPattern<'a, '_> { + fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { + write!(f, "[")?; + + if self.elements.is_empty() && self.rest.is_none() { + write!(f, [format_dangling_comments(self.span()).with_block_indent()])?; + } else { + write!( + f, + group(&soft_block_indent(&format_once(|f| { + if !self.elements.is_empty() { + write_array_node( + self.elements.len() + usize::from(self.rest.is_some()), + self.elements().iter().map(AstNode::as_ref), + f, + )?; + } + if let Some(rest) = self.rest() { + write!(f, [soft_line_break_or_space(), rest]); + } + Ok(()) + }))) + )?; + } + + write!(f, "]") + } +} + +impl<'a> FormatWrite<'a> for AstNode<'a, ArrayPattern<'a>> { + fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { + if matches!(self.parent, AstNodes::FormalParameter(param) if param.pattern.type_annotation.is_some()) + { + FormatNodeWithoutTrailingComments(&FormatArrayPattern(self)).fmt(f) + } else { + FormatArrayPattern(self).fmt(f) + } + } +} diff --git a/crates/oxc_formatter/src/write/mod.rs b/crates/oxc_formatter/src/write/mod.rs index 5cbbb75f6cd46..544d030ae2907 100644 --- a/crates/oxc_formatter/src/write/mod.rs +++ b/crates/oxc_formatter/src/write/mod.rs @@ -1,5 +1,6 @@ mod array_element_list; mod array_expression; +mod array_pattern; mod arrow_function_expression; mod as_or_satisfies_expression; mod assignment_pattern_property_list; @@ -871,7 +872,12 @@ impl<'a> FormatWrite<'a> for AstNode<'a, AssignmentPattern<'a>> { impl<'a> FormatWrite<'a> for AstNode<'a, ObjectPattern<'a>> { fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - ObjectPatternLike::ObjectPattern(self).fmt(f) + if matches!(self.parent, AstNodes::FormalParameter(param) if param.pattern.type_annotation.is_some()) + { + FormatNodeWithoutTrailingComments(&ObjectPatternLike::ObjectPattern(self)).fmt(f) + } else { + ObjectPatternLike::ObjectPattern(self).fmt(f) + } } } @@ -906,35 +912,6 @@ impl<'a> FormatWrite<'a> for AstNode<'a, BindingProperty<'a>> { } } -impl<'a> FormatWrite<'a> for AstNode<'a, ArrayPattern<'a>> { - fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - write!(f, "[")?; - - if self.elements.is_empty() && self.rest.is_none() { - write!(f, [format_dangling_comments(self.span()).with_block_indent()])?; - } else { - write!( - f, - group(&soft_block_indent(&format_once(|f| { - if !self.elements.is_empty() { - write_array_node( - self.elements.len() + usize::from(self.rest.is_some()), - self.elements().iter().map(AstNode::as_ref), - f, - )?; - } - if let Some(rest) = self.rest() { - write!(f, [soft_line_break_or_space(), rest]); - } - Ok(()) - }))) - )?; - } - - write!(f, "]") - } -} - impl<'a> FormatWrite<'a> for AstNode<'a, BindingRestElement<'a>> { fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { write!(f, ["...", self.argument()]) diff --git a/crates/oxc_formatter/src/write/object_pattern_like.rs b/crates/oxc_formatter/src/write/object_pattern_like.rs index 58590344f535b..6391c43252eb8 100644 --- a/crates/oxc_formatter/src/write/object_pattern_like.rs +++ b/crates/oxc_formatter/src/write/object_pattern_like.rs @@ -1,4 +1,5 @@ use oxc_ast::ast::*; +use oxc_span::GetSpan; use crate::{ formatter::{ @@ -21,14 +22,29 @@ pub enum ObjectPatternLike<'a, 'b> { ObjectAssignmentTarget(&'b AstNode<'a, ObjectAssignmentTarget<'a>>), } -impl<'a> ObjectPatternLike<'a, '_> { +impl GetSpan for ObjectPatternLike<'_, '_> { fn span(&self) -> Span { match self { - Self::ObjectPattern(o) => o.span, - Self::ObjectAssignmentTarget(o) => o.span, + Self::ObjectPattern(node) => { + // `{a, b}: {a: number, b: string}` + // ^^^^^^^^^^^^^^ ObjectPattern's span covers the type annotation if exists, + // ^^^^^^ but we want the span to cover only the pattern itself, otherwise, + // the comments of type annotation will be treated as dangling comments + // of ObjectPattern. + if let AstNodes::FormalParameter(param) = node.parent + && let Some(ty) = ¶m.pattern.type_annotation + { + Span::new(node.span.start, ty.span.start) + } else { + node.span + } + } + Self::ObjectAssignmentTarget(node) => node.span, } } +} +impl<'a> ObjectPatternLike<'a, '_> { fn is_empty(&self) -> bool { match self { Self::ObjectPattern(o) => o.is_empty(), diff --git a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md index bd547345c30d1..14612de2c106c 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: 527/573 (91.97%) +ts compatibility: 528/573 (92.15%) # Failed @@ -19,7 +19,6 @@ ts compatibility: 527/573 (91.97%) | typescript/chain-expression/test.ts | 💥 | 0.00% | | typescript/class/empty-method-body.ts | 💥 | 80.00% | | typescript/class/quoted-property.ts | 💥 | 66.67% | -| typescript/comments/location.ts | 💥 | 95.00% | | typescript/comments/method_types.ts | 💥 | 84.62% | | typescript/comments/type-parameters.ts | 💥 | 65.52% | | typescript/conditional-types/comments.ts | 💥✨ | 31.51% |