diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 7860b30c..5a5c3f6b 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -188,3 +188,27 @@ Examples: width, }); ``` + +- JavaScript: Don't break simple template literals ([#5979] by [@jwbay]) + + + ```js + // Input + console.log(chalk.white(`Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${coverageSummary.total.lines.pct}%`)) + + // Output (Prettier stable) + console.log( + chalk.white( + `Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${ + coverageSummary.total.lines.pct + }%` + ) + ); + + // Output (Prettier master) + console.log( + chalk.white( + `Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${coverageSummary.total.lines.pct}%` + ) + ); + ``` diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index b9c440d7..4575d5c1 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -2364,127 +2364,29 @@ function printPathNoParens(path, options, print, args) { case "TemplateElement": return join(literalline, n.value.raw.split(/\r?\n/g)); case "TemplateLiteral": { - const expressions = path.map(print, "expressions"); + let expressions = path.map(print, "expressions"); const parentNode = path.getParentNode(); - /** - * describe.each`table`(name, fn) - * describe.only.each`table`(name, fn) - * describe.skip.each`table`(name, fn) - * test.each`table`(name, fn) - * test.only.each`table`(name, fn) - * test.skip.each`table`(name, fn) - * - * Ref: https://github.com/facebook/jest/pull/6102 - */ - const jestEachTriggerRegex = /^[xf]?(describe|it|test)$/; - if ( - parentNode.type === "TaggedTemplateExpression" && - parentNode.quasi === n && - parentNode.tag.type === "MemberExpression" && - parentNode.tag.property.type === "Identifier" && - parentNode.tag.property.name === "each" && - ((parentNode.tag.object.type === "Identifier" && - jestEachTriggerRegex.test(parentNode.tag.object.name)) || - (parentNode.tag.object.type === "MemberExpression" && - parentNode.tag.object.property.type === "Identifier" && - (parentNode.tag.object.property.name === "only" || - parentNode.tag.object.property.name === "skip") && - parentNode.tag.object.object.type === "Identifier" && - jestEachTriggerRegex.test(parentNode.tag.object.object.name))) - ) { - /** - * a | b | expected - * ${1} | ${1} | ${2} - * ${1} | ${2} | ${3} - * ${2} | ${1} | ${3} - */ - const headerNames = n.quasis[0].value.raw.trim().split(/\s*\|\s*/); - if ( - headerNames.length > 1 || - headerNames.some(headerName => headerName.length !== 0) - ) { - const stringifiedExpressions = expressions.map( - doc => - "${" + - printDocToString( - doc, - Object.assign({}, options, { - printWidth: Infinity, - endOfLine: "lf" - }) - ).formatted + - "}" - ); - - const tableBody = [{ hasLineBreak: false, cells: [] }]; - for (let i = 1; i < n.quasis.length; i++) { - const row = tableBody[tableBody.length - 1]; - const correspondingExpression = stringifiedExpressions[i - 1]; - - row.cells.push(correspondingExpression); - if (correspondingExpression.indexOf("\n") !== -1) { - row.hasLineBreak = true; - } - - if (n.quasis[i].value.raw.indexOf("\n") !== -1) { - tableBody.push({ hasLineBreak: false, cells: [] }); - } - } - - const maxColumnCount = tableBody.reduce( - (maxColumnCount, row) => Math.max(maxColumnCount, row.cells.length), - headerNames.length - ); - - const maxColumnWidths = Array.from( - new Array(maxColumnCount), - () => 0 - ); - const table = [{ cells: headerNames }].concat( - tableBody.filter(row => row.cells.length !== 0) - ); - table - .filter(row => !row.hasLineBreak) - .forEach(row => { - row.cells.forEach((cell, index) => { - maxColumnWidths[index] = Math.max( - maxColumnWidths[index], - getStringWidth(cell) - ); - }); - }); - - parts.push( - "`", - indent( - concat([ - hardline, - join( - hardline, - table.map(row => - join( - " | ", - row.cells.map((cell, index) => - row.hasLineBreak - ? cell - : cell + - " ".repeat( - maxColumnWidths[index] - getStringWidth(cell) - ) - ) - ) - ) - ) - ]) - ), - hardline, - "`" - ); - return concat(parts); + if (isJestEachTemplateLiteral(n, parentNode)) { + const printed = printJestEachTemplateLiteral(n, expressions, options); + if (printed) { + return printed; } } + const isSimple = isSimpleTemplateLiteral(n); + if (isSimple) { + expressions = expressions.map( + doc => + printDocToString( + doc, + Object.assign({}, options, { + printWidth: Infinity + }) + ).formatted + ); + } + parts.push("`"); path.each(childPath => { @@ -2510,13 +2412,17 @@ function printPathNoParens(path, options, print, args) { let printed = expressions[i]; - if ( - (n.expressions[i].comments && n.expressions[i].comments.length) || - n.expressions[i].type === "MemberExpression" || - n.expressions[i].type === "OptionalMemberExpression" || - n.expressions[i].type === "ConditionalExpression" - ) { - printed = concat([indent(concat([softline, printed])), softline]); + if (!isSimple) { + // Breaks at the template element boundaries (${ and }) are preferred to breaking + // in the middle of a MemberExpression + if ( + (n.expressions[i].comments && n.expressions[i].comments.length) || + n.expressions[i].type === "MemberExpression" || + n.expressions[i].type === "OptionalMemberExpression" || + n.expressions[i].type === "ConditionalExpression" + ) { + printed = concat([indent(concat([softline, printed])), softline]); + } } const aligned = @@ -3863,6 +3769,172 @@ function isSimpleFlowType(node) { ); } +function isJestEachTemplateLiteral(node, parentNode) { + /** + * describe.each`table`(name, fn) + * describe.only.each`table`(name, fn) + * describe.skip.each`table`(name, fn) + * test.each`table`(name, fn) + * test.only.each`table`(name, fn) + * test.skip.each`table`(name, fn) + * + * Ref: https://github.com/facebook/jest/pull/6102 + */ + const jestEachTriggerRegex = /^[xf]?(describe|it|test)$/; + return ( + parentNode.type === "TaggedTemplateExpression" && + parentNode.quasi === node && + parentNode.tag.type === "MemberExpression" && + parentNode.tag.property.type === "Identifier" && + parentNode.tag.property.name === "each" && + ((parentNode.tag.object.type === "Identifier" && + jestEachTriggerRegex.test(parentNode.tag.object.name)) || + (parentNode.tag.object.type === "MemberExpression" && + parentNode.tag.object.property.type === "Identifier" && + (parentNode.tag.object.property.name === "only" || + parentNode.tag.object.property.name === "skip") && + parentNode.tag.object.object.type === "Identifier" && + jestEachTriggerRegex.test(parentNode.tag.object.object.name))) + ); +} + +function printJestEachTemplateLiteral(node, expressions, options) { + /** + * a | b | expected + * ${1} | ${1} | ${2} + * ${1} | ${2} | ${3} + * ${2} | ${1} | ${3} + */ + const headerNames = node.quasis[0].value.raw.trim().split(/\s*\|\s*/); + if ( + headerNames.length > 1 || + headerNames.some(headerName => headerName.length !== 0) + ) { + const parts = []; + const stringifiedExpressions = expressions.map( + doc => + "${" + + printDocToString( + doc, + Object.assign({}, options, { + printWidth: Infinity, + endOfLine: "lf" + }) + ).formatted + + "}" + ); + + const tableBody = [{ hasLineBreak: false, cells: [] }]; + for (let i = 1; i < node.quasis.length; i++) { + const row = tableBody[tableBody.length - 1]; + const correspondingExpression = stringifiedExpressions[i - 1]; + + row.cells.push(correspondingExpression); + if (correspondingExpression.indexOf("\n") !== -1) { + row.hasLineBreak = true; + } + + if (node.quasis[i].value.raw.indexOf("\n") !== -1) { + tableBody.push({ hasLineBreak: false, cells: [] }); + } + } + + const maxColumnCount = tableBody.reduce( + (maxColumnCount, row) => Math.max(maxColumnCount, row.cells.length), + headerNames.length + ); + + const maxColumnWidths = Array.from(new Array(maxColumnCount), () => 0); + const table = [{ cells: headerNames }].concat( + tableBody.filter(row => row.cells.length !== 0) + ); + table + .filter(row => !row.hasLineBreak) + .forEach(row => { + row.cells.forEach((cell, index) => { + maxColumnWidths[index] = Math.max( + maxColumnWidths[index], + getStringWidth(cell) + ); + }); + }); + + parts.push( + "`", + indent( + concat([ + hardline, + join( + hardline, + table.map(row => + join( + " | ", + row.cells.map((cell, index) => + row.hasLineBreak + ? cell + : cell + + " ".repeat(maxColumnWidths[index] - getStringWidth(cell)) + ) + ) + ) + ) + ]) + ), + hardline, + "`" + ); + return concat(parts); + } +} + +/** @param node {import("estree").TemplateLiteral} */ +function isSimpleTemplateLiteral(node) { + if (node.expressions.length === 0) { + return false; + } + + return node.expressions.every(expr => { + // Disallow comments since printDocToString can't print them here + if (expr.comments) { + return false; + } + + // Allow `x` and `this` + if (expr.type === "Identifier" || expr.type === "ThisExpression") { + return true; + } + + // Allow `a.b.c`, `a.b[c]`, and `this.x.y` + if ( + (expr.type === "MemberExpression" || + expr.type === "OptionalMemberExpression") && + (expr.property.type === "Identifier" || expr.property.type === "Literal") + ) { + let ancestor = expr; + while ( + ancestor.type === "MemberExpression" || + ancestor.type === "OptionalMemberExpression" + ) { + ancestor = ancestor.object; + if (ancestor.comments) { + return false; + } + } + + if ( + ancestor.type === "Identifier" || + ancestor.type === "ThisExpression" + ) { + return true; + } + + return false; + } + + return false; + }); +} + const functionCompositionFunctionNames = new Set([ "pipe", // RxJS, Ramda "pipeP", // Ramda diff --git a/tests/template_literals/__snapshots__/jsfmt.spec.js.snap b/tests/template_literals/__snapshots__/jsfmt.spec.js.snap index b076d809..4e9221f1 100644 --- a/tests/template_literals/__snapshots__/jsfmt.spec.js.snap +++ b/tests/template_literals/__snapshots__/jsfmt.spec.js.snap @@ -88,6 +88,11 @@ const description = const foo = \`such a long template string \${foo.bar.baz} that prettier will want to wrap it\`; +const shouldWrapForNow = \`such a long template string \${foo().bar.baz} that prettier will want to wrap it\`; + +const shouldNotWrap = \`simple expressions should not break \${this} \${variable} \${a.b.c} \${this.b.c} \${a[b].c} \${a.b[c]} \${a.b['c']} \${a?.b?.c}\`; + +console.log(chalk.white(\`Covered Lines below threshold: \${coverageSettings.lines}%. Actual: \${coverageSummary.total.lines.pct}%\`)) x = \`mdl-textfield mdl-js-textfield \${className} \${content.length > 0 ? 'is-dirty' @@ -125,22 +130,26 @@ const long = \`long \${ } long longlong \${a.b.c.d.e} long longlong \${a.b.c.d.e} long longlong \${ a.b.c.d.e } long long\`; -const long = \`long \${ - a.b.c.d.e -} long longlong \${loooooooooooooooooong} long longlong \${loooooooooooooooooong} long longlong \${loooooooooooooooooong} long long\`; +const long = \`long \${a.b.c.d.e} long longlong \${loooooooooooooooooong} long longlong \${loooooooooooooooooong} long longlong \${loooooooooooooooooong} long long\`; -const long = \`long long long long long long long long long long long \${ - a.b.c.d.e -} long long long long long long long long long long long long long\`; +const long = \`long long long long long long long long long long long \${a.b.c.d.e} long long long long long long long long long long long long long\`; -const description = \`The value of the \${cssName} css of the \${ - this._name -} element\`; +const description = \`The value of the \${cssName} css of the \${this._name} element\`; -const foo = \`such a long template string \${ - foo.bar.baz +const foo = \`such a long template string \${foo.bar.baz} that prettier will want to wrap it\`; + +const shouldWrapForNow = \`such a long template string \${ + foo().bar.baz } that prettier will want to wrap it\`; +const shouldNotWrap = \`simple expressions should not break \${this} \${variable} \${a.b.c} \${this.b.c} \${a[b].c} \${a.b[c]} \${a.b["c"]} \${a?.b?.c}\`; + +console.log( + chalk.white( + \`Covered Lines below threshold: \${coverageSettings.lines}%. Actual: \${coverageSummary.total.lines.pct}%\` + ) +); + x = \`mdl-textfield mdl-js-textfield \${className} \${ content.length > 0 ? "is-dirty" : "" } combo-box__input\`; @@ -160,9 +169,7 @@ function testing() { } console.log( - \`Trying update appcast for \${app.name} (\${app.cask.appcast}) -> (\${ - app.cask.appcastGenerated - })\` + \`Trying update appcast for \${app.name} (\${app.cask.appcast}) -> (\${app.cask.appcastGenerated})\` ); console.log( @@ -170,9 +177,7 @@ console.log( ); console.log( - \`\\nApparently jetbrains changed the release artifact for \${app.name}@\${ - app.jetbrains.version - }.\\n\` + \`\\nApparently jetbrains changed the release artifact for \${app.name}@\${app.jetbrains.version}.\\n\` ); descirbe("something", () => { diff --git a/tests/template_literals/expressions.js b/tests/template_literals/expressions.js index 3265f836..c6e3febd 100644 --- a/tests/template_literals/expressions.js +++ b/tests/template_literals/expressions.js @@ -9,6 +9,11 @@ const description = const foo = `such a long template string ${foo.bar.baz} that prettier will want to wrap it`; +const shouldWrapForNow = `such a long template string ${foo().bar.baz} that prettier will want to wrap it`; + +const shouldNotWrap = `simple expressions should not break ${this} ${variable} ${a.b.c} ${this.b.c} ${a[b].c} ${a.b[c]} ${a.b['c']} ${a?.b?.c}`; + +console.log(chalk.white(`Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${coverageSummary.total.lines.pct}%`)) x = `mdl-textfield mdl-js-textfield ${className} ${content.length > 0 ? 'is-dirty'