Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Catch more cases in no-constant-condition #15613

Merged
merged 1 commit into from Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/rules/no-constant-condition.md
Expand Up @@ -42,6 +42,14 @@ if (new Boolean(x)) {
doSomethingAlways();
}

if (Boolean(1)) {
doSomethingAlways();
}

if (undefined) {
doSomethingUnfinished();
}

if (x ||= true) {
doSomethingAlways();
}
Expand Down
33 changes: 30 additions & 3 deletions lib/rules/no-constant-condition.js
Expand Up @@ -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.
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
* @returns {Bool} true when node's truthiness is constant
* @private
*/
Expand Down Expand Up @@ -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
}
Expand Down
26 changes: 24 additions & 2 deletions tests/lib/rules/no-constant-condition.js
Expand Up @@ -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" }] },
Expand Down Expand Up @@ -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" }] }
]
});