Skip to content

Commit

Permalink
Update: add fixer for prefer-template (fixes #6978) (#7165)
Browse files Browse the repository at this point in the history
  • Loading branch information
@not-an-aardvark @nzakas
not-an-aardvark authored and nzakas committed Sep 20, 2016
1 parent 745343f commit 91bf477
Show file tree
Hide file tree
Showing 3 changed files with 276 additions and 12 deletions.
2 changes: 2 additions & 0 deletions docs/rules/prefer-template.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Suggest using template literals instead of string concatenation. (prefer-template)

(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule.

In ES2015 (ES6), we can use template literals instead of string concatenation.

```js
Expand Down
130 changes: 126 additions & 4 deletions lib/rules/prefer-template.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ function getTopConcatBinaryExpression(node) {
return node;
}

/**
* Checks whether or not a given binary expression has string literals.
* @param {ASTNode} node - A node to check.
* @returns {boolean} `true` if the node has string literals.
*/
function hasStringLiteral(node) {
if (isConcatenation(node)) {

// `left` is deeper than `right` normally.
return hasStringLiteral(node.right) || hasStringLiteral(node.left);
}
return astUtils.isStringLiteral(node);
}

/**
* Checks whether or not a given binary expression has non string literals.
* @param {ASTNode} node - A node to check.
Expand All @@ -50,6 +64,36 @@ function hasNonStringLiteral(node) {
return !astUtils.isStringLiteral(node);
}

/**
* Determines whether a given node will start with a template curly expression (`${}`) when being converted to a template literal.
* @param {ASTNode} node The node that will be fixed to a template literal
* @returns {boolean} `true` if the node will start with a template curly.
*/
function startsWithTemplateCurly(node) {
if (node.type === "BinaryExpression") {
return startsWithTemplateCurly(node.left);
}
if (node.type === "TemplateLiteral") {
return node.expressions.length && node.quasis.length && node.quasis[0].start === node.quasis[0].end;
}
return node.type !== "Literal" || typeof node.value !== "string";
}

/**
* Determines whether a given node end with a template curly expression (`${}`) when being converted to a template literal.
* @param {ASTNode} node The node that will be fixed to a template literal
* @returns {boolean} `true` if the node will end with a template curly.
*/
function endsWithTemplateCurly(node) {
if (node.type === "BinaryExpression") {
return startsWithTemplateCurly(node.right);
}
if (node.type === "TemplateLiteral") {
return node.expressions.length && node.quasis.length && node.quasis[node.quasis.length - 1].start === node.quasis[node.quasis.length - 1].end;
}
return node.type !== "Literal" || typeof node.value !== "string";
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -62,12 +106,86 @@ module.exports = {
recommended: false
},

schema: []
schema: [],

fixable: "code"
},

create(context) {
const sourceCode = context.getSourceCode();
let done = Object.create(null);

/**
* Gets the non-token text between two nodes, ignoring any other tokens that appear between the two tokens.
* @param {ASTNode} node1 The first node
* @param {ASTNode} node2 The second node
* @returns {string} The text between the nodes, excluding other tokens
*/
function getTextBetween(node1, node2) {
const allTokens = [node1].concat(sourceCode.getTokensBetween(node1, node2)).concat(node2);
const sourceText = sourceCode.getText();

return allTokens.slice(0, -1).reduce((accumulator, token, index) => accumulator + sourceText.slice(token.range[1], allTokens[index + 1].range[0]), "");
}

/**
* Returns a template literal form of the given node.
* @param {ASTNode} currentNode A node that should be converted to a template literal
* @param {string} textBeforeNode Text that should appear before the node
* @param {string} textAfterNode Text that should appear after the node
* @returns {string} A string form of this node, represented as a template literal
*/
function getTemplateLiteral(currentNode, textBeforeNode, textAfterNode) {
if (currentNode.type === "Literal" && typeof currentNode.value === "string") {

// If the current node is a string literal, escape any instances of ${ or ` to prevent them from being interpreted
// as a template placeholder. However, if the code already contains a backslash before the ${ or `
// for some reason, don't add another backslash, because that would change the meaning of the code (it would cause
// an actual backslash character to appear before the dollar sign).
return `\`${currentNode.raw.slice(1, -1).replace(/\\*(\${|`)/g, matched => {
if (matched.lastIndexOf("\\") % 2) {
return `\\${matched}`;
}
return matched;
})}\``;
}

if (currentNode.type === "TemplateLiteral") {
return sourceCode.getText(currentNode);
}

if (isConcatenation(currentNode) && hasStringLiteral(currentNode) && hasNonStringLiteral(currentNode)) {
const plusSign = sourceCode.getTokensBetween(currentNode.left, currentNode.right).find(token => token.value === "+");
const textBeforePlus = getTextBetween(currentNode.left, plusSign);
const textAfterPlus = getTextBetween(plusSign, currentNode.right);
const leftEndsWithCurly = endsWithTemplateCurly(currentNode.left);
const rightStartsWithCurly = startsWithTemplateCurly(currentNode.right);

if (leftEndsWithCurly) {

// If the left side of the expression ends with a template curly, add the extra text to the end of the curly bracket.
// `foo${bar}` /* comment */ + 'baz' --> `foo${bar /* comment */ }${baz}`
return getTemplateLiteral(currentNode.left, textBeforeNode, textBeforePlus + textAfterPlus).slice(0, -1) +
getTemplateLiteral(currentNode.right, null, textAfterNode).slice(1);
}
if (rightStartsWithCurly) {

// Otherwise, if the right side of the expression starts with a template curly, add the text there.
// 'foo' /* comment */ + `${bar}baz` --> `foo${ /* comment */ bar}baz`
return getTemplateLiteral(currentNode.left, textBeforeNode, null).slice(0, -1) +
getTemplateLiteral(currentNode.right, textBeforePlus + textAfterPlus, textAfterNode).slice(1);
}

// Otherwise, these nodes should not be combined into a template curly, since there is nowhere to put
// the text between them.
return getTemplateLiteral(currentNode.left, textBeforeNode, null) +
textBeforePlus + "+" + textAfterPlus +
getTemplateLiteral(currentNode.right, textAfterNode, null);
}

return `\`\${${textBeforeNode || ""}${sourceCode.getText(currentNode)}${textAfterNode || ""}}\``;
}

/**
* Reports if a given node is string concatenation with non string literals.
*
Expand All @@ -88,9 +206,13 @@ module.exports = {
done[topBinaryExpr.range[0]] = true;

if (hasNonStringLiteral(topBinaryExpr)) {
context.report(
topBinaryExpr,
"Unexpected string concatenation.");
context.report({
node: topBinaryExpr,
message: "Unexpected string concatenation.",
fix(fixer) {
return fixer.replaceText(topBinaryExpr, getTemplateLiteral(topBinaryExpr, null, null));
}
});
}
}

Expand Down
156 changes: 148 additions & 8 deletions tests/lib/rules/prefer-template.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,153 @@ ruleTester.run("prefer-template", rule, {
{code: "var foo = `foo` +\n `bar` +\n \"hoge\";", parserOptions: { ecmaVersion: 6 }}
],
invalid: [
{code: "var foo = 'hello, ' + name + '!';", errors},
{code: "var foo = bar + 'baz';", errors},
{code: "var foo = bar + `baz`;", parserOptions: { ecmaVersion: 6 }, errors},
{code: "var foo = +100 + 'yen';", errors},
{code: "var foo = 'bar' + baz;", errors},
{code: "var foo = '￥' + (n * 1000) + '-'", errors},
{code: "var foo = 'aaa' + aaa; var bar = 'bbb' + bbb;", errors: [errors[0], errors[0]]},
{code: "var string = (number + 1) + 'px';", errors}
{
code: "var foo = 'hello, ' + name + '!';",
output: "var foo = `hello, ${ name }!`;",
errors,
parserOptions: { ecmaVersion: 6 }
},
{
code: "var foo = bar + 'baz';",
output: "var foo = `${bar }baz`;",
errors,
parserOptions: { ecmaVersion: 6 }
},
{
code: "var foo = bar + `baz`;",
output: "var foo = `${bar }baz`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = +100 + 'yen';",
output: "var foo = `${+100 }yen`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = 'bar' + baz;",
output: "var foo = `bar${ baz}`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = '￥' + (n * 1000) + '-'",
output: "var foo = `￥${ n * 1000 }-`",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = 'aaa' + aaa; var bar = 'bbb' + bbb;",
output: "var foo = `aaa${ aaa}`; var bar = `bbb${ bbb}`;",
parserOptions: { ecmaVersion: 6 },
errors: [errors[0], errors[0]]
},
{
code: "var string = (number + 1) + 'px';",
output: "var string = `${number + 1 }px`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = 'bar' + baz + 'qux';",
output: "var foo = `bar${ baz }qux`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = '0 backslashes: ${bar}' + baz;",
output: "var foo = `0 backslashes: \\${bar}${ baz}`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = '1 backslash: \\${bar}' + baz;",
output: "var foo = `1 backslash: \\${bar}${ baz}`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = '2 backslashes: \\\\${bar}' + baz;",
output: "var foo = `2 backslashes: \\\\\\${bar}${ baz}`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = '3 backslashes: \\\\\\${bar}' + baz;",
output: "var foo = `3 backslashes: \\\\\\${bar}${ baz}`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = bar + 'this is a backtick: `' + baz;",
output: "var foo = `${bar }this is a backtick: \\`${ baz}`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = bar + 'this is a backtick preceded by a backslash: \\`' + baz;",
output: "var foo = `${bar }this is a backtick preceded by a backslash: \\`${ baz}`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = bar + 'this is a backtick preceded by two backslashes: \\\\`' + baz;",
output: "var foo = `${bar }this is a backtick preceded by two backslashes: \\\\\\`${ baz}`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = bar + `${baz}foo`;",
output: "var foo = `${bar }${baz}foo`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code:
"var foo = 'favorites: ' + favorites.map(f => {\n" +
" return f.name;\n" +
"}) + ';';",
output:
"var foo = `favorites: ${ favorites.map(f => {\n" +
" return f.name;\n" +
"}) };`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = bar + baz + 'qux';",
output: "var foo = `${bar + baz }qux`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code:
"var foo = 'favorites: ' +\n" +
" favorites.map(f => {\n" +
" return f.name;\n" +
" }) +\n" +
"';';",
output:
"var foo = `favorites: ${ \n" +
" favorites.map(f => {\n" +
" return f.name;\n" +
" }) \n" +
"};`;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = /* a */ 'bar' /* b */ + /* c */ baz /* d */ + 'qux' /* e */ ;",
output: "var foo = /* a */ `bar${ /* b */ /* c */ baz /* d */ }qux` /* e */ ;",
parserOptions: { ecmaVersion: 6 },
errors
},
{
code: "var foo = bar + ('baz') + 'qux' + (boop);",
output: "var foo = `${bar }baz` + `qux${ boop}`;",
parserOptions: { ecmaVersion: 6 },
errors
}
]
});

0 comments on commit 91bf477

Please sign in to comment.