From 8846448b85565d553013d9bbfc76bbbef21edbbe Mon Sep 17 00:00:00 2001 From: Georgii Dolzhykov Date: Thu, 28 Jan 2021 23:45:08 +0200 Subject: [PATCH] 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 | 25 +++ src/language-handlebars/clean.js | 5 + src/language-handlebars/printer-glimmer.js | 168 ++++++++++-------- .../__snapshots__/jsfmt.spec.js.snap | 70 ++++---- tests/handlebars/attr-node/class-name.hbs | 3 + tests/handlebars/attr-node/quotes.hbs | 2 + .../basics/__snapshots__/jsfmt.spec.js.snap | 3 +- .../__snapshots__/jsfmt.spec.js.snap | 44 +++-- .../escape/__snapshots__/jsfmt.spec.js.snap | 19 ++ tests/handlebars/escape/mustache.hbs | 2 + .../__snapshots__/jsfmt.spec.js.snap | 72 ++++---- .../__snapshots__/jsfmt.spec.js.snap | 40 ++--- 12 files changed, 259 insertions(+), 194 deletions(-) create mode 100644 changelog_unreleased/handlebars/10145.md create mode 100644 tests/handlebars/attr-node/quotes.hbs create mode 100644 tests/handlebars/escape/mustache.hbs diff --git a/changelog_unreleased/handlebars/10145.md b/changelog_unreleased/handlebars/10145.md new file mode 100644 index 000000000..2ba7277f5 --- /dev/null +++ b/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 + + +```html +// Input +
+
+
+ +// Prettier stable +
+
+
+ +// Prettier main +
+
+
+``` diff --git a/src/language-handlebars/clean.js b/src/language-handlebars/clean.js index db7b843ff..2ded535ca 100644 --- a/src/language-handlebars/clean.js +++ b/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"]); diff --git a/src/language-handlebars/printer-glimmer.js b/src/language-handlebars/printer-glimmer.js index 408c0cb4e..fccfa2dea 100644 --- a/src/language-handlebars/printer-glimmer.js +++ b/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 */ diff --git a/tests/handlebars/attr-node/__snapshots__/jsfmt.spec.js.snap b/tests/handlebars/attr-node/__snapshots__/jsfmt.spec.js.snap index f5438b7a5..455724984 100644 --- a/tests/handlebars/attr-node/__snapshots__/jsfmt.spec.js.snap +++ b/tests/handlebars/attr-node/__snapshots__/jsfmt.spec.js.snap @@ -41,21 +41,26 @@ printWidth: 80 '> =====================================output===================================== -
+ newlines" +> -
+ newlines" +> -
+" +> -
+" +> ================================================================================ `; @@ -108,38 +116,38 @@ weirdly formatted {{this.classes}}">String

+ + =====================================output=====================================
-

+

String

-

+

String

-

+

String

+ + +================================================================================ +`; + +exports[`quotes.hbs format 1`] = ` +====================================options===================================== +parsers: ["glimmer"] +printWidth: 80 + | printWidth +=====================================input====================================== +
+
+ +=====================================output===================================== +
+
================================================================================ `; diff --git a/tests/handlebars/attr-node/class-name.hbs b/tests/handlebars/attr-node/class-name.hbs index c14f0fbe9..93d5a2b96 100644 --- a/tests/handlebars/attr-node/class-name.hbs +++ b/tests/handlebars/attr-node/class-name.hbs @@ -22,3 +22,6 @@ weirdly formatted {{this.classes}}">String

+ + diff --git a/tests/handlebars/attr-node/quotes.hbs b/tests/handlebars/attr-node/quotes.hbs new file mode 100644 index 000000000..75726267b --- /dev/null +++ b/tests/handlebars/attr-node/quotes.hbs @@ -0,0 +1,2 @@ +
+
diff --git a/tests/handlebars/basics/__snapshots__/jsfmt.spec.js.snap b/tests/handlebars/basics/__snapshots__/jsfmt.spec.js.snap index 56317cf5a..33c6a0773 100644 --- a/tests/handlebars/basics/__snapshots__/jsfmt.spec.js.snap +++ b/tests/handlebars/basics/__snapshots__/jsfmt.spec.js.snap @@ -68,8 +68,7 @@ printWidth: 80 Hello -
    +
    ================================================================================ `; diff --git a/tests/handlebars/concat-statement/__snapshots__/jsfmt.spec.js.snap b/tests/handlebars/concat-statement/__snapshots__/jsfmt.spec.js.snap index c66778ee0..f3b990352 100644 --- a/tests/handlebars/concat-statement/__snapshots__/jsfmt.spec.js.snap +++ b/tests/handlebars/concat-statement/__snapshots__/jsfmt.spec.js.snap @@ -33,9 +33,12 @@ singleQuote: true
    Hello
    @@ -51,7 +54,7 @@ singleQuote: true
    -
    +
    ================================================================================ `; @@ -87,9 +90,12 @@ printWidth: 80
    Hello
    @@ -105,7 +111,7 @@ printWidth: 80
    -
    +
    ================================================================================ `; @@ -126,9 +132,9 @@ singleQuote: true =====================================output=====================================
    hey
    @@ -141,8 +147,9 @@ singleQuote: true @@ -165,9 +172,9 @@ printWidth: 80 =====================================output=====================================
    hey
    @@ -180,8 +187,9 @@ printWidth: 80 diff --git a/tests/handlebars/escape/__snapshots__/jsfmt.spec.js.snap b/tests/handlebars/escape/__snapshots__/jsfmt.spec.js.snap index 63fa6f626..53584eb01 100644 --- a/tests/handlebars/escape/__snapshots__/jsfmt.spec.js.snap +++ b/tests/handlebars/escape/__snapshots__/jsfmt.spec.js.snap @@ -13,3 +13,22 @@ printWidth: 80

    ================================================================================ `; + +exports[`mustache.hbs format 1`] = ` +====================================options===================================== +parsers: ["glimmer"] +printWidth: 80 + | printWidth +=====================================input====================================== +
    \\{{
    +
    \\{{
    + +=====================================output===================================== +
    + \\{{ +
    +
    + \\{{ +
    +================================================================================ +`; diff --git a/tests/handlebars/escape/mustache.hbs b/tests/handlebars/escape/mustache.hbs new file mode 100644 index 000000000..96e7a2f8c --- /dev/null +++ b/tests/handlebars/escape/mustache.hbs @@ -0,0 +1,2 @@ +
    \{{
    +
    \{{
    diff --git a/tests/handlebars/mustache-statement/__snapshots__/jsfmt.spec.js.snap b/tests/handlebars/mustache-statement/__snapshots__/jsfmt.spec.js.snap index 7890ecc17..c5aadaabb 100644 --- a/tests/handlebars/mustache-statement/__snapshots__/jsfmt.spec.js.snap +++ b/tests/handlebars/mustache-statement/__snapshots__/jsfmt.spec.js.snap @@ -99,41 +99,37 @@ singleQuote: true />
    @@ -266,41 +262,37 @@ printWidth: 80 />
    diff --git a/tests/handlebars/single-quote-option/__snapshots__/jsfmt.spec.js.snap b/tests/handlebars/single-quote-option/__snapshots__/jsfmt.spec.js.snap index 379c2bf64..891b513bc 100644 --- a/tests/handlebars/single-quote-option/__snapshots__/jsfmt.spec.js.snap +++ b/tests/handlebars/single-quote-option/__snapshots__/jsfmt.spec.js.snap @@ -98,24 +98,20 @@ singleQuote: false ================================================================================ `; @@ -150,24 +146,20 @@ singleQuote: true ================================================================================ `;