Skip to content

Commit f043f30

Browse files
fix(eslint-plugin): [strict-boolean-expressions] consider assertion function argument a boolean context (#9074)
* [strict-boolean-expressions] Consider assertion function argument a boolean context * Apply suggestions from code review Co-authored-by: Josh Goldberg ✨ <[email protected]> * extract to function * gross tests * myAssert => assert * suggestions * codecov * no call signatures * 100% cov --------- Co-authored-by: Josh Goldberg ✨ <[email protected]>
1 parent 0108e9c commit f043f30

File tree

4 files changed

+756
-1
lines changed

4 files changed

+756
-1
lines changed

Diff for: packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx

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

2526
## Examples
2627

@@ -55,6 +56,11 @@ let obj = {};
5556
while (obj) {
5657
obj = getObj();
5758
}
59+
60+
// assertion functions without an `is` are boolean contexts.
61+
declare function assert(value: unknown): asserts value;
62+
let maybeString = Math.random() > 0.5 ? '' : undefined;
63+
assert(maybeString);
5864
```
5965

6066
</TabItem>

Diff for: packages/eslint-plugin/src/rules/strict-boolean-expressions.ts

+131-1
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ export default createRule<Options, MessageId>({
189189
WhileStatement: traverseTestExpression,
190190
'LogicalExpression[operator!="??"]': traverseLogicalExpression,
191191
'UnaryExpression[operator="!"]': traverseUnaryLogicalExpression,
192+
CallExpression: traverseCallExpression,
192193
};
193194

194195
type TestExpression =
@@ -232,10 +233,139 @@ export default createRule<Options, MessageId>({
232233
// left argument is always treated as a condition
233234
traverseNode(node.left, true);
234235
// if the logical expression is used for control flow,
235-
// then it's right argument is used for it's side effects only
236+
// then its right argument is used for its side effects only
236237
traverseNode(node.right, isCondition);
237238
}
238239

240+
function traverseCallExpression(node: TSESTree.CallExpression): void {
241+
const assertedArgument = findAssertedArgument(node);
242+
if (assertedArgument != null) {
243+
traverseNode(assertedArgument, true);
244+
}
245+
}
246+
247+
/**
248+
* Inspect a call expression to see if it's a call to an assertion function.
249+
* If it is, return the node of the argument that is asserted.
250+
*/
251+
function findAssertedArgument(
252+
node: TSESTree.CallExpression,
253+
): TSESTree.Expression | undefined {
254+
// If the call looks like `assert(expr1, expr2, ...c, d, e, f)`, then we can
255+
// only care if `expr1` or `expr2` is asserted, since anything that happens
256+
// within or after a spread argument is out of scope to reason about.
257+
const checkableArguments: TSESTree.Expression[] = [];
258+
for (const argument of node.arguments) {
259+
if (argument.type === AST_NODE_TYPES.SpreadElement) {
260+
break;
261+
}
262+
263+
checkableArguments.push(argument);
264+
}
265+
266+
// nothing to do
267+
if (checkableArguments.length === 0) {
268+
return undefined;
269+
}
270+
271+
// Game plan: we're going to check the type of the callee. If it has call
272+
// signatures and they _ALL_ agree that they assert on a parameter at the
273+
// _SAME_ position, we'll consider the argument in that position to be an
274+
// asserted argument.
275+
const calleeType = getConstrainedTypeAtLocation(services, node.callee);
276+
const callSignatures = tsutils.getCallSignaturesOfType(calleeType);
277+
278+
let assertedParameterIndex: number | undefined = undefined;
279+
for (const signature of callSignatures) {
280+
const declaration = signature.getDeclaration();
281+
const returnTypeAnnotation = declaration.type;
282+
283+
// Be sure we're dealing with a truthiness assertion function.
284+
if (
285+
!(
286+
returnTypeAnnotation != null &&
287+
ts.isTypePredicateNode(returnTypeAnnotation) &&
288+
// This eliminates things like `x is string` and `asserts x is T`
289+
// leaving us with just the `asserts x` cases.
290+
returnTypeAnnotation.type == null &&
291+
// I think this is redundant but, still, it needs to be true
292+
returnTypeAnnotation.assertsModifier != null
293+
)
294+
) {
295+
return undefined;
296+
}
297+
298+
const assertionTarget = returnTypeAnnotation.parameterName;
299+
if (assertionTarget.kind !== ts.SyntaxKind.Identifier) {
300+
// This can happen when asserting on `this`. Ignore!
301+
return undefined;
302+
}
303+
304+
// If the first parameter is `this`, skip it, so that our index matches
305+
// the index of the argument at the call site.
306+
const firstParameter = declaration.parameters.at(0);
307+
const nonThisParameters =
308+
firstParameter?.name.kind === ts.SyntaxKind.Identifier &&
309+
firstParameter.name.text === 'this'
310+
? declaration.parameters.slice(1)
311+
: declaration.parameters;
312+
313+
// Don't bother inspecting parameters past the number of
314+
// arguments we have at the call site.
315+
const checkableNonThisParameters = nonThisParameters.slice(
316+
0,
317+
checkableArguments.length,
318+
);
319+
320+
let assertedParameterIndexForThisSignature: number | undefined;
321+
for (const [index, parameter] of checkableNonThisParameters.entries()) {
322+
if (parameter.dotDotDotToken != null) {
323+
// Cannot assert a rest parameter, and can't have a rest parameter
324+
// before the asserted parameter. It's not only a TS error, it's
325+
// not something we can logically make sense of, so give up here.
326+
return undefined;
327+
}
328+
329+
if (parameter.name.kind !== ts.SyntaxKind.Identifier) {
330+
// Only identifiers are valid for assertion targets, so skip over
331+
// anything like `{ destructuring: parameter }: T`
332+
continue;
333+
}
334+
335+
// we've found a match between the "target"s in
336+
// `function asserts(target: T): asserts target;`
337+
if (parameter.name.text === assertionTarget.text) {
338+
assertedParameterIndexForThisSignature = index;
339+
break;
340+
}
341+
}
342+
343+
if (assertedParameterIndexForThisSignature == null) {
344+
// Didn't find an assertion target in this signature that could match
345+
// the call site.
346+
return undefined;
347+
}
348+
349+
if (
350+
assertedParameterIndex != null &&
351+
assertedParameterIndex !== assertedParameterIndexForThisSignature
352+
) {
353+
// The asserted parameter we found for this signature didn't match
354+
// previous signatures.
355+
return undefined;
356+
}
357+
358+
assertedParameterIndex = assertedParameterIndexForThisSignature;
359+
}
360+
361+
// Didn't find a unique assertion index.
362+
if (assertedParameterIndex == null) {
363+
return undefined;
364+
}
365+
366+
return checkableArguments[assertedParameterIndex];
367+
}
368+
239369
/**
240370
* Inspects any node.
241371
*

Diff for: packages/eslint-plugin/tests/docs-eslint-output-snapshots/strict-boolean-expressions.shot

+6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)