diff --git a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx index 94e647872a4f..c0e71ec42c95 100644 --- a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx +++ b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.mdx @@ -22,6 +22,7 @@ The following nodes are considered boolean expressions and their type is checked - 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)`). +- Return type of array predicate functions such as `filter()`, `some()`, etc. ## Examples @@ -61,6 +62,9 @@ while (obj) { declare function assert(value: unknown): asserts value; let maybeString = Math.random() > 0.5 ? '' : undefined; assert(maybeString); + +// array predicates' return types are boolean contexts. +['one', null].filter(x => x); ``` diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts index 395cf8524f8d..216929d62630 100644 --- a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -3,7 +3,7 @@ import type { TSESTree, } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES, ASTUtils } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; @@ -12,7 +12,10 @@ import { getConstrainedTypeAtLocation, getParserServices, getWrappingFixer, + isArrayMethodCallWithPredicate, + isParenlessArrowFunction, isTypeArrayTypeOrUnionOfArrayTypes, + nullThrows, } from '../util'; import { findTruthinessAssertedArgument } from '../util/assertionFunctionUtils'; @@ -53,7 +56,10 @@ export type MessageId = | 'conditionFixDefaultEmptyString' | 'conditionFixDefaultFalse' | 'conditionFixDefaultZero' - | 'noStrictNullCheck'; + | 'explicitBooleanReturnType' + | 'noStrictNullCheck' + | 'predicateCannotBeAsync' + | 'predicateReturnsNonBoolean'; export default createRule({ name: 'strict-boolean-expressions', @@ -122,8 +128,13 @@ export default createRule({ 'Explicitly treat nullish value the same as false (`value ?? false`)', conditionFixDefaultZero: 'Explicitly treat nullish value the same as 0 (`value ?? 0`)', + explicitBooleanReturnType: + 'Add an explicit `boolean` return type annotation.', noStrictNullCheck: 'This rule requires the `strictNullChecks` compiler option to be turned on to function correctly.', + predicateCannotBeAsync: + "Predicate function should not be 'async'; expected a boolean return type.", + predicateReturnsNonBoolean: 'Predicate function should return a boolean.', }, schema: [ { @@ -275,6 +286,104 @@ export default createRule({ if (assertedArgument != null) { traverseNode(assertedArgument, true); } + if (isArrayMethodCallWithPredicate(context, services, node)) { + const predicate = node.arguments.at(0); + + if (predicate) { + checkArrayMethodCallPredicate(predicate); + } + } + } + + /** + * Dedicated function to check array method predicate calls. Reports predicate + * arguments that don't return a boolean value. + * + * Ignores the `allow*` options and requires a boolean value. + */ + function checkArrayMethodCallPredicate( + predicateNode: TSESTree.CallExpressionArgument, + ): void { + const isFunctionExpression = ASTUtils.isFunction(predicateNode); + + // custom message for accidental `async` function expressions + if (isFunctionExpression && predicateNode.async) { + return context.report({ + node: predicateNode, + messageId: 'predicateCannotBeAsync', + }); + } + + const returnTypes = services + .getTypeAtLocation(predicateNode) + .getCallSignatures() + .map(signature => { + const type = signature.getReturnType(); + + if (tsutils.isTypeParameter(type)) { + return checker.getBaseConstraintOfType(type) ?? type; + } + + return type; + }); + + if (returnTypes.every(returnType => isBooleanType(returnType))) { + return; + } + + const canFix = isFunctionExpression && !predicateNode.returnType; + + return context.report({ + node: predicateNode, + messageId: 'predicateReturnsNonBoolean', + suggest: canFix + ? [ + { + messageId: 'explicitBooleanReturnType', + fix: fixer => { + if ( + predicateNode.type === + AST_NODE_TYPES.ArrowFunctionExpression && + isParenlessArrowFunction(predicateNode, context.sourceCode) + ) { + return [ + fixer.insertTextBefore(predicateNode.params[0], '('), + fixer.insertTextAfter( + predicateNode.params[0], + '): boolean', + ), + ]; + } + + if (predicateNode.params.length === 0) { + const closingBracket = nullThrows( + context.sourceCode.getFirstToken( + predicateNode, + token => token.value === ')', + ), + 'function expression has to have a closing parenthesis.', + ); + + return fixer.insertTextAfter(closingBracket, ': boolean'); + } + + const lastClosingParenthesis = nullThrows( + context.sourceCode.getTokenAfter( + predicateNode.params[predicateNode.params.length - 1], + token => token.value === ')', + ), + 'function expression has to have a closing parenthesis.', + ); + + return fixer.insertTextAfter( + lastClosingParenthesis, + ': boolean', + ); + }, + }, + ] + : null, + }); } /** @@ -1007,11 +1116,13 @@ function isArrayLengthExpression( function isBrandedBoolean(type: ts.Type): boolean { return ( type.isIntersection() && - type.types.some(childType => - tsutils.isTypeFlagSet( - childType, - ts.TypeFlags.BooleanLiteral | ts.TypeFlags.Boolean, - ), - ) + type.types.some(childType => isBooleanType(childType)) + ); +} + +function isBooleanType(expressionType: ts.Type): boolean { + return tsutils.isTypeFlagSet( + expressionType, + ts.TypeFlags.Boolean | ts.TypeFlags.BooleanLiteral, ); } 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 ba1353e67dcc..25b351d15360 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 @@ -41,6 +41,10 @@ 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. + +// array predicates' return types are boolean contexts. +['one', null].filter(x => x); + ~~~~~~ Predicate function should return a boolean. " `; 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 d705d6402b2d..4b7d772c8cc8 100644 --- a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -521,6 +521,74 @@ assert(nullableString); break; } `, + ` +[true, false].some(function (x) { + return x; +}); + `, + ` +[true, false].some(function check(x) { + return x; +}); + `, + ` +[true, false].some(x => { + return x; +}); + `, + ` +[1, null].filter(function (x) { + return x != null; +}); + `, + ` +['one', 'two', ''].filter(function (x) { + return !!x; +}); + `, + ` +['one', 'two', ''].filter(function (x): boolean { + return !!x; +}); + `, + ` +['one', 'two', ''].filter(function (x): boolean { + if (x) { + return true; + } +}); + `, + ` +['one', 'two', ''].filter(function (x): boolean { + if (x) { + return true; + } + + throw new Error('oops'); +}); + `, + ` +declare const predicate: (string) => boolean; +['one', 'two', ''].filter(predicate); + `, + ` +declare function notNullish(x: T): x is NonNullable; +['one', null].filter(notNullish); + `, + ` +declare function predicate(x: string | null): x is string; +['one', null].filter(predicate); + `, + ` +declare function predicate(x: string | null): T; +['one', null].filter(predicate); + `, + ` +declare function f(x: number): boolean; +declare function f(x: string | null): boolean; + +[35].filter(f); + `, ], invalid: [ @@ -2910,5 +2978,358 @@ assert(Boolean(nullableString)); }, ], }, + { + code: ` +['one', 'two', ''].find(x => { + return x; +}); + `, + errors: [ + { + column: 25, + endColumn: 2, + endLine: 4, + line: 2, + messageId: 'predicateReturnsNonBoolean', + suggestions: [ + { + messageId: 'explicitBooleanReturnType', + output: ` +['one', 'two', ''].find((x): boolean => { + return x; +}); + `, + }, + ], + }, + ], + }, + { + code: ` +['one', 'two', ''].find(x => { + return; +}); + `, + errors: [ + { + column: 25, + endColumn: 2, + endLine: 4, + line: 2, + messageId: 'predicateReturnsNonBoolean', + suggestions: [ + { + messageId: 'explicitBooleanReturnType', + output: ` +['one', 'two', ''].find((x): boolean => { + return; +}); + `, + }, + ], + }, + ], + }, + { + code: ` +['one', 'two', ''].findLast(x => { + return undefined; +}); + `, + errors: [ + { + column: 29, + endColumn: 2, + endLine: 4, + line: 2, + messageId: 'predicateReturnsNonBoolean', + suggestions: [ + { + messageId: 'explicitBooleanReturnType', + output: ` +['one', 'two', ''].findLast((x): boolean => { + return undefined; +}); + `, + }, + ], + }, + ], + }, + { + code: ` +['one', 'two', ''].find(x => { + if (x) { + return true; + } +}); + `, + errors: [ + { + column: 25, + endColumn: 2, + endLine: 6, + line: 2, + messageId: 'predicateReturnsNonBoolean', + suggestions: [ + { + messageId: 'explicitBooleanReturnType', + output: ` +['one', 'two', ''].find((x): boolean => { + if (x) { + return true; + } +}); + `, + }, + ], + }, + ], + }, + { + code: ` +const predicate = (x: string) => { + if (x) { + return true; + } +}; + +['one', 'two', ''].find(predicate); + `, + errors: [ + { + column: 25, + endColumn: 34, + endLine: 8, + line: 8, + messageId: 'predicateReturnsNonBoolean', + }, + ], + }, + { + code: ` +[1, null].every(async x => { + return x != null; +}); + `, + errors: [ + { + column: 17, + data: { + type: 'Promise', + }, + endColumn: 2, + endLine: 4, + line: 2, + messageId: 'predicateCannotBeAsync', + }, + ], + }, + { + code: ` +const predicate = async x => { + return x != null; +}; + +[1, null].every(predicate); + `, + errors: [ + { + column: 17, + endColumn: 26, + endLine: 6, + line: 6, + messageId: 'predicateReturnsNonBoolean', + }, + ], + }, + { + code: ` +[1, null].every((x): boolean | number => { + return x != null; +}); + `, + errors: [ + { + column: 17, + endColumn: 2, + endLine: 4, + line: 2, + messageId: 'predicateReturnsNonBoolean', + }, + ], + }, + { + code: ` +[1, null].every((x): boolean | undefined => { + return x != null; +}); + `, + errors: [ + { + column: 17, + endColumn: 2, + endLine: 4, + line: 2, + messageId: 'predicateReturnsNonBoolean', + }, + ], + }, + // various cases for the suggestion fix + { + code: ` +[1, null].every((x, i) => {}); + `, + errors: [ + { + column: 17, + endColumn: 29, + endLine: 2, + line: 2, + messageId: 'predicateReturnsNonBoolean', + suggestions: [ + { + messageId: 'explicitBooleanReturnType', + output: ` +[1, null].every((x, i): boolean => {}); + `, + }, + ], + }, + ], + }, + { + code: ` +[() => {}, null].every((x: () => void) => {}); + `, + errors: [ + { + column: 24, + endColumn: 45, + endLine: 2, + line: 2, + messageId: 'predicateReturnsNonBoolean', + suggestions: [ + { + messageId: 'explicitBooleanReturnType', + output: ` +[() => {}, null].every((x: () => void): boolean => {}); + `, + }, + ], + }, + ], + }, + { + code: ` +[() => {}, null].every(function (x: () => void) {}); + `, + errors: [ + { + column: 24, + endColumn: 51, + endLine: 2, + line: 2, + messageId: 'predicateReturnsNonBoolean', + suggestions: [ + { + messageId: 'explicitBooleanReturnType', + output: ` +[() => {}, null].every(function (x: () => void): boolean {}); + `, + }, + ], + }, + ], + }, + { + code: ` +[() => {}, null].every(() => {}); + `, + errors: [ + { + column: 24, + endColumn: 32, + endLine: 2, + line: 2, + messageId: 'predicateReturnsNonBoolean', + suggestions: [ + { + messageId: 'explicitBooleanReturnType', + output: ` +[() => {}, null].every((): boolean => {}); + `, + }, + ], + }, + ], + }, + // function overloading + { + code: ` +declare function f(x: number): string; +declare function f(x: string | null): boolean; + +[35].filter(f); + `, + errors: [ + { + column: 13, + endColumn: 14, + endLine: 5, + line: 5, + messageId: 'predicateReturnsNonBoolean', + }, + ], + }, + { + code: ` +declare function f(x: number): string; +declare function f(x: number | boolean): boolean; +declare function f(x: string | null): boolean; + +[35].filter(f); + `, + errors: [ + { + column: 13, + endColumn: 14, + endLine: 6, + line: 6, + messageId: 'predicateReturnsNonBoolean', + }, + ], + }, + // type constraints + { + code: ` +function foo(x: number): T {} +[1, null].every(foo); + `, + errors: [ + { + column: 17, + endColumn: 20, + endLine: 3, + line: 3, + messageId: 'predicateReturnsNonBoolean', + }, + ], + }, + { + code: ` +function foo(x: number): T {} +[1, null].every(foo); + `, + errors: [ + { + column: 17, + endColumn: 20, + endLine: 3, + line: 3, + messageId: 'predicateReturnsNonBoolean', + }, + ], + }, ], });