fix(formatter): keep comment position on holey array (#27730)

Fixes #22799, closes #26376

Fundamentally, the pattern AST side needs to be fixed (https://github.com/oxc-project/backlog/issues/244), but I'll do it while working on another task.

crates/oxc_formatter/AGENTS.md

@@ -81,7 +81,7 @@ js-in-xxx works with `prettier-plugin-oxfmt` which uses `format_fragment`. See `
 
 Comments (and parentheses) are where a local fix has side effects; keep the rules below in view.
 The invariants and the invariants-vs-compat-tables split live in FORMATTER_POLICY.md "Comment placement invariants"; this section is their JS/TSX translation.
-Prettier's attachment artifacts that relocate a comment across tokens are divergences to pin, never rules to emulate (DIVERGENCES.md#array-hole-trailing-comment, DIVERGENCES.md#type-alias-trailing-comment-move).
+Prettier's attachment artifacts that relocate a comment across tokens are divergences to pin, never rules to emulate (DIVERGENCES.md#array-hole-comments, DIVERGENCES.md#type-alias-trailing-comment-move).
 
 ### Vocabulary
 

crates/oxc_formatter/DIVERGENCES.md

@@ -2,23 +2,48 @@
 
 Admission reasons and rules: see `crates/oxc_formatter_core/FORMATTER_POLICY.md` "Known divergences".
 
-## array-hole-trailing-comment
+## array-hole-comments
 
 - Why: invariant
 - Pin: `tests/fixtures/js/comments/array-holes.js`
+- Conformance: `js/arrays/numbers-with-holes.js`
 
 ```js
 // input
 const a = [1, , /* c */];
+[parser, /* c */, optional] = x;
+const b = [
+  1,
+  // c
+  ,
+  2,
+];
 
 // ours
-const a = [1, ,/* c */];
+const a = [1, , /* c */];
+[parser, /* c */, optional] = x;
+const b = [
+  1,
+  // c
+  ,
+  2,
+];
 
 // prettier
 const a = [1 /* c */, ,];
+[parser /* c */, , optional] = x;
+const b = [
+  1,
+  ,
+  // c
+  2,
+];
 ```
 
-Prettier's attachment relocates the comment backward across the commas (user content) to the last real element; we keep it in place.
+A hole has no node, so Prettier's attachment relocates a comment next to it across the commas (user content, they keep the hole count) to a real element;
+we keep every comment on its source side of each hole's `,`, in array expressions and array patterns alike.
+Spacing follows the `,` like any separator: a comment before it is glued to it (`/* c */,`, as in `a /* c */,`),
+a comment after it is spaced (`, /* c */`, as in `a, /* c */ b`), also after the last hole where Prettier glues it (`, ,/* c */`).
 
 ## type-alias-trailing-comment-move
 

crates/oxc_formatter/src/ast_nodes/iterator.rs

@@ -180,7 +180,8 @@ impl_ast_node_vec!(Directive<'a>, has_following_span_in_the_last_item);
 impl_ast_node_vec!(FormalParameter<'a>, has_following_span_in_the_last_item);
 impl_ast_node_vec!(BindingProperty<'a>, has_following_span_in_the_last_item);
 impl_ast_node_vec!(AssignmentTargetProperty<'a>, has_following_span_in_the_last_item);
-// A `None` element is an elision hole with no span of its own
+// In these two, a `None` element is an elision hole with no span of its own;
+// `write_array_node` bounds the trailing comments of the element before it at the hole's `,` found in the source
 impl_ast_node_vec!(
     Option<BindingPattern<'a>>,
     has_following_span_in_the_last_item,

crates/oxc_formatter/src/print/array_element_list.rs

@@ -6,13 +6,8 @@ use oxc_span::GetSpan;
 use crate::{
     FormatTrailingCommas,
     ast_nodes::AstNode,
-    formatter::{
-        JsFormatContext, JsFormatter,
-        prelude::*,
-        separated::FormatSeparatedIter,
-        trivia::{DanglingIndentMode, FormatDanglingComments},
-    },
-    utils::array::write_array_node,
+    formatter::{JsFormatContext, JsFormatter, prelude::*, separated::FormatSeparatedIter},
+    utils::array::{write_array_node, write_unclaimed_comments},
     write,
 };
 
@@ -32,12 +27,12 @@ impl<'a, 'b> ArrayElementList<'a, 'b> {
 
 impl<'a> Format<'a, JsFormatContext<'a>> for ArrayElementList<'a, '_> {
     fn fmt(&self, f: &mut JsFormatter<'_, 'a>) {
-        let layout =
-            if can_concisely_print_array_list(self.elements.parent().span(), self.elements, f) {
-                ArrayLayout::Fill
-            } else {
-                ArrayLayout::OnePerLine
-            };
+        let array_span = self.elements.parent().span();
+        let layout = if can_concisely_print_array_list(array_span, self.elements, f) {
+            ArrayLayout::Fill
+        } else {
+            ArrayLayout::OnePerLine
+        };
 
         match layout {
             ArrayLayout::Fill => {
@@ -68,30 +63,15 @@ impl<'a> Format<'a, JsFormatContext<'a>> for ArrayElementList<'a, '_> {
                 }
 
                 filler.finish();
+                write_unclaimed_comments(array_span.end - 1, f);
             }
             ArrayLayout::OnePerLine => write_array_node(
-                self.elements.len(),
+                array_span,
+                None,
                 self.elements.iter().map(|e| if e.is_elision() { None } else { Some(e) }),
                 f,
             ),
         }
-
-        // Comments that no element consumed as a trailing comment
-        // (e.g. after a trailing hole: `[,, /* comment */]`)
-        // would otherwise escape the brackets; print them right before the `]` like Prettier
-        let dangling_comments =
-            f.context().comments().comments_before(self.elements.parent().span().end);
-        write!(
-            f,
-            FormatDanglingComments::Comments {
-                comments: dangling_comments,
-                indent: DanglingIndentMode::None
-            }
-        );
-        // A line comment would swallow the `]` (`[, ,// c]`), so break before it
-        if dangling_comments.last().is_some_and(Comment::is_line) {
-            write!(f, hard_line_break());
-        }
     }
 }
 

crates/oxc_formatter/src/print/array_pattern.rs

@@ -36,7 +36,8 @@ impl<'a> Format<'a, JsFormatContext<'a>> for FormatArrayPattern<'a, '_> {
                     let has_element = !self.elements.is_empty();
                     if has_element {
                         write_array_node(
-                            self.elements.len() + usize::from(self.rest.is_some()),
+                            self.span(),
+                            self.rest.as_ref().map(|rest| rest.span.start),
                             self.elements().iter().map(AstNode::as_ref),
                             f,
                         );

crates/oxc_formatter/src/print/mod.rs

@@ -394,7 +394,8 @@ impl<'a> FormatWrite<'a> for AstNode<'a, ArrayAssignmentTarget<'a>> {
                     let has_element = !self.elements.is_empty();
                     if has_element {
                         write_array_node(
-                            self.elements.len() + usize::from(self.rest.is_some()),
+                            self.span(),
+                            self.rest.as_ref().map(|rest| rest.span.start),
                             self.elements().iter().map(AstNode::as_ref),
                             f,
                         );

crates/oxc_formatter/src/utils/array.rs

@@ -1,90 +1,159 @@
 use oxc_span::{GetSpan, SPAN, Span};
 
-use crate::{formatter::prelude::*, options::FormatTrailingCommas, write};
+use crate::{
+    ast_nodes::AstNode,
+    format_args,
+    formatter::{
+        prelude::*,
+        trivia::{FormatCommentBeforeContent, format_trailing_comments},
+    },
+    options::FormatTrailingCommas,
+    utils::{
+        format_node_without_trailing_comments::FormatNodeWithoutTrailingComments,
+        statement_body::write_trailing_comments_before,
+    },
+    write,
+};
 
 /// Utility function to print array-like nodes (array expressions, array bindings and assignment patterns)
-pub fn write_array_node<'a, 'b, N>(
-    len: usize,
-    array: impl IntoIterator<Item = Option<&'a N>> + 'b,
+///
+/// `rest_start` is the start of the rest element (`[a, ...rest]`) printed by the caller after this.
+///
+/// A hole's `,` is content (it keeps the hole count), so comments around it stay on their source side:
+/// the hole prints its leading comments, its `,`, then its same-line trailing comments.
+/// Comments after the last entry print before the `]` ([`write_unclaimed_comments`]), or lead the rest element.
+pub fn write_array_node<'a, T>(
+    array_span: Span,
+    rest_start: Option<u32>,
+    array: impl IntoIterator<Item = Option<&'a AstNode<'a, T>>>,
     f: &mut JsFormatter<'_, 'a>,
 ) where
-    N: Format<'a, JsFormatContext<'a>> + GetSpan + std::fmt::Debug + 'a,
+    T: 'a,
+    AstNode<'a, T>: Format<'a, JsFormatContext<'a>> + GetSpan,
 {
-    // Specifically do not use format_separated as arrays need separators
-    // inserted after holes regardless of the formatting since this makes a
-    // semantic difference
+    // The elements: from the `[` up to the `]` (exclusive) or the rest element
+    let range = Span::new(array_span.start, rest_start.unwrap_or(array_span.end - 1));
+    let has_rest = rest_start.is_some();
+
+    // Specifically do not use `format_separated` as arrays need separators inserted after holes
+    // regardless of the formatting since this makes a semantic difference.
 
-    let last_index = len - 1;
-    let source_text = f.context().source_text();
     let mut join = f.join_nodes_with_soft_line();
     let mut has_seen_elision = false;
+    // Holes come as `None` without a span (patterns have no `Elision`),
+    // so each hole's `,` is found in the source, once, by the entry before it (or at the `[` for a leading hole).
+    //
+    // TODO: Once `ArrayPattern` / `ArrayAssignmentTarget` hold an `Elision` (a span-only hole) like `ArrayExpression`,
+    // pass the holes' spans here instead of this scan, and format the element before a hole as is
+    // (the iterator would see the hole as the following sibling).
+    let comma_after =
+        |f: &JsFormatter<'_, 'a>, pos: u32| f.comments().position_after_character(pos, b',') - 1;
+    let mut hole_comma = None;
 
-    let mut array_iter = array.into_iter().enumerate().peekable();
+    let mut array_iter = array.into_iter().peekable();
 
-    while let Some((index, element)) = array_iter.next() {
-        let separator_mode = if element.is_none() {
-            TrailingSeparatorMode::Force
+    while let Some(element) = array_iter.next() {
+        let next = array_iter.peek().copied();
+
+        let span = if let Some(element) = element {
+            element.span()
         } else {
-            TrailingSeparatorMode::Auto
+            Span::sized(
+                hole_comma.take().unwrap_or_else(|| comma_after(join.fmt(), range.start)),
+                1,
+            )
         };
 
-        let is_disallow = matches!(separator_mode, TrailingSeparatorMode::Disallow);
-        let is_force = matches!(separator_mode, TrailingSeparatorMode::Force);
+        // Use fake span to avoid add any empty line between elision and expression element
+        let entry_span = if has_seen_elision { SPAN } else { span };
 
-        join.entry(
-            // Note(different-with-Biome): this implementation isn't the same as Biome, because its output doesn't exactly match Prettier.
-            if has_seen_elision {
-                // Use fake span to avoid add any empty line between elision and expression element.
-                SPAN
-            } else {
-                has_seen_elision = false;
-                element.map_or_else(
-                    || {
-                        // TODO: improve the `ArrayPattern` AST to simplify this logic.
-                        // Since `ArrayPattern` doesn't have a elision node, so that we have to find the comma position
-                        // by looking through the source text.
-                        let next_span =
-                            array_iter.peek().map_or(SPAN, |e| e.1.map_or(SPAN, GetSpan::span));
+        // The `,` of the next hole, past this entry's own `,`
+        if matches!(next, Some(None)) {
+            let own_comma =
+                if element.is_some() { comma_after(join.fmt(), span.end) } else { span.start };
+            hole_comma = Some(comma_after(join.fmt(), own_comma + 1));
+        }
 
-                        let comma_position =
-                            source_text.bytes_to(next_span.start).position(|c| c == b',');
-
-                        // comma span
-                        #[expect(clippy::cast_possible_truncation)]
-                        comma_position
-                            .map_or(SPAN, |pos| Span::sized(next_span.start - pos as u32, 1))
-                    },
-                    GetSpan::span,
-                )
-            },
-            &format_once(|f| {
-                if let Some(element) = element {
-                    write!(f, group(&element));
-
-                    if is_disallow {
-                    } else if is_force || index != last_index {
-                        ",".fmt(f);
+        if let Some(element) = element {
+            let is_last = next.is_none() && !has_rest;
+            join.entry(
+                entry_span,
+                &format_once(|f| {
+                    if let Some(comma) = hole_comma {
+                        // The iterator skips holes, so bound the trailing pass at the next hole's `,`
+                        let trailing =
+                            format_trailing_comments(element.parent().span(), span, comma);
+                        write!(
+                            f,
+                            group(&format_args!(
+                                FormatNodeWithoutTrailingComments(element),
+                                trailing
+                            ))
+                        );
                     } else {
-                        write!(f, FormatTrailingCommas::ES5);
+                        write!(f, group(&element));
                     }
-                } else {
-                    has_seen_elision = true;
+                    if is_last {
+                        write!(f, FormatTrailingCommas::ES5);
+                    } else {
+                        ",".fmt(f);
+                    }
+                }),
+            );
+        } else {
+            // `None` after the last hole: see the unclaimed comments below
+            let following = match next {
+                Some(Some(next)) => Some(next.span().start),
+                _ => hole_comma,
+            };
+            join.entry(
+                entry_span,
+                &format_once(|f| {
+                    write_hole_slot_comments(span.start, f);
                     write!(f, ",");
-                }
-            }),
-        );
+                    if let Some(following) = following {
+                        write!(f, format_trailing_comments(range, span, following));
+                    }
+                }),
+            );
+            has_seen_elision = true;
+        }
+    }
+
+    if !has_rest {
+        write_unclaimed_comments(range.end, join.fmt_mut());
     }
 }
 
-/// Determines if a trailing separator should be inserted after an array element
-#[derive(Copy, Clone, Eq, PartialEq, Debug)]
-pub enum TrailingSeparatorMode {
-    /// Trailing separators are not allowed after this element (eg. rest elements)
-    #[expect(unused)]
-    Disallow,
-    /// Trailing separators are inserted after this element except if its the
-    /// last element and the group is not breaking
-    Auto,
-    /// Trailing separators will always be inserted after this element (eg. hole elements)
-    Force,
+/// Comments in a hole's slot, before its `,` at `comma`.
+/// A hole has no content, so the `,` follows a block comment directly (`[a, /* c */, b]`),
+/// as a separator follows any content (`[a /* c */, b]`); an own-line comment keeps its line.
+///
+/// Not [`write_trailing_comments_before`] like an empty for-head slot:
+/// it would space a comment away from the `[` (`[ /* c */, b]`),
+/// and let a line comment right after the `[` ride past the hole's `,` (`[// c` + break + `, b]` -> `, // c`).
+fn write_hole_slot_comments(comma: u32, f: &mut JsFormatter<'_, '_>) {
+    let comments = f.context().comments().comments_before(comma);
+    for (index, comment) in comments.iter().enumerate() {
+        // A line comment ends with a hard break
+        write!(f, FormatCommentBeforeContent::new(comment));
+        if comment.is_block() {
+            if comment.followed_by_newline() {
+                write!(f, hard_line_break());
+            } else if index + 1 < comments.len() {
+                write!(f, space());
+            }
+        }
+    }
+}
+
+/// Comments that no element consumed (e.g. after a trailing hole: `[,, /* comment */]`)
+/// would otherwise escape the brackets; print them before the `]` at `end`,
+/// trailing the last `,` like after any separator (`[, , /* comment */]`).
+pub fn write_unclaimed_comments(end: u32, f: &mut JsFormatter<'_, '_>) {
+    let Some(first) = f.context().comments().comments_before(end).first() else { return };
+    // Discarded flush obligation:
+    // a riding line comment's `expand_parent` breaks the array group,
+    // so a real break already follows before the `]`
+    let _line_comment_riding = write_trailing_comments_before(first.span.start, b']', f);
 }

crates/oxc_formatter/tests/fixtures/js/comments/array-holes.js

@@ -1,5 +1,5 @@
-// DIVERGES: a comment after a trailing hole stays after the hole (Prettier moves `f`'s back to the last real element);
-// see DIVERGENCES.md#array-hole-trailing-comment
+// DIVERGES: comments around a hole stay on their side of its `,`, spaced after it (all but `e` and `m` differ from Prettier);
+// see DIVERGENCES.md#array-hole-comments
 const a = [,, /* comment */];
 const b = [
   , /* comment */
@@ -14,7 +14,32 @@ const d = [,, // line
 const e = [1, 2 /* t */];
 // A real element before the hole (the entry's shape)
 const f = [1, , /* comment */];
-// An own-line line comment before the last hole must not swallow the `]`
+// An own-line line comment before the last hole stays before its `,`
 const g = [,
   // line
 ,];
+// A comment before a hole stays before its `,`, in every array-like node
+const h = [a, /* comment */, b];
+[a, /* comment */, b] = x;
+const [i, /* comment */, j] = x;
+// Own-line comments keep their side of the hole's `,`
+const k = [
+  a,
+  // before
+  ,
+  // after
+  b,
+];
+// A same-line line comment after a hole keeps its line
+const l = [, // line
+  ,];
+// Comments in a pattern of holes stay inside the brackets
+const [, /* comment */ ,] = x;
+[, // line
+] = x;
+// A trailing comment past a JSDoc cast's parens stays outside them
+const m = [/** @type {X} */ (a) /* comment */, , b];
+// A leading hole, consecutive holes, and comments on both sides of a separator
+const n = [/* lead */, 2];
+const o = [1, /* c1 */, /* c2 */, 4];
+const p = [1 /* t */, /* slot */, 3];

crates/oxc_formatter/tests/fixtures/js/comments/array-holes.js.snap

@@ -2,8 +2,8 @@
 source: crates/oxc_formatter/tests/fixtures/mod.rs
 ---
 ==================== Input ====================
-// DIVERGES: a comment after a trailing hole stays after the hole (Prettier moves `f`'s back to the last real element);
-// see DIVERGENCES.md#array-hole-trailing-comment
+// DIVERGES: comments around a hole stay on their side of its `,`, spaced after it (all but `e` and `m` differ from Prettier);
+// see DIVERGENCES.md#array-hole-comments
 const a = [,, /* comment */];
 const b = [
   , /* comment */
@@ -18,56 +18,139 @@ const d = [,, // line
 const e = [1, 2 /* t */];
 // A real element before the hole (the entry's shape)
 const f = [1, , /* comment */];
-// An own-line line comment before the last hole must not swallow the `]`
+// An own-line line comment before the last hole stays before its `,`
 const g = [,
   // line
 ,];
+// A comment before a hole stays before its `,`, in every array-like node
+const h = [a, /* comment */, b];
+[a, /* comment */, b] = x;
+const [i, /* comment */, j] = x;
+// Own-line comments keep their side of the hole's `,`
+const k = [
+  a,
+  // before
+  ,
+  // after
+  b,
+];
+// A same-line line comment after a hole keeps its line
+const l = [, // line
+  ,];
+// Comments in a pattern of holes stay inside the brackets
+const [, /* comment */ ,] = x;
+[, // line
+] = x;
+// A trailing comment past a JSDoc cast's parens stays outside them
+const m = [/** @type {X} */ (a) /* comment */, , b];
+// A leading hole, consecutive holes, and comments on both sides of a separator
+const n = [/* lead */, 2];
+const o = [1, /* c1 */, /* c2 */, 4];
+const p = [1 /* t */, /* slot */, 3];
 
 ==================== Output ====================
 ------------------
 { printWidth: 80 }
 ------------------
-// DIVERGES: a comment after a trailing hole stays after the hole (Prettier moves `f`'s back to the last real element);
-// see DIVERGENCES.md#array-hole-trailing-comment
-const a = [, ,/* comment */];
-const b = [,/* comment */];
-const c = [, ,/* comment */];
+// DIVERGES: comments around a hole stay on their side of its `,`, spaced after it (all but `e` and `m` differ from Prettier);
+// see DIVERGENCES.md#array-hole-comments
+const a = [, , /* comment */];
+const b = [, /* comment */];
+const c = [, , /* comment */];
 // A trailing line comment forces the array to break
 const d = [
   ,
-  ,// line
+  , // line
 ];
 // Comments trailing a real element are unaffected
 const e = [1, 2 /* t */];
 // A real element before the hole (the entry's shape)
-const f = [1, ,/* comment */];
-// An own-line line comment before the last hole must not swallow the `]`
+const f = [1, , /* comment */];
+// An own-line line comment before the last hole stays before its `,`
 const g = [
   ,
-  ,// line
+  // line
+  ,
 ];
+// A comment before a hole stays before its `,`, in every array-like node
+const h = [a, /* comment */, b];
+[a, /* comment */, b] = x;
+const [i, /* comment */, j] = x;
+// Own-line comments keep their side of the hole's `,`
+const k = [
+  a,
+  // before
+  ,
+  // after
+  b,
+];
+// A same-line line comment after a hole keeps its line
+const l = [
+  , // line
+  ,
+];
+// Comments in a pattern of holes stay inside the brackets
+const [, /* comment */,] = x;
+[
+  , // line
+] = x;
+// A trailing comment past a JSDoc cast's parens stays outside them
+const m = [/** @type {X} */ (a) /* comment */, , b];
+// A leading hole, consecutive holes, and comments on both sides of a separator
+const n = [/* lead */, 2];
+const o = [1, /* c1 */, /* c2 */, 4];
+const p = [1 /* t */, /* slot */, 3];
 
 -------------------
 { printWidth: 100 }
 -------------------
-// DIVERGES: a comment after a trailing hole stays after the hole (Prettier moves `f`'s back to the last real element);
-// see DIVERGENCES.md#array-hole-trailing-comment
-const a = [, ,/* comment */];
-const b = [,/* comment */];
-const c = [, ,/* comment */];
+// DIVERGES: comments around a hole stay on their side of its `,`, spaced after it (all but `e` and `m` differ from Prettier);
+// see DIVERGENCES.md#array-hole-comments
+const a = [, , /* comment */];
+const b = [, /* comment */];
+const c = [, , /* comment */];
 // A trailing line comment forces the array to break
 const d = [
   ,
-  ,// line
+  , // line
 ];
 // Comments trailing a real element are unaffected
 const e = [1, 2 /* t */];
 // A real element before the hole (the entry's shape)
-const f = [1, ,/* comment */];
-// An own-line line comment before the last hole must not swallow the `]`
+const f = [1, , /* comment */];
+// An own-line line comment before the last hole stays before its `,`
 const g = [
   ,
-  ,// line
+  // line
+  ,
 ];
+// A comment before a hole stays before its `,`, in every array-like node
+const h = [a, /* comment */, b];
+[a, /* comment */, b] = x;
+const [i, /* comment */, j] = x;
+// Own-line comments keep their side of the hole's `,`
+const k = [
+  a,
+  // before
+  ,
+  // after
+  b,
+];
+// A same-line line comment after a hole keeps its line
+const l = [
+  , // line
+  ,
+];
+// Comments in a pattern of holes stay inside the brackets
+const [, /* comment */,] = x;
+[
+  , // line
+] = x;
+// A trailing comment past a JSDoc cast's parens stays outside them
+const m = [/** @type {X} */ (a) /* comment */, , b];
+// A leading hole, consecutive holes, and comments on both sides of a separator
+const n = [/* lead */, 2];
+const o = [1, /* c1 */, /* c2 */, 4];
+const p = [1 /* t */, /* slot */, 3];
 
 ===================== End =====================

crates/oxc_formatter/tests/snapshots/conformance__prettier-js.snap

@@ -2,12 +2,13 @@
 source: crates/oxc_formatter/tests/conformance.rs
 expression: report
 ---
-js compatibility: 762/811 (93.96%), 23 files skipped
+js compatibility: 761/811 (93.83%), 23 files skipped
 
 # Failed
 
 | Spec path | Failed or Passed | Match ratio |
 | :-------- | :--------------: | :---------: |
+| js/arrays/numbers-with-holes.js | 💥 | 92.86% |
 | js/arrows/arrow-chain-with-trailing-comments.js | 💥💥 | 93.75% |
 | js/arrows/issue-17421.js | 💥💥 | 88.70% |
 | js/assignment-comments/string.js | 💥 | 96.59% |
@@ -87,7 +88,6 @@ js compatibility: 762/811 (93.96%), 23 files skipped
 # Not idempotent
 
 - js/arrays/numbers-negative-comment-after-minus.js
-- js/arrays/numbers-with-holes.js
 - js/comments/return-statement.js
 - js/comments/first-argument/first-argument.js
 - js/for/continue-and-break-comment-without-blocks.js