fix(format/html): fix case where comments cause invalid html (#10363)

Author: dyc3

.changeset/cyan-mangos-sit.md

@@ -0,0 +1,25 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed HTML formatting for a case where comments could cause the formatter to split up a closing tag, which would cause the resulting HTML to be syntactically invalid.
+
+Input:
+```html
+<span><!-- 1
+--><span>a</span><!-- 2
+--><span>b</span><!-- 3
+--></span>
+```
+
+Output:
+```diff
+  <span
+	  ><!-- 1
+- --> <span>a</span<!-- 2
+- --> ><span>b</span><!-- 3
++ --><span>a</span><!-- 2
++ --><span>b</span><!-- 3
+  --></span
+  >
+```

Cargo.lock

@@ -15,6 +15,7 @@ harness = false
 name    = "html_formatter"
 
 [dependencies]
+biome_console                = { workspace = true }
 biome_deserialize            = { workspace = true }
 biome_deserialize_macros     = { workspace = true }
 biome_diagnostics_categories = { workspace = true }

crates/biome_html_formatter/Cargo.toml

@@ -38,6 +38,8 @@ pub(crate) struct FormatHtmlElement {
     /// A `>` token borrowed from the previous sibling element's closing tag.
     /// This is printed at the start of this element's opening tag group.
     borrowed_sibling_r_angle: Option<HtmlSyntaxToken>,
+    /// Whether comments adjacent to this element are formatted by the containing child list.
+    comments_as_children: bool,
 }
 
 #[derive(Debug, Clone, Default)]
@@ -46,6 +48,8 @@ pub(crate) struct FormatHtmlElementOptions {
     pub closing_r_angle_borrowed: bool,
     /// A `>` token borrowed from the previous sibling element's closing tag.
     pub borrowed_sibling_r_angle: Option<HtmlSyntaxToken>,
+    /// Whether comments adjacent to this element are formatted by the containing child list.
+    pub comments_as_children: bool,
 }
 
 impl FormatRuleWithOptions<HtmlElement> for FormatHtmlElement {
@@ -54,6 +58,7 @@ impl FormatRuleWithOptions<HtmlElement> for FormatHtmlElement {
     fn with_options(mut self, options: Self::Options) -> Self {
         self.closing_r_angle_borrowed = options.closing_r_angle_borrowed;
         self.borrowed_sibling_r_angle = options.borrowed_sibling_r_angle;
+        self.comments_as_children = options.comments_as_children;
         self
     }
 }
@@ -72,6 +77,11 @@ impl FormatNodeRule<HtmlElement> for FormatHtmlElement {
     }
 
     fn fmt_leading_comments(&self, node: &HtmlElement, f: &mut HtmlFormatter) -> FormatResult<()> {
+        if self.comments_as_children {
+            // handled by element list formatter
+            return Ok(());
+        }
+
         // For block elements, use hard line break after leading comments even if
         // there's no newline in the source. This ensures proper formatting like:
         // <!-- comment -->
@@ -90,6 +100,11 @@ impl FormatNodeRule<HtmlElement> for FormatHtmlElement {
     }
 
     fn fmt_trailing_comments(&self, node: &HtmlElement, f: &mut HtmlFormatter) -> FormatResult<()> {
+        if self.comments_as_children {
+            // handled by element list formatter
+            return Ok(());
+        }
+
         // If there is leading whitespace before a leading comment, we need to preserve it because it's probably indentation.
         // See prettier test case: crates/biome_html_formatter/tests/specs/prettier/html/comments/hidden.html
         // The current implementation for `biome_formatter::FormatTrailingComments` actually has a ton of js specific behavior that we don't want in the html formatter.

crates/biome_html_formatter/src/html/auxiliary/element.rs

@@ -1,11 +1,32 @@
 use crate::{
-    html::lists::attribute_list::FormatHtmlAttributeListOptions, prelude::*,
+    html::lists::attribute_list::FormatHtmlAttributeListOptions,
+    prelude::*,
     utils::metadata::should_lowercase_html_tag,
+    verbatim::{format_html_leading_comments, format_html_trailing_comments},
 };
-use biome_formatter::write;
+use biome_formatter::{FormatRuleWithOptions, write};
 use biome_html_syntax::{HtmlSelfClosingElement, HtmlSelfClosingElementFields};
 #[derive(Debug, Clone, Default)]
-pub(crate) struct FormatHtmlSelfClosingElement;
+pub(crate) struct FormatHtmlSelfClosingElement {
+    /// Whether comments adjacent to this element are formatted by the containing child list.
+    comments_as_children: bool,
+}
+
+#[derive(Debug, Clone, Default)]
+pub(crate) struct FormatHtmlSelfClosingElementOptions {
+    /// Whether comments adjacent to this element are formatted by the containing child list.
+    pub comments_as_children: bool,
+}
+
+impl FormatRuleWithOptions<HtmlSelfClosingElement> for FormatHtmlSelfClosingElement {
+    type Options = FormatHtmlSelfClosingElementOptions;
+
+    fn with_options(mut self, options: Self::Options) -> Self {
+        self.comments_as_children = options.comments_as_children;
+        self
+    }
+}
+
 impl FormatNodeRule<HtmlSelfClosingElement> for FormatHtmlSelfClosingElement {
     fn fmt_fields(&self, node: &HtmlSelfClosingElement, f: &mut HtmlFormatter) -> FormatResult<()> {
         let HtmlSelfClosingElementFields {
@@ -93,4 +114,30 @@ impl FormatNodeRule<HtmlSelfClosingElement> for FormatHtmlSelfClosingElement {
 
         Ok(())
     }
+
+    fn fmt_leading_comments(
+        &self,
+        _node: &HtmlSelfClosingElement,
+        _f: &mut HtmlFormatter,
+    ) -> FormatResult<()> {
+        if self.comments_as_children {
+            // handled by element list formatter
+            return Ok(());
+        }
+
+        format_html_leading_comments(_node.syntax()).fmt(_f)
+    }
+
+    fn fmt_trailing_comments(
+        &self,
+        _node: &HtmlSelfClosingElement,
+        _f: &mut HtmlFormatter,
+    ) -> FormatResult<()> {
+        if self.comments_as_children {
+            // handled by element list formatter
+            return Ok(());
+        }
+
+        format_html_trailing_comments(_node.syntax()).fmt(_f)
+    }
 }

crates/biome_html_formatter/src/html/auxiliary/self_closing_element.rs

@@ -36,6 +36,24 @@ impl FormatNodeRule<HtmlSingleTextExpression> for FormatHtmlSingleTextExpression
             )
         }
     }
+
+    fn fmt_leading_comments(
+        &self,
+        _node: &HtmlSingleTextExpression,
+        _f: &mut HtmlFormatter,
+    ) -> FormatResult<()> {
+        // handled by element list formatter
+        Ok(())
+    }
+
+    fn fmt_trailing_comments(
+        &self,
+        _node: &HtmlSingleTextExpression,
+        _f: &mut HtmlFormatter,
+    ) -> FormatResult<()> {
+        // handled by element list formatter
+        Ok(())
+    }
 }
 
 impl FormatRuleWithOptions<HtmlSingleTextExpression> for FormatHtmlSingleTextExpression {

crates/biome_html_formatter/src/html/auxiliary/single_text_expression.rs

@@ -182,14 +182,20 @@
 //! ```
 
 use crate::{
-    html::auxiliary::element::{FormatHtmlElement, FormatHtmlElementOptions},
+    html::auxiliary::{
+        element::{FormatHtmlElement, FormatHtmlElementOptions},
+        self_closing_element::{FormatHtmlSelfClosingElement, FormatHtmlSelfClosingElementOptions},
+    },
     prelude::*,
     utils::{
-        children::{HtmlChild, HtmlChildrenIterator, html_split_children},
+        children::{
+            DisplayHtmlChildSequence, HtmlChild, HtmlChildrenIterator, html_split_children,
+        },
         css_display::CssDisplay,
         metadata::get_element_css_display,
     },
 };
+use biome_console::{ColorMode, ConsoleExt, EnvConsole, markup};
 use biome_formatter::{FormatRuleWithOptions, GroupId, prelude::*};
 use biome_formatter::{format_args, write};
 use biome_html_syntax::{
@@ -369,12 +375,23 @@ impl FormatHtmlElementList {
         // Split children into HtmlChild variants
         let children = html_split_children(
             list.iter(),
+            list.parent::<HtmlElement>()
+                .and_then(|p| p.opening_element().ok())
+                .and_then(|opening| opening.r_angle_token().ok())
+                .as_ref(),
             list.parent::<HtmlElement>()
                 .and_then(|p| p.closing_element().ok())
                 .as_ref(),
             f,
         )?;
 
+        #[cfg(debug_assertions)]
+        if std::env::var("DEBUG_HTML_FORMATTER_CHILDREN").is_ok() {
+            EnvConsole::new(ColorMode::Auto).error(markup! {
+                {DisplayHtmlChildSequence::new(&children)}
+            });
+        }
+
         let children_meta = self.children_meta(&children);
 
         // Trim trailing new lines from children
@@ -594,6 +611,15 @@ impl FormatHtmlElementList {
                     // HTML comment
                     HtmlChild::Comment(comment) => {
                         write!(f, [comment])?;
+
+                        let next = children_iter.peek();
+                        if let Some(HtmlChild::NonText(next_non_text)) = next {
+                            // when a comment is followed by a block element, we need to add a line break.
+                            let next_css_display = get_element_css_display(next_non_text);
+                            if !next_css_display.is_externally_whitespace_sensitive(f) {
+                                write!(f, [hard_line_break()])?;
+                            }
+                        }
                     }
 
                     // Whitespace between children
@@ -650,6 +676,8 @@ impl FormatHtmlElementList {
                             && !self.is_container_whitespace_sensitive
                         {
                             // If the container is not whitespace sensitive, we trim trailing whitespace
+                        } else if is_first_child && !self.is_container_whitespace_sensitive {
+                            // if the container is not whitespace sensitive, we trim leading whitespace
                         } else {
                             write!(f, [space()])?;
                         }
@@ -1026,12 +1054,23 @@ fn format_element_with_borrowing(
                 &FormatHtmlElement::default().with_options(FormatHtmlElementOptions {
                     closing_r_angle_borrowed,
                     borrowed_sibling_r_angle: borrowed_sibling_r_angle.clone(),
+                    comments_as_children: true,
                 }),
                 html_element,
                 f,
             )
+        } else if let AnyHtmlElement::HtmlSelfClosingElement(self_closing_element) = element {
+            FormatNodeRule::fmt(
+                &FormatHtmlSelfClosingElement::default().with_options(
+                    FormatHtmlSelfClosingElementOptions {
+                        comments_as_children: true,
+                    },
+                ),
+                self_closing_element,
+                f,
+            )
         } else {
-            // For other element types (self-closing, etc.), use default formatting
+            // For other element types, use default formatting.
             write!(f, [element.format()])
         }
     })

crates/biome_html_formatter/src/html/lists/element_list.rs

@@ -20,4 +20,13 @@ impl FormatNodeRule<SvelteAwaitBlock> for FormatSvelteAwaitBlock {
             ]
         )
     }
+
+    fn fmt_leading_comments(
+        &self,
+        _node: &SvelteAwaitBlock,
+        _f: &mut HtmlFormatter,
+    ) -> FormatResult<()> {
+        // handled by element list formatter
+        Ok(())
+    }
 }

crates/biome_html_formatter/src/svelte/auxiliary/await_block.rs

@@ -25,4 +25,13 @@ impl FormatNodeRule<SvelteEachBlock> for FormatSvelteEachBlock {
 
         write!(f, [closing_block.format()])
     }
+
+    fn fmt_leading_comments(
+        &self,
+        _node: &SvelteEachBlock,
+        _f: &mut HtmlFormatter,
+    ) -> FormatResult<()> {
+        // handled by element list formatter
+        Ok(())
+    }
 }

crates/biome_html_formatter/src/svelte/auxiliary/each_block.rs

@@ -23,4 +23,22 @@ impl FormatNodeRule<SvelteHtmlBlock> for FormatSvelteHtmlBlock {
             ]
         )
     }
+
+    fn fmt_leading_comments(
+        &self,
+        _node: &SvelteHtmlBlock,
+        _f: &mut HtmlFormatter,
+    ) -> FormatResult<()> {
+        // handled by element list formatter
+        Ok(())
+    }
+
+    fn fmt_trailing_comments(
+        &self,
+        _node: &SvelteHtmlBlock,
+        _f: &mut HtmlFormatter,
+    ) -> FormatResult<()> {
+        // handled by element list formatter
+        Ok(())
+    }
 }