Skip to content

Commit

Permalink
make code more logical hopefully
Browse files Browse the repository at this point in the history
  • Loading branch information
phaux committed Aug 24, 2022
1 parent 14695ca commit fd8ffbb
Showing 1 changed file with 61 additions and 26 deletions.
87 changes: 61 additions & 26 deletions packages/eslint-plugin/src/rules/strict-boolean-expressions.ts
Expand Up @@ -166,16 +166,16 @@ export default util.createRule<Options, MessageId>({
});
}

const checkedNodes = new Set<TSESTree.Node>();
const traversedNodes = new Set<TSESTree.Node>();

return {
ConditionalExpression: checkTestExpression,
DoWhileStatement: checkTestExpression,
ForStatement: checkTestExpression,
IfStatement: checkTestExpression,
WhileStatement: checkTestExpression,
'LogicalExpression[operator!="??"]': checkNode,
'UnaryExpression[operator="!"]': checkUnaryLogicalExpression,
ConditionalExpression: traverseTestExpression,
DoWhileStatement: traverseTestExpression,
ForStatement: traverseTestExpression,
IfStatement: traverseTestExpression,
WhileStatement: traverseTestExpression,
'LogicalExpression[operator!="??"]': traverseLogicalExpression,
'UnaryExpression[operator="!"]': traverseUnaryLogicalExpression,
};

type TestExpression =
Expand All @@ -185,46 +185,81 @@ export default util.createRule<Options, MessageId>({
| TSESTree.IfStatement
| TSESTree.WhileStatement;

function checkTestExpression(node: TestExpression): void {
/**
* Inspects condition of a test expression. (`if`, `while`, `for`, etc.)
*/
function traverseTestExpression(node: TestExpression): void {
if (node.test == null) {
return;
}
checkNode(node.test, true);
traverseNode(node.test, true);
}

/**
* Inspects the argument of a unary logical expression (`!`).
*/
function traverseUnaryLogicalExpression(
node: TSESTree.UnaryExpression,
): void {
traverseNode(node.argument, true);
}

function checkUnaryLogicalExpression(node: TSESTree.UnaryExpression): void {
checkNode(node.argument, true);
/**
* Inspects the arguments of a logical expression (`&&`, `||`).
*
* If the logical expression is a descendant of a test expression,
* the `isCondition` flag should be set to true.
* Otherwise, if the logical expression is there on it's own,
* it's used for control flow and is not a condition itself.
*/
function traverseLogicalExpression(
node: TSESTree.LogicalExpression,
isCondition = false,
): void {
// left argument is always treated as a condition
traverseNode(node.left, true);
// if the logical expression is used for control flow,
// then it's right argument is used for it's side effects only
traverseNode(node.right, isCondition);
}

/**
* This function analyzes the type of a node and checks if it is allowed in a boolean context.
* It can recurse when checking nested logical operators, so that only the outermost operands are reported.
* The right operand of a logical expression is ignored unless it's a part of a test expression (if/while/ternary/etc).
* @param node The AST node to check.
* @param isTestExpr Whether the node is a descendant of a test expression.
* Inspects any node.
*
* If it's a logical expression then it recursively traverses its arguments.
* If it's any other kind of node then it's type is finally checked against the rule,
* unless `isCondition` flag is set to false, in which case
* it's assumed to be used for side effects only and is skipped.
*/
function checkNode(node: TSESTree.Node, isTestExpr = false): void {
function traverseNode(node: TSESTree.Node, isCondition: boolean): void {
// prevent checking the same node multiple times
if (checkedNodes.has(node)) {
if (traversedNodes.has(node)) {
return;
}
checkedNodes.add(node);
traversedNodes.add(node);

// for logical operator, we check its operands
if (
node.type === AST_NODE_TYPES.LogicalExpression &&
node.operator !== '??'
) {
checkNode(node.left, true);
traverseLogicalExpression(node, isCondition);
return;
}

// we ignore the right operand when not in a context of a test expression
// if the right operand is itself a logical expression, it will be checked separately
if (isTestExpr) {
checkNode(node.right, true);
}
// skip if node is not a condition
if (!isCondition) {
return;
}

checkNode(node);
}

/**
* This function does the actual type check on a node.
* It analyzes the type of a node and checks if it is allowed in a boolean context.
*/
function checkNode(node: TSESTree.Node): void {
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
const type = util.getConstrainedTypeAtLocation(typeChecker, tsNode);
const types = inspectVariantTypes(tsutils.unionTypeParts(type));
Expand Down

0 comments on commit fd8ffbb

Please sign in to comment.