Formatter: Fix infinite loop when rendering multiple groups of adjacent inline elements (#1008)

Fixes: #978

---------

Co-authored-by: Marco Roth <marco.roth@intergga.ch>

Author: ydah

javascript/packages/formatter/src/format-helpers.ts

@@ -398,19 +398,24 @@ export function isContentPreserving(element: HTMLElementNode | HTMLOpenTagNode |
 }
 
 /**
- * Count consecutive inline elements/ERB at the start of children (with no whitespace between)
+ * Count consecutive inline elements/ERB with no whitespace between them.
+ * Starts from startIndex and skips indices in processedIndices.
  */
-export function countAdjacentInlineElements(children: Node[]): number {
+export function countAdjacentInlineElements(children: Node[], startIndex = 0, processedIndices?: Set<number>): number {
   let count = 0
   let lastSignificantIndex = -1
 
-  for (let i = 0; i < children.length; i++) {
+  for (let i = startIndex; i < children.length; i++) {
     const child = children[i]
 
     if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
       continue
     }
 
+    if (processedIndices?.has(i)) {
+      break
+    }
+
     const isInlineOrERB = (isNode(child, HTMLElementNode) && isInlineElement(getTagName(child))) || isNode(child, ERBContentNode)
 
     if (!isInlineOrERB) {

javascript/packages/formatter/src/format-printer.ts

@@ -1881,7 +1881,7 @@ export class FormatPrinter extends Printer {
       }
     }
 
-    const match = trimmedText.match(/^[.!?:;]+/)
+    const match = trimmedText.match(/^[.!?:;%]+/)
 
     if (!match) {
       return {
@@ -1940,19 +1940,23 @@ export class FormatPrinter extends Printer {
   /**
    * Render adjacent inline elements together on one line
    */
-  private renderAdjacentInlineElements(children: Node[], count: number): { processedIndices: Set<number> } {
+  private renderAdjacentInlineElements(children: Node[], count: number, startIndex = 0, alreadyProcessed?: Set<number>): { processedIndices: Set<number>; lastIndex: number } {
     let inlineContent = ""
     let processedCount = 0
     let lastProcessedIndex = -1
     const processedIndices = new Set<number>()
 
-    for (let index = 0; index < children.length && processedCount < count; index++) {
+    for (let index = startIndex; index < children.length && processedCount < count; index++) {
       const child = children[index]
 
       if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
         continue
       }
 
+      if (alreadyProcessed?.has(index)) {
+        continue
+      }
+
       if (isNode(child, HTMLElementNode) && isInlineElement(getTagName(child))) {
         inlineContent += this.renderInlineElementAsString(child)
         processedCount++
@@ -1979,21 +1983,27 @@ export class FormatPrinter extends Printer {
           continue
         }
 
+        if (alreadyProcessed?.has(index)) {
+          break
+        }
+
         if (isNode(child, ERBContentNode)) {
           inlineContent += this.renderERBAsString(child)
           processedIndices.add(index)
+          lastProcessedIndex = index
           continue
         }
 
         if (isNode(child, HTMLTextNode)) {
           const trimmed = child.content.trim()
 
-          if (trimmed && /^[.!?:;]/.test(trimmed)) {
+          if (trimmed && /^[.!?:;%]/.test(trimmed)) {
             const wrapWidth = this.maxLineLength - this.indent.length
             const result = this.tryMergePunctuationText(inlineContent, trimmed, wrapWidth)
 
             inlineContent = result.mergedContent
             processedIndices.add(index)
+            lastProcessedIndex = index
 
             if (result.shouldStop) {
               if (inlineContent) {
@@ -2002,7 +2012,7 @@ export class FormatPrinter extends Printer {
 
               result.wrappedLines.forEach(line => this.push(line))
 
-              return { processedIndices }
+              return { processedIndices, lastIndex: lastProcessedIndex }
             }
           }
         }
@@ -2015,7 +2025,10 @@ export class FormatPrinter extends Printer {
       this.pushWithIndent(inlineContent)
     }
 
-    return { processedIndices }
+    return {
+      processedIndices,
+      lastIndex: lastProcessedIndex >= 0 ? lastProcessedIndex : startIndex + count - 1
+    }
   }
 
   /**
@@ -2054,21 +2067,37 @@ export class FormatPrinter extends Printer {
   }
 
   /**
-   * Visit remaining children after processing adjacent inline elements
+   * Visit remaining children after processing adjacent inline elements.
+   * Detects and renders subsequent groups of adjacent inline elements.
    */
   private visitRemainingChildren(children: Node[], processedIndices: Set<number>): void {
-    for (let index = 0; index < children.length; index++) {
+    let index = 0
+
+    while (index < children.length) {
       const child = children[index]
 
       if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
+        index++
         continue
       }
 
       if (processedIndices.has(index)) {
+        index++
         continue
       }
 
-      this.visit(child)
+      const adjacentCount = countAdjacentInlineElements(children, index, processedIndices)
+
+      if (adjacentCount >= 2) {
+        const { processedIndices: newProcessedIndices, lastIndex } =
+          this.renderAdjacentInlineElements(children, adjacentCount, index, processedIndices)
+
+        newProcessedIndices.forEach(i => processedIndices.add(i))
+        index = lastIndex + 1
+      } else {
+        this.visit(child)
+        index++
+      }
     }
   }
 

javascript/packages/formatter/test/erb/erb.test.ts

@@ -269,6 +269,30 @@ describe("@herb-tools/formatter", () => {
     expect(secondFormat).toBe(result)
   })
 
+  test("issue 978: does not duplicate content with multiple adjacent inline/ERB groups separated by br", () => {
+    const input = dedent`
+      <p class="text-reversed">
+        <strong><%= t("admin.tickets.form.savings") %></strong><%= ticket.ticketable.savings_percentage %>%
+        <br>
+        <strong><%= t("admin.tickets.form.price_per_ticket") %></strong><%= format_price(ticket.ticketable.price_per_ticket) %>
+      </p>
+    `
+
+    const result = formatter.format(input)
+
+    expect(result).toBe(dedent`
+      <p class="text-reversed">
+        <strong><%= t("admin.tickets.form.savings") %></strong><%= ticket.ticketable.savings_percentage %>%
+        <br>
+        <strong><%= t("admin.tickets.form.price_per_ticket") %></strong><%= format_price(ticket.ticketable.price_per_ticket) %>
+      </p>
+    `)
+
+    // Test idempotency
+    const secondFormat = formatter.format(result)
+    expect(secondFormat).toBe(result)
+  })
+
   test("https://github.com/hanakai-rb/site/blob/8adc128d9d464f3e37615be2aa29d57979904533/app/templates/pages/home.html.erb", () => {
     const input = dedent`
       <h1>Hanakai</h1>