fix: Prevent false positives with no-constant-condition (#15486)

Fixes #15467 by defining `isConstant` with `inBooleanPosition` set to `false` as meaning: > For both string and number, if coerced to that type, the value will be constant.
eslint · Jan 11, 2022 · 03ac8cf · 03ac8cf
1 parent 564ecdb
commit 03ac8cf
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 20 deletions.
diff --git a/lib/rules/no-constant-condition.js b/lib/rules/no-constant-condition.js
@@ -124,7 +124,8 @@ module.exports = {
          * Checks if a node has a constant truthiness value.
          * @param {ASTNode} node The AST node to check.
          * @param {boolean} inBooleanPosition `false` if checking branch of a condition.
-         *  `true` in all other cases
+         *  `true` in all other cases. When `false`, checks if -- for both string and
+         *  number -- if coerced to that type, the value will be constant.
          * @returns {Bool} true when node's truthiness is constant
          * @private
          */
@@ -138,15 +139,31 @@ module.exports = {
                 case "Literal":
                 case "ArrowFunctionExpression":
                 case "FunctionExpression":
-                case "ObjectExpression":
+                    return true;
                 case "ClassExpression":
+                case "ObjectExpression":
+
+                    /**
+                     * In theory objects like:
+                     *
+                     * `{toString: () => a}`
+                     * `{valueOf: () => a}`
+                     *
+                     * Or a classes like:
+                     *
+                     * `class { static toString() { return a } }`
+                     * `class { static valueOf() { return a } }`
+                     *
+                     * Are not constant verifiably when `inBooleanPosition` is
+                     * false, but it's an edge case we've opted not to handle.
+                     */
                     return true;
                 case "TemplateLiteral":
                     return (inBooleanPosition && node.quasis.some(quasi => quasi.value.cooked.length)) ||
-                        node.expressions.every(exp => isConstant(exp, inBooleanPosition));
+                        node.expressions.every(exp => isConstant(exp, false));
 
                 case "ArrayExpression": {
-                    if (node.parent.type === "BinaryExpression" && node.parent.operator === "+") {
+                    if (!inBooleanPosition) {
                         return node.elements.every(element => isConstant(element, false));
                     }
                     return true;
@@ -196,6 +213,8 @@ module.exports = {
 
                 case "SequenceExpression":
                     return isConstant(node.expressions[node.expressions.length - 1], inBooleanPosition);
+                case "SpreadElement":
+                    return isConstant(node.argument, inBooleanPosition);
 
                 // no default
             }

diff --git a/tests/lib/rules/no-constant-condition.js b/tests/lib/rules/no-constant-condition.js
@@ -175,7 +175,17 @@ ruleTester.run("no-constant-condition", rule, {
         "function* foo() {while (true) {function* foo() {yield;}yield;}}",
         "function* foo() { for (let x = yield; x < 10; x++) {yield;}yield;}",
         "function* foo() { for (let x = yield; ; x++) { yield; }}",
-        "if (new Number(x) + 1 === 2) {}"
+        "if (new Number(x) + 1 === 2) {}",
+
+        // #15467
+        "if([a]==[b]) {}",
+        "if (+[...a]) {}",
+        "if (+[...[...a]]) {}",
+        "if (`${[...a]}`) {}",
+        "if (`${[a]}`) {}",
+        "if (+[a]) {}",
+        "if (0 - [a]) {}",
+        "if (1 * [a]) {}"
     ],
     invalid: [
         { code: "for(;true;);", errors: [{ messageId: "unexpected", type: "Literal" }] },
@@ -262,8 +272,6 @@ ruleTester.run("no-constant-condition", rule, {
 
         // #5228 , typeof conditions
         { code: "if(typeof x){}", errors: [{ messageId: "unexpected", type: "UnaryExpression" }] },
-        { code: "if(`${typeof x}`){}", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
-        { code: "if(`${''}${typeof x}`){}", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
         { code: "if(typeof 'abc' === 'string'){}", errors: [{ messageId: "unexpected", type: "BinaryExpression" }] },
         { code: "if(a = typeof b){}", errors: [{ messageId: "unexpected", type: "AssignmentExpression" }] },
         { code: "if(a, typeof b){}", errors: [{ messageId: "unexpected", type: "SequenceExpression" }] },
@@ -358,18 +366,6 @@ ruleTester.run("no-constant-condition", rule, {
             code: "if(''+[]) {}",
             errors: [{ messageId: "unexpected", type: "BinaryExpression" }]
         },
-        {
-            code: "if([a]==[a]) {}",
-            errors: [{ messageId: "unexpected", type: "BinaryExpression" }]
-        },
-        {
-            code: "if([a] - '') {}",
-            errors: [{ messageId: "unexpected", type: "BinaryExpression" }]
-        },
-        {
-            code: "if(+[a]) {}",
-            errors: [{ messageId: "unexpected", type: "UnaryExpression" }]
-        },
         {
             code: "if(+1) {}",
             errors: [{ messageId: "unexpected", type: "UnaryExpression" }]
@@ -397,7 +393,9 @@ ruleTester.run("no-constant-condition", rule, {
         // Boxed primitives are always truthy
         { code: "if(new Boolean(foo)) {}", errors: [{ messageId: "unexpected" }] },
         { code: "if(new String(foo)) {}", errors: [{ messageId: "unexpected" }] },
-        { code: "if(new Number(foo)) {}", errors: [{ messageId: "unexpected" }] }
+        { code: "if(new Number(foo)) {}", errors: [{ messageId: "unexpected" }] },
 
+        // Spreading a constant array
+        { code: "if(`${[...['a']]}`) {}", errors: [{ messageId: "unexpected" }] }
     ]
 });