Skip to content

Commit

Permalink
Update: treat all literals like boolean literal in no-constant-condit…
Browse files Browse the repository at this point in the history 
…ion (#13245)

* Fix: treet all literals like boolean literal in no-constant-condition

* fix unexpected short circuit

* handle bigin/regex on unsupported runtime

* fix bigint tests

* simplify literal truthiness

* handle regex literal in unsupported environments

* Update lib/rules/no-constant-condition.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
@Zzzen @mdjermanovic
Zzzen and mdjermanovic committed Oct 23, 2020
1 parent 289aa6f commit 603de04
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 23 deletions.
62 changes: 42 additions & 20 deletions lib/rules/no-constant-condition.js
View file
Expand Up @@ -9,9 +9,6 @@
// Helpers
//------------------------------------------------------------------------------

const EQUALITY_OPERATORS = ["===", "!==", "==", "!="];
const RELATIONAL_OPERATORS = [">", "<", ">=", "<=", "in", "instanceof"];

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -56,6 +53,35 @@ module.exports = {
// Helpers
//--------------------------------------------------------------------------

/**
* Returns literal's value converted to the Boolean type
* @param {ASTNode} node any `Literal` node
* @returns {boolean | null} `true` when node is truthy, `false` when node is falsy,
* `null` when it cannot be determined.
*/
function getBooleanValue(node) {
if (node.value === null) {

/*
* it might be a null literal or bigint/regex literal in unsupported environments .
* https://github.com/estree/estree/blob/14df8a024956ea289bd55b9c2226a1d5b8a473ee/es5.md#regexpliteral
* https://github.com/estree/estree/blob/14df8a024956ea289bd55b9c2226a1d5b8a473ee/es2020.md#bigintliteral
*/

if (node.raw === "null") {
return false;
}

// regex is always truthy
if (typeof node.regex === "object") {
return true;
}

return null;
}

return !!node.value;
}

/**
* Checks if a branch node of LogicalExpression short circuits the whole condition
Expand All @@ -66,15 +92,23 @@ module.exports = {
function isLogicalIdentity(node, operator) {
switch (node.type) {
case "Literal":
return (operator === "||" && node.value === true) ||
(operator === "&&" && node.value === false);
return (operator === "||" && getBooleanValue(node) === true) ||
(operator === "&&" && getBooleanValue(node) === false);

case "UnaryExpression":
return (operator === "&&" && node.operator === "void");

case "LogicalExpression":
return isLogicalIdentity(node.left, node.operator) ||
isLogicalIdentity(node.right, node.operator);

/*
* handles `a && false || b`
* `false` is an identity element of `&&` but not `||`
*/
return operator === node.operator &&
(
isLogicalIdentity(node.left, node.operator) ||
isLogicalIdentity(node.right, node.operator)
);

// no default
}
Expand Down Expand Up @@ -129,21 +163,9 @@ module.exports = {
const isLeftConstant = isConstant(node.left, inBooleanPosition);
const isRightConstant = isConstant(node.right, inBooleanPosition);
const isLeftShortCircuit = (isLeftConstant && isLogicalIdentity(node.left, node.operator));
const isRightShortCircuit = (isRightConstant && isLogicalIdentity(node.right, node.operator));
const isRightShortCircuit = (inBooleanPosition && isRightConstant && isLogicalIdentity(node.right, node.operator));

return (isLeftConstant && isRightConstant) ||
(

// in the case of an "OR", we need to know if the right constant value is truthy
node.operator === "||" &&
isRightConstant &&
node.right.value &&
(
!node.parent ||
node.parent.type !== "BinaryExpression" ||
!(EQUALITY_OPERATORS.includes(node.parent.operator) || RELATIONAL_OPERATORS.includes(node.parent.operator))
)
) ||
isLeftShortCircuit ||
isRightShortCircuit;
}
Expand Down
38 changes: 35 additions & 3 deletions tests/lib/rules/no-constant-condition.js
View file
Expand Up @@ -74,12 +74,11 @@ ruleTester.run("no-constant-condition", rule, {
"if(true && typeof abc==='string'){}",

// #11181, string literals
"if('str' || a){}",
"if('str1' && a){}",
"if(a && 'str'){}",
"if('str' || abc==='str'){}",

// #11306
"if ((foo || true) === 'baz') {}",
"if ((foo || 'bar') === 'baz') {}",
"if ((foo || 'bar') !== 'baz') {}",
"if ((foo || 'bar') == 'baz') {}",
Expand All @@ -90,6 +89,19 @@ ruleTester.run("no-constant-condition", rule, {
"if ((foo || 233) <= 666) {}",
"if ((key || 'k') in obj) {}",
"if ((foo || {}) instanceof obj) {}",
"if ((foo || 'bar' || 'bar') === 'bar');",
{
code: "if ((foo || 1n) === 'baz') {}",
parserOptions: { ecmaVersion: 11 }
},
{
code: "if (a && 0n || b);",
parserOptions: { ecmaVersion: 11 }
},
{
code: "if(1n && a){};",
parserOptions: { ecmaVersion: 11 }
},

// #12225
"if ('' + [y] === '' + [ty]) {}",
Expand Down Expand Up @@ -121,18 +133,22 @@ ruleTester.run("no-constant-condition", rule, {
{ code: "for(;`foo`;);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "for(;`foo${bar}`;);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "do{}while(true)", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "do{}while('1')", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "do{}while(0)", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "do{}while(t = -2)", errors: [{ messageId: "unexpected", type: "AssignmentExpression" }] },
{ code: "do{}while(``)", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "do{}while(`foo`)", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "do{}while(`foo${bar}`)", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "true ? 1 : 2;", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "1 ? 1 : 2;", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "q = 0 ? 1 : 2;", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "(q = 0) ? 1 : 2;", errors: [{ messageId: "unexpected", type: "AssignmentExpression" }] },
{ code: "`` ? 1 : 2;", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "`foo` ? 1 : 2;", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "`foo${bar}` ? 1 : 2;", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "if(-2);", errors: [{ messageId: "unexpected", type: "UnaryExpression" }] },
{ code: "if(true);", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(1);", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if({});", errors: [{ messageId: "unexpected", type: "ObjectExpression" }] },
{ code: "if(0 < 1);", errors: [{ messageId: "unexpected", type: "BinaryExpression" }] },
{ code: "if(0 || 1);", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
Expand All @@ -143,6 +159,7 @@ ruleTester.run("no-constant-condition", rule, {
{ code: "if(`${'bar'}`);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "if(`${'bar' + `foo`}`);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "if(`foo${false || true}`);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "if(`foo${0 || 1}`);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "if(`foo${bar}`);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "if(`${bar}foo`);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },

Expand All @@ -152,6 +169,7 @@ ruleTester.run("no-constant-condition", rule, {
{ code: "while(x = 1);", errors: [{ messageId: "unexpected", type: "AssignmentExpression" }] },
{ code: "while(function(){});", errors: [{ messageId: "unexpected", type: "FunctionExpression" }] },
{ code: "while(true);", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "while(1);", errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "while(() => {});", errors: [{ messageId: "unexpected", type: "ArrowFunctionExpression" }] },
{ code: "while(`foo`);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
{ code: "while(``);", errors: [{ messageId: "unexpected", type: "TemplateLiteral" }] },
Expand Down Expand Up @@ -180,12 +198,15 @@ ruleTester.run("no-constant-condition", rule, {
// #5693
{ code: "if(false && abc==='str'){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if(true || abc==='str'){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if(1 || abc==='str'){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if(abc==='str' || true){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if(abc==='str' || true || def ==='str'){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if(false || true){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if(typeof abc==='str' || true){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },

// #11181, string literals
{ code: "if('str' || a){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if('str' || abc==='str'){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if('str1' || 'str2'){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if('str1' && 'str2'){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
{ code: "if(abc==='str' || 'str'){}", errors: [{ messageId: "unexpected", type: "LogicalExpression" }] },
Expand Down Expand Up @@ -272,6 +293,17 @@ ruleTester.run("no-constant-condition", rule, {
{
code: "if ([,] + ''){}",
errors: [{ messageId: "unexpected", type: "BinaryExpression" }]
}
},

// #13238
{ code: "if(/foo/ui);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(0n);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(0b0n);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(0o0n);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(0x0n);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(0b1n);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(0o1n);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(0x1n);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "Literal" }] },
{ code: "if(0x1n || foo);", parserOptions: { ecmaVersion: 11 }, errors: [{ messageId: "unexpected", type: "LogicalExpression" }] }
]
});

0 comments on commit 603de04

Please sign in to comment.