Skip to content

fix(eslint-plugin): [strict-boolean-expressions] consider assertion function argument a boolean context #9074

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

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The following nodes are considered boolean expressions and their type is checked
- Operands of logical binary operators (`lhs || rhs` and `lhs && rhs`).
- Right-hand side operand is ignored when it's not a descendant of another boolean expression.
This is to allow usage of boolean operators for their short-circuiting behavior.
- Asserted argument of an assertion function (`assert(arg)`).

## Examples

Expand Down Expand Up @@ -55,6 +56,11 @@ let obj = {};
while (obj) {
obj = getObj();
}

// assertion functions without an `is` are boolean contexts.
declare function assert(value: unknown): asserts value;
let maybeString = Math.random() > 0.5 ? '' : undefined;
assert(maybeString);
```

</TabItem>
Expand Down
132 changes: 131 additions & 1 deletion packages/eslint-plugin/src/rules/strict-boolean-expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export default createRule<Options, MessageId>({
WhileStatement: traverseTestExpression,
'LogicalExpression[operator!="??"]': traverseLogicalExpression,
'UnaryExpression[operator="!"]': traverseUnaryLogicalExpression,
CallExpression: traverseCallExpression,
};

type TestExpression =
Expand Down Expand Up @@ -232,10 +233,139 @@ export default createRule<Options, MessageId>({
// 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
// then its right argument is used for its side effects only
traverseNode(node.right, isCondition);
}

function traverseCallExpression(node: TSESTree.CallExpression): void {
const assertedArgument = findAssertedArgument(node);
if (assertedArgument != null) {
traverseNode(assertedArgument, true);
}
}

/**
* Inspect a call expression to see if it's a call to an assertion function.
* If it is, return the node of the argument that is asserted.
*/
function findAssertedArgument(
node: TSESTree.CallExpression,
): TSESTree.Expression | undefined {
// If the call looks like `assert(expr1, expr2, ...c, d, e, f)`, then we can
// only care if `expr1` or `expr2` is asserted, since anything that happens
// within or after a spread argument is out of scope to reason about.
const checkableArguments: TSESTree.Expression[] = [];
for (const argument of node.arguments) {
if (argument.type === AST_NODE_TYPES.SpreadElement) {
break;
}

checkableArguments.push(argument);
}

// nothing to do
if (checkableArguments.length === 0) {
return undefined;
}

// Game plan: we're going to check the type of the callee. If it has call
// signatures and they _ALL_ agree that they assert on a parameter at the
// _SAME_ position, we'll consider the argument in that position to be an
// asserted argument.
const calleeType = getConstrainedTypeAtLocation(services, node.callee);
const callSignatures = tsutils.getCallSignaturesOfType(calleeType);

let assertedParameterIndex: number | undefined = undefined;
for (const signature of callSignatures) {
const declaration = signature.getDeclaration();
const returnTypeAnnotation = declaration.type;

// Be sure we're dealing with a truthiness assertion function.
if (
!(
returnTypeAnnotation != null &&
ts.isTypePredicateNode(returnTypeAnnotation) &&
// This eliminates things like `x is string` and `asserts x is T`
// leaving us with just the `asserts x` cases.
returnTypeAnnotation.type == null &&
// I think this is redundant but, still, it needs to be true
returnTypeAnnotation.assertsModifier != null
)
) {
return undefined;
}

const assertionTarget = returnTypeAnnotation.parameterName;
if (assertionTarget.kind !== ts.SyntaxKind.Identifier) {
// This can happen when asserting on `this`. Ignore!
return undefined;
}

// If the first parameter is `this`, skip it, so that our index matches
// the index of the argument at the call site.
const firstParameter = declaration.parameters.at(0);
const nonThisParameters =
firstParameter?.name.kind === ts.SyntaxKind.Identifier &&
firstParameter.name.text === 'this'
? declaration.parameters.slice(1)
: declaration.parameters;

// Don't bother inspecting parameters past the number of
// arguments we have at the call site.
const checkableNonThisParameters = nonThisParameters.slice(
0,
checkableArguments.length,
);

let assertedParameterIndexForThisSignature: number | undefined;
for (const [index, parameter] of checkableNonThisParameters.entries()) {
if (parameter.dotDotDotToken != null) {
// Cannot assert a rest parameter, and can't have a rest parameter
// before the asserted parameter. It's not only a TS error, it's
// not something we can logically make sense of, so give up here.
return undefined;
}

if (parameter.name.kind !== ts.SyntaxKind.Identifier) {
// Only identifiers are valid for assertion targets, so skip over
// anything like `{ destructuring: parameter }: T`
continue;
}

// we've found a match between the "target"s in
// `function asserts(target: T): asserts target;`
if (parameter.name.text === assertionTarget.text) {
assertedParameterIndexForThisSignature = index;
break;
}
}

if (assertedParameterIndexForThisSignature == null) {
// Didn't find an assertion target in this signature that could match
// the call site.
return undefined;
}

if (
assertedParameterIndex != null &&
assertedParameterIndex !== assertedParameterIndexForThisSignature
) {
// The asserted parameter we found for this signature didn't match
// previous signatures.
return undefined;
}

assertedParameterIndex = assertedParameterIndexForThisSignature;
}

// Didn't find a unique assertion index.
if (assertedParameterIndex == null) {
return undefined;
}

return checkableArguments[assertedParameterIndex];
}

/**
* Inspects any node.
*
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading