fix(formatter): correct printing comments within the type annotation of ArrayPattern and ObjectPattern

Author: Dunqing

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) = &param.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)
+        }
+    }
+}

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()])

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) = &param.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(),

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% |