Handlebars: fix formatting of attributes (#10145)

* Handlebars: fix formatting of attributes

* Add group only for class

* Add changelog

* Update clean.js

* Simplify ConcatStatement

* Lowercase attr name in clean

* Use text instead of n.chars

changelog_unreleased/handlebars/10145.md

@@ -0,0 +1,25 @@
+#### Fix formatting of attributes (#10145 by @thorn0)
+
+- fix escaping of `{{` in attributes and text
+- fix the choice between `'` and `"` for attributes with interpolations
+- fix the bug with `[object Object]` printed in the `class` attribute
+- implement simple formatting for the `class` attribute, like Prettier formatted it in HTML before v2.3.0
+
+<!-- prettier-ignore -->
+```html
+// Input
+<div class="
+foo"></div>
+<div bar='"{{expr}}"'></div>
+<div baz="\{{ non-expression }}"></div>
+
+// Prettier stable
+<div class="[object Object],foo"></div>
+<div bar=""{{expr}}""></div>
+<div baz="{{ non-expression }}"></div>
+
+// Prettier main
+<div class="foo"></div>
+<div bar='"{{expr}}"'></div>
+<div baz="\{{ non-expression }}"></div>
+```

src/language-handlebars/clean.js

@@ -9,6 +9,11 @@ function clean(ast, newNode /*, parent*/) {
     }
     newNode.chars = trimmed;
   }
+
+  // `class` is reformatted
+  if (ast.type === "AttrNode" && ast.name.toLowerCase() === "class") {
+    delete newNode.value;
+  }
 }
 clean.ignoredProperties = new Set(["loc", "selfClosing"]);
 

src/language-handlebars/printer-glimmer.js

@@ -2,10 +2,10 @@
 
 const {
   builders: { dedent, group, hardline, ifBreak, indent, join, line, softline },
-  utils: { getDocParts },
 } = require("../document");
-const { isNonEmptyArray } = require("../common/util");
+const { isNonEmptyArray, replaceEndOfLineWith } = require("../common/util");
 
+const { literalline } = require("../document/doc-builders");
 const { locStart, locEnd } = require("./loc");
 const clean = require("./clean");
 const {
@@ -152,23 +152,34 @@ function print(path, options, print) {
       if (isEmptyText && locStart(n.value) === locEnd(n.value)) {
         return n.name;
       }
-      const value = path.call(print, "value");
-      const quotedValue = isText
-        ? printStringLiteral(
-            typeof value === "string" ? value : getDocParts(value).join(),
-            options
-          )
-        : value;
-      return [n.name, "=", quotedValue];
+
+      // Let's assume quotes inside the content of text nodes are already
+      // properly escaped with entities, otherwise the parse wouldn't have parsed them.
+      const quote = isText
+        ? chooseEnclosingQuote(options, n.value.chars).quote
+        : n.value.type === "ConcatStatement"
+        ? chooseEnclosingQuote(
+            options,
+            n.value.parts
+              .filter((part) => part.type === "TextNode")
+              .map((part) => part.chars)
+              .join("")
+          ).quote
+        : "";
+
+      const valueDoc = path.call(print, "value");
+
+      return [
+        n.name,
+        "=",
+        quote,
+        n.name === "class" && quote ? group(indent(valueDoc)) : valueDoc,
+        quote,
+      ];
     }
 
     case "ConcatStatement": {
-      const quote = options.singleQuote ? "'" : '"';
-      return [
-        quote,
-        ...path.map((partPath) => print(partPath), "parts"),
-        quote,
-      ];
+      return path.map(print, "parts");
     }
 
     case "Hash": {
@@ -178,18 +189,57 @@ function print(path, options, print) {
       return [n.key, "=", path.call(print, "value")];
     }
     case "TextNode": {
-      const inAttrNode = path.stack.includes("attributes");
+      /* if `{{my-component}}` (or any text containing "{{")
+       * makes it to the TextNode, it means it was escaped,
+       * so let's print it escaped, ie.; `\{{my-component}}` */
+      let text = n.chars.replace(/{{/g, "\\{{");
 
-      if (options.htmlWhitespaceSensitivity === "strict" && !inAttrNode) {
+      const attrName = getCurrentAttributeName(path);
+
+      if (attrName) {
+        // TODO: format style and srcset attributes
+        if (attrName === "class") {
+          const formattedClasses = text.trim().split(/\s+/).join(" ");
+
+          let leadingSpace = false;
+          let trailingSpace = false;
+
+          if (isParentOfSomeType(path, ["ConcatStatement"])) {
+            if (
+              isPreviousNodeOfSomeType(path, ["MustacheStatement"]) &&
+              /^\s/.test(text)
+            ) {
+              leadingSpace = true;
+            }
+            if (
+              isNextNodeOfSomeType(path, ["MustacheStatement"]) &&
+              /\s$/.test(text) &&
+              formattedClasses !== ""
+            ) {
+              trailingSpace = true;
+            }
+          }
+
+          return [
+            leadingSpace ? line : "",
+            formattedClasses,
+            trailingSpace ? line : "",
+          ];
+        }
+
+        return replaceEndOfLineWith(text, literalline);
+      }
+
+      if (options.htmlWhitespaceSensitivity === "strict") {
         // https://infra.spec.whatwg.org/#ascii-whitespace
         const leadingWhitespacesRE = /^[\t\n\f\r ]*/;
         const trailingWhitespacesRE = /[\t\n\f\r ]*$/;
         const whitespacesOnlyRE = /^[\t\n\f\r ]*$/;
 
-        if (whitespacesOnlyRE.test(n.chars)) {
+        if (whitespacesOnlyRE.test(text)) {
           let breaks = [line];
 
-          const newlines = countNewLines(n.chars);
+          const newlines = countNewLines(text);
           if (newlines) {
             breaks = generateHardlines(newlines, 2);
           }
@@ -201,10 +251,8 @@ function print(path, options, print) {
           return breaks;
         }
 
-        const [lead] = n.chars.match(leadingWhitespacesRE);
-        const [tail] = n.chars.match(trailingWhitespacesRE);
-
-        let text = n.chars;
+        const [lead] = text.match(leadingWhitespacesRE);
+        const [tail] = text.match(trailingWhitespacesRE);
 
         let leadBreaks = [];
         if (lead) {
@@ -240,11 +288,11 @@ function print(path, options, print) {
       const maxLineBreaksToPreserve = 2;
       const isFirstElement = !getPreviousNode(path);
       const isLastElement = !getNextNode(path);
-      const isWhitespaceOnly = !/\S/.test(n.chars);
-      const lineBreaksCount = countNewLines(n.chars);
+      const isWhitespaceOnly = !/\S/.test(text);
+      const lineBreaksCount = countNewLines(text);
 
-      let leadingLineBreaksCount = countLeadingNewLines(n.chars);
-      let trailingLineBreaksCount = countTrailingNewLines(n.chars);
+      let leadingLineBreaksCount = countLeadingNewLines(text);
+      let trailingLineBreaksCount = countTrailingNewLines(text);
 
       if (
         (isFirstElement || isLastElement) &&
@@ -270,34 +318,6 @@ function print(path, options, print) {
         }
       }
 
-      if (inAttrNode) {
-        // TODO: format style and srcset attributes
-        if (!isInAttributeOfName(path, "class")) {
-          return n.chars;
-        }
-
-        let leadingSpace = "";
-        let trailingSpace = "";
-
-        if (isParentOfSomeType(path, ["ConcatStatement"])) {
-          if (isPreviousNodeOfSomeType(path, ["MustacheStatement"])) {
-            leadingSpace = " ";
-          }
-          if (isNextNodeOfSomeType(path, ["MustacheStatement"])) {
-            trailingSpace = " ";
-          }
-        }
-
-        return [
-          ...generateHardlines(leadingLineBreaksCount, maxLineBreaksToPreserve),
-          n.chars.replace(/^\s+/g, leadingSpace).replace(/\s+$/, trailingSpace),
-          ...generateHardlines(
-            trailingLineBreaksCount,
-            maxLineBreaksToPreserve
-          ),
-        ];
-      }
-
       let leadingSpace = "";
       let trailingSpace = "";
 
@@ -325,15 +345,6 @@ function print(path, options, print) {
         trailingSpace = "";
       }
 
-      let text = n.chars;
-      /* if `{{my-component}}` (or any text starting with a mustache)
-       * makes it to the TextNode,
-       * it means it was escaped,
-       * so let's print it escaped, ie.; `\{{my-component}}` */
-      if (text.startsWith("{{") && text.includes("}}")) {
-        text = "\\" + text;
-      }
-
       return [
         ...generateHardlines(leadingLineBreaksCount, maxLineBreaksToPreserve),
         text.replace(/^\s+/g, leadingSpace).replace(/\s+$/, trailingSpace),
@@ -617,13 +628,13 @@ function printInverse(path, print, options) {
 
 /* TextNode print helpers */
 
-function isInAttributeOfName(path, type) {
-  return (
-    (isParentOfSomeType(path, ["AttrNode"]) &&
-      path.getParentNode().name.toLowerCase() === type) ||
-    (isParentOfSomeType(path, ["ConcatStatement"]) &&
-      path.getParentNode(1).name.toLowerCase() === type)
-  );
+function getCurrentAttributeName(path) {
+  for (let depth = 0; depth < 2; depth++) {
+    const parentNode = path.getParentNode(depth);
+    if (parentNode && parentNode.type === "AttrNode") {
+      return parentNode.name.toLowerCase();
+    }
+  }
 }
 
 function countNewLines(string) {
@@ -662,6 +673,11 @@ function generateHardlines(number = 0, max = 0) {
  * @param {object} options - the prettier options object
  */
 function printStringLiteral(stringLiteral, options) {
+  const { quote, regex } = chooseEnclosingQuote(options, stringLiteral);
+  return [quote, stringLiteral.replace(regex, `\\${quote}`), quote];
+}
+
+function chooseEnclosingQuote(options, stringLiteral) {
   const double = { quote: '"', regex: /"/g };
   const single = { quote: "'", regex: /'/g };
 
@@ -685,13 +701,7 @@ function printStringLiteral(stringLiteral, options) {
     shouldUseAlternateQuote = numPreferredQuotes > numAlternateQuotes;
   }
 
-  const enclosingQuote = shouldUseAlternateQuote ? alternate : preferred;
-  const escapedStringLiteral = stringLiteral.replace(
-    enclosingQuote.regex,
-    `\\${enclosingQuote.quote}`
-  );
-
-  return [enclosingQuote.quote, escapedStringLiteral, enclosingQuote.quote];
+  return shouldUseAlternateQuote ? alternate : preferred;
 }
 
 /* SubExpression print helpers */

tests/handlebars/attr-node/__snapshots__/jsfmt.spec.js.snap

@@ -41,21 +41,26 @@ printWidth: 80
 '></div>
 
 =====================================output=====================================
-<div title="
+<div
+  title="
 
   a title with random
 
 
-  newlines"></div>
+  newlines"
+></div>
 
-<div TITLE="
+<div
+  TITLE="
 
   a title with random
 
 
-  newlines"></div>
+  newlines"
+></div>
 
-<div title="
+<div
+  title="
 
   a title with random
 
@@ -63,9 +68,11 @@ printWidth: 80
   newlines
   and {{mustaches}}
 
-"></div>
+"
+></div>
 
-<div TITLE="
+<div
+  TITLE="
 
   a title with random
 
@@ -73,7 +80,8 @@ printWidth: 80
   newlines
   and {{mustaches}}
 
-"></div>
+"
+></div>
 ================================================================================
 `;
 
@@ -108,38 +116,38 @@ weirdly formatted
 
 {{this.classes}}">String</p>
 
+<Foo class="
+"/>
+
 =====================================output=====================================
 <div class="class and other"></div>
 <div class="class and other"></div>
 <div CLASs="class and other"></div>
-<p class="a list of
-
-
-weirdly formatted
-
-
-classes">
+<p class="a list of weirdly formatted classes">
   String
 </p>
-<p
-  class="a list of
-
-
-weirdly formatted
-
-  {{this.classes}}"
->
+<p class="a list of weirdly formatted {{this.classes}}">
   String
 </p>
-<p
-  CLASS="a list of
-
-
-weirdly formatted
-
-  {{this.classes}}"
->
+<p CLASS="a list of weirdly formatted {{this.classes}}">
   String
 </p>
+
+<Foo class="" />
+================================================================================
+`;
+
+exports[`quotes.hbs format 1`] = `
+====================================options=====================================
+parsers: ["glimmer"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+<div title="'' {{ foo }}"></div>
+<div title='"" {{ bar }}'></div>
+
+=====================================output=====================================
+<div title="'' {{foo}}"></div>
+<div title='"" {{bar}}'></div>
 ================================================================================
 `;

tests/handlebars/attr-node/class-name.hbs

@@ -22,3 +22,6 @@ weirdly formatted
 
 
 {{this.classes}}">String</p>
+
+<Foo class="
+"/>

tests/handlebars/attr-node/quotes.hbs

@@ -0,0 +1,2 @@
+<div title="'' {{ foo }}"></div>
+<div title='"" {{ bar }}'></div>

tests/handlebars/basics/__snapshots__/jsfmt.spec.js.snap

@@ -68,8 +68,7 @@ printWidth: 80
   Hello
 </div>
 
-<ul class="list-unstyled
-  one-tab">
+<ul class="list-unstyled one-tab">
 </ul>
 ================================================================================
 `;

tests/handlebars/concat-statement/__snapshots__/jsfmt.spec.js.snap

@@ -33,9 +33,12 @@ singleQuote: true
 </div>
 
 <div
-  class='hello {{if goodbye true}} {{if goodbye false}} {{if goodbye true}} {{
-    if goodbye false
-  }} {{if goodbye true}}'
+  class='hello
+    {{if goodbye true}}
+    {{if goodbye false}}
+    {{if goodbye true}}
+    {{if goodbye false}}
+    {{if goodbye true}}'
 >
   Hello
 </div>
@@ -51,7 +54,7 @@ singleQuote: true
 <div class='class-i {{myClass}}class-j'></div>
 <div class='class-k {{myClass}} class-l'></div>
 <div class='class-m {{myClass}} class-n {{myClass}}class-o'></div>
-<div class='class-p  class-q'></div>
+<div class='class-p class-q'></div>
 ================================================================================
 `;
 
@@ -87,9 +90,12 @@ printWidth: 80
 </div>
 
 <div
-  class="hello {{if goodbye true}} {{if goodbye false}} {{if goodbye true}} {{
-    if goodbye false
-  }} {{if goodbye true}}"
+  class="hello
+    {{if goodbye true}}
+    {{if goodbye false}}
+    {{if goodbye true}}
+    {{if goodbye false}}
+    {{if goodbye true}}"
 >
   Hello
 </div>
@@ -105,7 +111,7 @@ printWidth: 80
 <div class="class-i {{myClass}}class-j"></div>
 <div class="class-k {{myClass}} class-l"></div>
 <div class="class-m {{myClass}} class-n {{myClass}}class-o"></div>
-<div class="class-p  class-q"></div>
+<div class="class-p class-q"></div>
 ================================================================================
 `;
 
@@ -126,9 +132,9 @@ singleQuote: true
 
 =====================================output=====================================
 <div
-  class='a very long list of classes that exceeds {{
-    eighty
-  }} chars with emtpy spaces'
+  class='a very long list of classes that exceeds
+    {{eighty}}
+    chars with emtpy spaces'
 >
   hey
 </div>
@@ -141,8 +147,9 @@ singleQuote: true
 </a>
 <button
   class='uk-padding-remove uk-button uk-button-default uk-button-small uk-width-1-{{
-    intl.locales.length
-  }} {{if (contains locale intl.locale) 'uk-button-primary'}}'
+      intl.locales.length
+    }}
+    {{if (contains locale intl.locale) 'uk-button-primary'}}'
 >
   test
 </button>
@@ -165,9 +172,9 @@ printWidth: 80
 
 =====================================output=====================================
 <div
-  class="a very long list of classes that exceeds {{
-    eighty
-  }} chars with emtpy spaces"
+  class="a very long list of classes that exceeds
+    {{eighty}}
+    chars with emtpy spaces"
 >
   hey
 </div>
@@ -180,8 +187,9 @@ printWidth: 80
 </a>
 <button
   class="uk-padding-remove uk-button uk-button-default uk-button-small uk-width-1-{{
-    intl.locales.length
-  }} {{if (contains locale intl.locale) "uk-button-primary"}}"
+      intl.locales.length
+    }}
+    {{if (contains locale intl.locale) "uk-button-primary"}}"
 >
   test
 </button>

tests/handlebars/escape/__snapshots__/jsfmt.spec.js.snap

@@ -13,3 +13,22 @@ printWidth: 80
 </p>
 ================================================================================
 `;
+
+exports[`mustache.hbs format 1`] = `
+====================================options=====================================
+parsers: ["glimmer"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+<div title="foo \\{{"> \\{{ </div>
+<div class=" bar \\{{">\\{{</div>
+
+=====================================output=====================================
+<div title="foo \\{{">
+  \\{{
+</div>
+<div class="bar \\{{">
+  \\{{
+</div>
+================================================================================
+`;

tests/handlebars/escape/mustache.hbs

@@ -0,0 +1,2 @@
+<div title="foo \{{"> \{{ </div>
+<div class=" bar \{{">\{{</div>

tests/handlebars/mustache-statement/__snapshots__/jsfmt.spec.js.snap

@@ -99,41 +99,37 @@ singleQuote: true
 />
 
 <div
-  class='a long list of classes more than 80 chars with a complex mustache statement {{
-    if (or this.a this.b) this.c this.d
-  }}'
+  class='a long list of classes more than 80 chars with a complex mustache statement
+    {{if (or this.a this.b) this.c this.d}}'
 >
 </div>
 
 <div
   class='a multi line list of classes with a complex mustache statement
-  {{if (or this.a this.b) this.c this.d}}'
+    {{if (or this.a this.b) this.c this.d}}'
 >
 </div>
 
 <div
-  class='a-first-class {{if this.optionOne 'optionOne'}} {{
-    if this.optionTwo 'optionTwo'
-  }} {{if this.optionThree 'optionThree'}} {{
-    if this.optionFour 'optionFour'
-  }} {{if this.optionFive 'optionFive'}} {{this.class}}'
+  class='a-first-class
+    {{if this.optionOne 'optionOne'}}
+    {{if this.optionTwo 'optionTwo'}}
+    {{if this.optionThree 'optionThree'}}
+    {{if this.optionFour 'optionFour'}}
+    {{if this.optionFive 'optionFive'}}
+    {{this.class}}'
   ...attributes
 >
 </div>
 
 <div
   class='a-first-class
-  {{if this.optionOne 'optionOne'}}
-
-   {{if this.optionTwo 'optionTwo'}}
-
-   {{if this.optionThree 'optionThree'}}
-
-   {{if this.optionFour 'optionFour'}}
-
-   {{if this.optionFive 'optionFive'}}
-
-   {{this.class}}'
+    {{if this.optionOne 'optionOne'}}
+    {{if this.optionTwo 'optionTwo'}}
+    {{if this.optionThree 'optionThree'}}
+    {{if this.optionFour 'optionFour'}}
+    {{if this.optionFive 'optionFive'}}
+    {{this.class}}'
   ...attributes
 >
 </div>
@@ -266,41 +262,37 @@ printWidth: 80
 />
 
 <div
-  class="a long list of classes more than 80 chars with a complex mustache statement {{
-    if (or this.a this.b) this.c this.d
-  }}"
+  class="a long list of classes more than 80 chars with a complex mustache statement
+    {{if (or this.a this.b) this.c this.d}}"
 >
 </div>
 
 <div
   class="a multi line list of classes with a complex mustache statement
-  {{if (or this.a this.b) this.c this.d}}"
+    {{if (or this.a this.b) this.c this.d}}"
 >
 </div>
 
 <div
-  class="a-first-class {{if this.optionOne "optionOne"}} {{
-    if this.optionTwo "optionTwo"
-  }} {{if this.optionThree "optionThree"}} {{
-    if this.optionFour "optionFour"
-  }} {{if this.optionFive "optionFive"}} {{this.class}}"
+  class="a-first-class
+    {{if this.optionOne "optionOne"}}
+    {{if this.optionTwo "optionTwo"}}
+    {{if this.optionThree "optionThree"}}
+    {{if this.optionFour "optionFour"}}
+    {{if this.optionFive "optionFive"}}
+    {{this.class}}"
   ...attributes
 >
 </div>
 
 <div
   class="a-first-class
-  {{if this.optionOne "optionOne"}}
-
-   {{if this.optionTwo "optionTwo"}}
-
-   {{if this.optionThree "optionThree"}}
-
-   {{if this.optionFour "optionFour"}}
-
-   {{if this.optionFive "optionFive"}}
-
-   {{this.class}}"
+    {{if this.optionOne "optionOne"}}
+    {{if this.optionTwo "optionTwo"}}
+    {{if this.optionThree "optionThree"}}
+    {{if this.optionFour "optionFour"}}
+    {{if this.optionFive "optionFive"}}
+    {{this.class}}"
   ...attributes
 >
 </div>

tests/handlebars/single-quote-option/__snapshots__/jsfmt.spec.js.snap

@@ -98,24 +98,20 @@ singleQuote: false
 <a href="/{{url}}/{{url}}"></a>
 
 <button
-  class="padding width-{{locales.length}} {{
-    if (has locale) "uk-button-primary"
-  }}"
+  class="padding width-{{locales.length}}
+    {{if (has locale) "uk-button-primary"}}"
 ></button>
 <button
-  class="padding width-{{locales.length}} {{
-    if (has locale) "uk-button-primary"
-  }}"
+  class="padding width-{{locales.length}}
+    {{if (has locale) "uk-button-primary"}}"
 ></button>
 <button
-  class="padding width-{{locales.length}} {{
-    if (has locale) "uk-button-primary"
-  }}"
+  class="padding width-{{locales.length}}
+    {{if (has locale) "uk-button-primary"}}"
 ></button>
 <button
-  class="padding width-{{locales.length}} {{
-    if (has locale) "uk-button-primary"
-  }}"
+  class="padding width-{{locales.length}}
+    {{if (has locale) "uk-button-primary"}}"
 ></button>
 ================================================================================
 `;
@@ -150,24 +146,20 @@ singleQuote: true
 <a href='/{{url}}/{{url}}'></a>
 
 <button
-  class='padding width-{{locales.length}} {{
-    if (has locale) 'uk-button-primary'
-  }}'
+  class='padding width-{{locales.length}}
+    {{if (has locale) 'uk-button-primary'}}'
 ></button>
 <button
-  class='padding width-{{locales.length}} {{
-    if (has locale) 'uk-button-primary'
-  }}'
+  class='padding width-{{locales.length}}
+    {{if (has locale) 'uk-button-primary'}}'
 ></button>
 <button
-  class='padding width-{{locales.length}} {{
-    if (has locale) 'uk-button-primary'
-  }}'
+  class='padding width-{{locales.length}}
+    {{if (has locale) 'uk-button-primary'}}'
 ></button>
 <button
-  class='padding width-{{locales.length}} {{
-    if (has locale) 'uk-button-primary'
-  }}'
+  class='padding width-{{locales.length}}
+    {{if (has locale) 'uk-button-primary'}}'
 ></button>
 ================================================================================
 `;