feat: Catch undefined and Boolean() in no-constant-condition (#15613)

captbaritone · web-flow · commit 8933fe7afcc7 · 2022-03-10T10:04:49.000+01:00
Identify `undefined` and `Boolean(somethingConstant)` as both being constant.
diff --git a/docs/rules/no-constant-condition.md b/docs/rules/no-constant-condition.md
@@ -42,6 +42,14 @@ if (new Boolean(x)) {
     doSomethingAlways();
 }
 
+if (Boolean(1)) {
+    doSomethingAlways();
+}
+
+if (undefined) {
+    doSomethingUnfinished();
+}
+
 if (x ||= true) {
     doSomethingAlways();
 }
diff --git a/lib/rules/no-constant-condition.js b/lib/rules/no-constant-condition.js
@@ -120,12 +120,30 @@ module.exports = {
             return false;
         }
 
+        /**
+         * Checks if an identifier is a reference to a global variable.
+         * @param {ASTNode} node An identifier node to check.
+         * @returns {boolean} `true` if the identifier is a reference to a global variable.
+         */
+        function isReferenceToGlobalVariable(node) {
+            const scope = context.getScope();
+            const reference = scope.references.find(ref => ref.identifier === node);
+
+            return Boolean(
+                reference &&
+                reference.resolved &&
+                reference.resolved.scope.type === "global" &&
+                reference.resolved.defs.length === 0
+            );
+        }
+
         /**
          * 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. When `false`, checks if -- for both string and
-         *  number -- if coerced to that type, the value will be constant.
+         * @param {boolean} inBooleanPosition `true` if checking the test of a
+         * condition. `false` 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
          */
@@ -215,6 +233,15 @@ module.exports = {
                     return isConstant(node.expressions[node.expressions.length - 1], inBooleanPosition);
                 case "SpreadElement":
                     return isConstant(node.argument, inBooleanPosition);
+                case "CallExpression":
+                    if (node.callee.type === "Identifier" && node.callee.name === "Boolean") {
+                        if (node.arguments.length === 0 || isConstant(node.arguments[0], true)) {
+                            return isReferenceToGlobalVariable(node.callee);
+                        }
+                    }
+                    return false;
+                case "Identifier":
+                    return node.name === "undefined" && isReferenceToGlobalVariable(node);
 
                 // no default
             }
diff --git a/tests/lib/rules/no-constant-condition.js b/tests/lib/rules/no-constant-condition.js
@@ -185,7 +185,17 @@ ruleTester.run("no-constant-condition", rule, {
         "if (`${[a]}`) {}",
         "if (+[a]) {}",
         "if (0 - [a]) {}",
-        "if (1 * [a]) {}"
+        "if (1 * [a]) {}",
+
+        // Boolean function
+        "if (Boolean(a)) {}",
+        "if (Boolean(...args)) {}",
+        "if (foo.Boolean(1)) {}",
+        "function foo(Boolean) { if (Boolean(1)) {} }",
+        "const Boolean = () => {}; if (Boolean(1)) {}",
+        { code: "if (Boolean()) {}", globals: { Boolean: "off" } },
+        "const undefined = 'lol'; if (undefined) {}",
+        { code: "if (undefined) {}", globals: { undefined: "off" } }
     ],
     invalid: [
         { code: "for(;true;);", errors: [{ messageId: "unexpected", type: "Literal" }] },
@@ -396,6 +406,18 @@ ruleTester.run("no-constant-condition", rule, {
         { code: "if(new Number(foo)) {}", errors: [{ messageId: "unexpected" }] },
 
         // Spreading a constant array
-        { code: "if(`${[...['a']]}`) {}", errors: [{ messageId: "unexpected" }] }
+        { code: "if(`${[...['a']]}`) {}", errors: [{ messageId: "unexpected" }] },
+
+        /*
+         * undefined is always falsy (except in old browsers that let you
+         * re-assign, but that's an abscure enough edge case to not worry about)
+         */
+        { code: "if (undefined) {}", errors: [{ messageId: "unexpected" }] },
+
+        // Coercion to boolean via Boolean function
+        { code: "if (Boolean(1)) {}", errors: [{ messageId: "unexpected" }] },
+        { code: "if (Boolean()) {}", errors: [{ messageId: "unexpected" }] },
+        { code: "if (Boolean([a])) {}", errors: [{ messageId: "unexpected" }] },
+        { code: "if (Boolean(1)) { function Boolean() {}}", errors: [{ messageId: "unexpected" }] }
     ]
 });

Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,14 @@ if (new Boolean(x)) {`
`42`	`42`	`doSomethingAlways();`
`43`	`43`	`}`
`44`	`44`
	`45`	`+if (Boolean(1)) {`
	`46`	`+ doSomethingAlways();`
	`47`	`+}`
	`48`	`+`
	`49`	`+if (undefined) {`
	`50`	`+ doSomethingUnfinished();`
	`51`	`+}`
	`52`	`+`
`45`	`53`	`if (x \|\|= true) {`
`46`	`54`	`doSomethingAlways();`
`47`	`55`	`}`