diff --git a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx index ab7653a5c0db..5ce9e5dd975d 100644 --- a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx +++ b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx @@ -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 @@ -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); ``` diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts index 01e15d3d88c5..f4b269ff7442 100644 --- a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -189,6 +189,7 @@ export default createRule({ WhileStatement: traverseTestExpression, 'LogicalExpression[operator!="??"]': traverseLogicalExpression, 'UnaryExpression[operator="!"]': traverseUnaryLogicalExpression, + CallExpression: traverseCallExpression, }; type TestExpression = @@ -232,10 +233,139 @@ export default createRule({ // 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. * diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/strict-boolean-expressions.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/strict-boolean-expressions.shot index b547a7e6e0c5..ac6b144bf4bd 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/strict-boolean-expressions.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/strict-boolean-expressions.shot @@ -34,6 +34,12 @@ while (obj) { ~~~ Unexpected object value in conditional. The condition is always true. 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); + ~~~~~~~~~~~ Unexpected nullable string value in conditional. Please handle the nullish/empty cases explicitly. " `; diff --git a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts index dd1bb0a92a38..640c9a212cd1 100644 --- a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -410,6 +410,139 @@ if (y) { }, ], }, + ` +declare function assert(a: number, b: unknown): asserts a; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, nullableString); + `, + ` +declare function assert(a: boolean, b: unknown): asserts b is string; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, nullableString); + `, + // Intentional TS error - cannot assert a parameter in a binding pattern. + ` +declare function assert(a: boolean, b: unknown): asserts b; +declare function assert(a: boolean, { b }: { b: unknown }): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, nullableString); + `, + ` +declare function assert(a: number, b: unknown): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(nullableString, boo); + `, + ` +declare function assert(a: number, b: unknown): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(...nullableString, nullableString); + `, + ` +declare function assert( + this: object, + a: number, + b?: unknown, + c?: unknown, +): asserts c; +declare const nullableString: string | null; +declare const foo: number; +const o: { assert: typeof assert } = { + assert, +}; +o.assert(foo, nullableString); + `, + { + code: ` +declare function assert(x: unknown): x is string; +declare const nullableString: string | null; +assert(nullableString); + `, + }, + { + code: ` +class ThisAsserter { + assertThis(this: unknown, arg2: unknown): asserts this {} +} + +declare const lol: string | number | unknown | null; + +const thisAsserter: ThisAsserter = new ThisAsserter(); +thisAsserter.assertThis(lol); + `, + }, + { + code: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; +function assert(...args: any[]): void; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, nullableString); + `, + }, + // Intentional TS error - A rest parameter must be last in a parameter list. + // This is just to test that we don't crash or falsely report. + ` +declare function assert(...a: boolean[], b: unknown): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, nullableString); + `, + // Intentional TS error - A type predicate cannot reference a rest parameter. + // This is just to test that we don't crash or falsely report. + ` +declare function assert(a: boolean, ...b: unknown[]): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, nullableString); + `, + // Intentional TS error - An assertion function must have a parameter to assert. + // This is just to test that we don't crash or falsely report. + ` +declare function assert(): asserts x; +declare const nullableString: string | null; +assert(nullableString); + `, + ` +function assert(one: unknown): asserts one; +function assert(one: unknown, two: unknown): asserts two; +function assert(...args: unknown[]) { + throw new Error('not implemented'); +} +declare const nullableString: string | null; +assert(nullableString); +assert('one', nullableString); + `, + // Intentional use of `any` to test a function call with no call signatures. + ` +declare const assert: any; +declare const nullableString: string | null; +assert(nullableString); + `, + // Coverage for absent "test expression". + // Ensure that no crash or false positive occurs + ` + for (let x = 0; ; x++) { + break; + } + `, ], invalid: [ @@ -2144,5 +2277,485 @@ if (x) { }, ], }, + { + code: ` +declare function assert(x: unknown): asserts x; +declare const nullableString: string | null; +assert(nullableString); + `, + output: null, + errors: [ + { + messageId: 'conditionErrorNullableString', + line: 4, + column: 8, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ` +declare function assert(x: unknown): asserts x; +declare const nullableString: string | null; +assert(nullableString != null); + `, + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ` +declare function assert(x: unknown): asserts x; +declare const nullableString: string | null; +assert(nullableString ?? ""); + `, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` +declare function assert(x: unknown): asserts x; +declare const nullableString: string | null; +assert(Boolean(nullableString)); + `, + }, + ], + }, + ], + }, + { + code: ` +declare function assert(a: number, b: unknown): asserts b; +declare const nullableString: string | null; +assert(foo, nullableString); + `, + output: null, + errors: [ + { + messageId: 'conditionErrorNullableString', + line: 4, + column: 13, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ` +declare function assert(a: number, b: unknown): asserts b; +declare const nullableString: string | null; +assert(foo, nullableString != null); + `, + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ` +declare function assert(a: number, b: unknown): asserts b; +declare const nullableString: string | null; +assert(foo, nullableString ?? ""); + `, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` +declare function assert(a: number, b: unknown): asserts b; +declare const nullableString: string | null; +assert(foo, Boolean(nullableString)); + `, + }, + ], + }, + ], + }, + { + code: ` +declare function assert(a: number, b: unknown): asserts b; +declare function assert(one: number, two: unknown): asserts two; +declare const nullableString: string | null; +assert(foo, nullableString); + `, + output: null, + errors: [ + { + messageId: 'conditionErrorNullableString', + line: 5, + column: 13, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ` +declare function assert(a: number, b: unknown): asserts b; +declare function assert(one: number, two: unknown): asserts two; +declare const nullableString: string | null; +assert(foo, nullableString != null); + `, + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ` +declare function assert(a: number, b: unknown): asserts b; +declare function assert(one: number, two: unknown): asserts two; +declare const nullableString: string | null; +assert(foo, nullableString ?? ""); + `, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` +declare function assert(a: number, b: unknown): asserts b; +declare function assert(one: number, two: unknown): asserts two; +declare const nullableString: string | null; +assert(foo, Boolean(nullableString)); + `, + }, + ], + }, + ], + }, + { + code: ` +declare function assert(this: object, a: number, b: unknown): asserts b; +declare const nullableString: string | null; +assert(foo, nullableString); + `, + output: null, + errors: [ + { + messageId: 'conditionErrorNullableString', + line: 4, + column: 13, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ` +declare function assert(this: object, a: number, b: unknown): asserts b; +declare const nullableString: string | null; +assert(foo, nullableString != null); + `, + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ` +declare function assert(this: object, a: number, b: unknown): asserts b; +declare const nullableString: string | null; +assert(foo, nullableString ?? ""); + `, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` +declare function assert(this: object, a: number, b: unknown): asserts b; +declare const nullableString: string | null; +assert(foo, Boolean(nullableString)); + `, + }, + ], + }, + ], + }, + { + code: ` +function asserts1(x: string | number | undefined): asserts x {} +function asserts2(x: string | number | undefined): asserts x {} + +const maybeString = Math.random() ? 'string'.slice() : undefined; + +const someAssert: typeof asserts1 | typeof asserts2 = + Math.random() > 0.5 ? asserts1 : asserts2; + +someAssert(maybeString); + `, + output: null, + errors: [ + { + messageId: 'conditionErrorNullableString', + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ` +function asserts1(x: string | number | undefined): asserts x {} +function asserts2(x: string | number | undefined): asserts x {} + +const maybeString = Math.random() ? 'string'.slice() : undefined; + +const someAssert: typeof asserts1 | typeof asserts2 = + Math.random() > 0.5 ? asserts1 : asserts2; + +someAssert(maybeString != null); + `, + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ` +function asserts1(x: string | number | undefined): asserts x {} +function asserts2(x: string | number | undefined): asserts x {} + +const maybeString = Math.random() ? 'string'.slice() : undefined; + +const someAssert: typeof asserts1 | typeof asserts2 = + Math.random() > 0.5 ? asserts1 : asserts2; + +someAssert(maybeString ?? ""); + `, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` +function asserts1(x: string | number | undefined): asserts x {} +function asserts2(x: string | number | undefined): asserts x {} + +const maybeString = Math.random() ? 'string'.slice() : undefined; + +const someAssert: typeof asserts1 | typeof asserts2 = + Math.random() > 0.5 ? asserts1 : asserts2; + +someAssert(Boolean(maybeString)); + `, + }, + ], + }, + ], + }, + { + // The implementation signature doesn't count towards the call signatures + code: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, nullableString); + `, + output: null, + errors: [ + { + messageId: 'conditionErrorNullableString', + line: 18, + column: 18, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, nullableString != null); + `, + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, nullableString ?? ""); + `, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, Boolean(nullableString)); + `, + }, + ], + }, + ], + }, + { + // The implementation signature doesn't count towards the call signatures + code: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; +function assert(a: any, two: unknown, ...rest: any[]): asserts two; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, nullableString, 'more', 'args', 'afterwards'); + `, + output: null, + errors: [ + { + messageId: 'conditionErrorNullableString', + line: 19, + column: 18, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; +function assert(a: any, two: unknown, ...rest: any[]): asserts two; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, nullableString != null, 'more', 'args', 'afterwards'); + `, + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; +function assert(a: any, two: unknown, ...rest: any[]): asserts two; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, nullableString ?? "", 'more', 'args', 'afterwards'); + `, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` +function assert(this: object, a: number, b: unknown): asserts b; +function assert(a: bigint, b: unknown): asserts b; +function assert(this: object, a: string, two: string): asserts two; +function assert( + this: object, + a: string, + assertee: string, + c: bigint, + d: object, +): asserts assertee; +function assert(a: any, two: unknown, ...rest: any[]): asserts two; + +function assert(...args: any[]) { + throw new Error('lol'); +} + +declare const nullableString: string | null; +assert(3 as any, Boolean(nullableString), 'more', 'args', 'afterwards'); + `, + }, + ], + }, + ], + }, + { + code: ` +declare function assert(a: boolean, b: unknown): asserts b; +declare function assert({ a }: { a: boolean }, b: unknown): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, nullableString); + `, + output: null, + errors: [ + { + line: 6, + messageId: 'conditionErrorNullableString', + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ` +declare function assert(a: boolean, b: unknown): asserts b; +declare function assert({ a }: { a: boolean }, b: unknown): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, nullableString != null); + `, + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ` +declare function assert(a: boolean, b: unknown): asserts b; +declare function assert({ a }: { a: boolean }, b: unknown): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, nullableString ?? ""); + `, + }, + + { + messageId: 'conditionFixCastBoolean', + output: ` +declare function assert(a: boolean, b: unknown): asserts b; +declare function assert({ a }: { a: boolean }, b: unknown): asserts b; +declare const nullableString: string | null; +declare const boo: boolean; +assert(boo, Boolean(nullableString)); + `, + }, + ], + }, + ], + }, ], });