From ac8580eb6bbe901ab8e98a1233ba84d3270f83b7 Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Mon, 5 Sep 2016 13:32:12 -0400 Subject: [PATCH] Fix: no-implicit-coercion string concat false positive (fixes #7057) --- lib/rules/no-implicit-coercion.js | 46 +++++++++++++++++-------- tests/lib/rules/no-implicit-coercion.js | 12 ++++++- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/lib/rules/no-implicit-coercion.js b/lib/rules/no-implicit-coercion.js index 0810781cf583..27ec87213471 100644 --- a/lib/rules/no-implicit-coercion.js +++ b/lib/rules/no-implicit-coercion.js @@ -5,6 +5,8 @@ "use strict"; +const astUtils = require("../ast-utils"); + //------------------------------------------------------------------------------ // Helpers //------------------------------------------------------------------------------ @@ -105,6 +107,26 @@ function getNonNumericOperand(node) { return null; } +/** + * Checks whether a node is a string literal or not. + * @param {ASTNode} node The node to check. + * @returns {boolean} Whether or not the passed in node is a + * string literal or not. + */ +function isStringLiteral(node) { + return astUtils.isStringLiteral(node) && node.type !== "TemplateLiteral"; +} + +/** + * Checks whether a node is an empty string literal or not. + * @param {ASTNode} node The node to check. + * @returns {boolean} Whether or not the passed in node is an + * empty string literal or not. + */ +function isEmptyString(node) { + return isStringLiteral(node) && node.value === ""; +} + /** * Checks whether or not a node is a concatenating with an empty string. * @param {ASTNode} node - A BinaryExpression node to check. @@ -112,8 +134,8 @@ function getNonNumericOperand(node) { */ function isConcatWithEmptyString(node) { return node.operator === "+" && ( - (node.left.type === "Literal" && node.left.value === "") || - (node.right.type === "Literal" && node.right.value === "") + (isEmptyString(node.left) && !isStringLiteral(node.right)) || + (isEmptyString(node.right) && !isStringLiteral(node.left)) ); } @@ -123,20 +145,16 @@ function isConcatWithEmptyString(node) { * @returns {boolean} Whether or not the node is appended with an empty string. */ function isAppendEmptyString(node) { - return node.operator === "+=" && node.right.type === "Literal" && node.right.value === ""; + return node.operator === "+=" && isEmptyString(node.right); } /** - * Gets a node that is the left or right operand of a node, is not the specified literal. - * @param {ASTNode} node - A BinaryExpression node to get. - * @param {any} value - A literal value to check. - * @returns {ASTNode} A node that is the left or right operand of the node, is not the specified literal. + * Returns the operand that is not an empty string from a flagged BinaryExpression. + * @param {ASTNode} node - The flagged BinaryExpression node to check. + * @returns {ASTNode} The operand that is not an empty string from a flagged BinaryExpression. */ -function getOtherOperand(node, value) { - if (node.left.type === "Literal" && node.left.value === value) { - return node.right; - } - return node.left; +function getNonEmptyOperand(node) { + return isEmptyString(node.left) ? node.right : node.left; } //------------------------------------------------------------------------------ @@ -236,7 +254,7 @@ module.exports = { context.report( node, "use `String({{code}})` instead.", { - code: sourceCode.getText(getOtherOperand(node, "")) + code: sourceCode.getText(getNonEmptyOperand(node)) }); } }, @@ -250,7 +268,7 @@ module.exports = { context.report( node, "use `{{code}} = String({{code}})` instead.", { - code: sourceCode.getText(getOtherOperand(node, "")) + code: sourceCode.getText(getNonEmptyOperand(node)) }); } } diff --git a/tests/lib/rules/no-implicit-coercion.js b/tests/lib/rules/no-implicit-coercion.js index 639ef046cb75..e42e648bc03c 100644 --- a/tests/lib/rules/no-implicit-coercion.js +++ b/tests/lib/rules/no-implicit-coercion.js @@ -64,6 +64,12 @@ ruleTester.run("no-implicit-coercion", rule, { {code: "0 + foo"}, {code: "~foo.bar()"}, + // https://github.com/eslint/eslint/issues/7057 + {code: "'' + 'foo'"}, + {code: "'foo' + ''"}, + {code: "foo += 'bar'"}, + {code: "+42"}, + {code: "!!foo", options: [{boolean: false}]}, {code: "~foo.indexOf(1)", options: [{boolean: false}]}, {code: "+foo", options: [{number: false}]}, @@ -95,6 +101,10 @@ ruleTester.run("no-implicit-coercion", rule, { {code: "var a = ~foo.indexOf(1)", options: [{boolean: true, allow: ["!!"]}], errors: [{message: "use `foo.indexOf(1) !== -1` instead.", type: "UnaryExpression"}]}, {code: "var a = 1 * foo", options: [{boolean: true, allow: ["+"]}], errors: [{message: "use `Number(foo)` instead.", type: "BinaryExpression"}]}, {code: "var a = +foo", options: [{boolean: true, allow: ["*"]}], errors: [{message: "use `Number(foo)` instead.", type: "UnaryExpression"}]}, - {code: "var a = \"\" + foo", options: [{boolean: true, allow: ["*"]}], errors: [{message: "use `String(foo)` instead.", type: "BinaryExpression"}]} + {code: "var a = \"\" + foo", options: [{boolean: true, allow: ["*"]}], errors: [{message: "use `String(foo)` instead.", type: "BinaryExpression"}]}, + + // https://github.com/eslint/eslint/issues/7057 + {code: "'' + foo", errors: [{message: "use `String(foo)` instead.", type: "BinaryExpression"}]}, + {code: "foo + ''", errors: [{message: "use `String(foo)` instead.", type: "BinaryExpression"}]}, ] });