Don't break liquid tags inside attributes when sorting classes (#143)

* Revert "Revert "Fix Liquid `capture` sorting (#135)" (#140)"

This reverts commit 2aabc3c.

* Fix off-by-1 error when String nodes don’t have quotes

* Refactor

* Add tests

* Simplify test cases

* Update changelog

CHANGELOG.md

@@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-- Nothing yet!
+### Fixed
+
+- Don't break liquid tags inside attributes when sorting classes ([#143](https://github.com/tailwindlabs/prettier-plugin-tailwindcss/pull/143))
 
 ## [0.2.6] - 2023-03-29
 

src/index.js

@@ -379,55 +379,89 @@ function transformLiquid(ast, { env }) {
       : node.name === 'class'
   }
 
-  function sortAttribute(attr, path) {
+  /**
+   * @param {string} str
+   */
+  function hasSurroundingQuotes(str) {
+    let start = str[0]
+    let end = str[str.length - 1]
+
+    return start === end && (start === '"' || start === "'" || start === "`")
+  }
+
+  /** @type {{type: string, source: string}[]} */
+  let sources = []
+
+  /** @type {{pos: {start: number, end: number}, value: string}[]} */
+  let changes = []
+
+  function sortAttribute(attr) {
     visit(attr.value, {
       TextNode(node) {
         node.value = sortClasses(node.value, { env });
-
-        let source = node.source.slice(0, node.position.start) + node.value + node.source.slice(node.position.end)
-        path.forEach(node => (node.source = source))
+        changes.push({
+          pos: node.position,
+          value: node.value,
+        })
       },
 
       String(node) {
-        node.value = sortClasses(node.value, { env });
+        let pos = { ...node.position }
+
+        // We have to offset the position ONLY when quotes are part of the String node
+        // This is because `value` does NOT include quotes
+        if (hasSurroundingQuotes(node.source.slice(pos.start, pos.end))) {
+          pos.start += 1;
+          pos.end -= 1;
+        }
 
-        // String position includes the quotes even if the value doesn't
-        // Hence the +1 and -1 when slicing
-        let source = node.source.slice(0, node.position.start+1) + node.value + node.source.slice(node.position.end-1)
-        path.forEach(node => (node.source = source))
+        node.value = sortClasses(node.value, { env })
+        changes.push({
+          pos,
+          value: node.value,
+        })
       },
     })
   }
 
   visit(ast, {
-    LiquidTag(node, _parent, _key, _index, meta) {
-      meta.path = [...meta.path ?? [], node];
+    LiquidTag(node) {
+      sources.push(node)
     },
 
-    HtmlElement(node, _parent, _key, _index, meta) {
-      meta.path = [...meta.path ?? [], node];
+    HtmlElement(node) {
+      sources.push(node)
     },
 
-    AttrSingleQuoted(node, _parent, _key, _index, meta) {
-      if (!isClassAttr(node)) {
-        return;
+    AttrSingleQuoted(node) {
+      if (isClassAttr(node)) {
+        sources.push(node)
+        sortAttribute(node)
       }
-
-      meta.path = [...meta.path ?? [], node];
-
-      sortAttribute(node, meta.path)
     },
 
-    AttrDoubleQuoted(node, _parent, _key, _index, meta) {
-      if (!isClassAttr(node)) {
-        return;
+    AttrDoubleQuoted(node) {
+      if (isClassAttr(node)) {
+        sources.push(node)
+        sortAttribute(node)
       }
-
-      meta.path = [...meta.path ?? [], node];
-
-      sortAttribute(node, meta.path)
     },
   });
+
+  // Sort so all changes occur in order
+  changes = changes.sort((a, b) => {
+    return a.start - b.start
+        || a.end - b.end
+  })
+
+  for (let change of changes) {
+    for (let node of sources) {
+      node.source =
+        node.source.slice(0, change.pos.start) +
+        change.value +
+        node.source.slice(change.pos.end)
+    }
+  }
 }
 
 function sortStringLiteral(node, { env }) {

tests/plugins.test.js

@@ -1,5 +1,6 @@
 const prettier = require('prettier')
 const path = require('path')
+const { t, yes } = require('./utils')
 
 function format(str, options = {}) {
   return prettier
@@ -245,18 +246,18 @@ let tests = [
     ],
     tests: {
       'liquid-html': [
-        [
-          `<a class="sm:p-0 p-4" href="https://www.example.com">Example</a>`,
-          `<a class='p-4 sm:p-0' href='https://www.example.com'>Example</a>`,
-        ],
-        [
-          `{% if state == true %}\n  <a class="{{ "sm:p-0 p-4" | escape }}" href="https://www.example.com">Example</a>\n{% endif %}`,
-          `{% if state == true %}\n  <a class='{{ "p-4 sm:p-0" | escape }}' href='https://www.example.com'>Example</a>\n{% endif %}`,
-        ],
-        [
-          `{%- capture class_ordering -%}<div class="sm:p-0 p-4"></div>{%- endcapture -%}`,
-          `{%- capture class_ordering -%}<div class="p-4 sm:p-0"></div>{%- endcapture -%}`,
-        ],
+        t`<a class='${yes}' href='https://www.example.com'>Example</a>`,
+        t`{% if state == true %}\n  <a class='{{ "${yes}" | escape }}' href='https://www.example.com'>Example</a>\n{% endif %}`,
+        t`{%- capture class_ordering -%}<div class="${yes}"></div>{%- endcapture -%}`,
+        t`{%- capture class_ordering -%}<div class="foo1 ${yes}"></div><div class="foo2 ${yes}"></div>{%- endcapture -%}`,
+        t`{%- capture class_ordering -%}<div class="foo1 ${yes}"><div class="foo2 ${yes}"></div></div>{%- endcapture -%}`,
+        t`<p class='${yes} {{ some.prop | prepend: 'is-' }} '></p>`,
+        t`<div class='${yes} {% render 'some-snippet', settings: section.settings %}'></div>`,
+        t`<div class='${yes} {{ foo }}'></div>`,
+        t`<div class='${yes} {% render 'foo' %}'></div>`,
+        t`<div class='${yes} {% render 'foo', bar: true %}'></div>`,
+        t`<div class='${yes} {% include 'foo' %}'></div>`,
+        t`<div class='${yes} {% include 'foo', bar: true %}'></div>`,
       ],
     }
   },

tests/test.js

@@ -1,6 +1,7 @@
 const prettier = require('prettier')
 const path = require('path')
 const { execSync } = require('child_process')
+const { t, yes, no } = require('./utils')
 
 function format(str, options = {}) {
   options.plugins = options.plugins ?? [
@@ -38,28 +39,6 @@ function formatFixture(name) {
     .trim()
 }
 
-let yes = '__YES__'
-let no = '__NO__'
-let testClassName = 'sm:p-0 p-0'
-let testClassNameSorted = 'p-0 sm:p-0'
-
-function t(strings, ...values) {
-  let input = ''
-  strings.forEach((string, i) => {
-    input += string + (values[i] ? testClassName : '')
-  })
-
-  let output = ''
-  strings.forEach((string, i) => {
-    let value = values[i] || ''
-    if (value === yes) value = testClassNameSorted
-    else if (value === no) value = testClassName
-    output += string + value
-  })
-
-  return [input, output]
-}
-
 let html = [
   t`<div class="${yes}"></div>`,
   t`<!-- <div class="${no}"></div> -->`,

tests/utils.js

@@ -0,0 +1,24 @@
+let testClassName = 'sm:p-0 p-0'
+let testClassNameSorted = 'p-0 sm:p-0'
+let yes = '__YES__'
+let no = '__NO__'
+
+module.exports.yes = yes
+module.exports.no = no
+
+module.exports.t = function t(strings, ...values) {
+  let input = ''
+  strings.forEach((string, i) => {
+    input += string + (values[i] ? testClassName : '')
+  })
+
+  let output = ''
+  strings.forEach((string, i) => {
+    let value = values[i] || ''
+    if (value === yes) value = testClassNameSorted
+    else if (value === no) value = testClassName
+    output += string + value
+  })
+
+  return [input, output]
+}