From 8f676dfae18f252649ff4ec6cd9f10b1f0c6fa00 Mon Sep 17 00:00:00 2001 From: Retsam Date: Fri, 7 Feb 2020 15:33:40 -0500 Subject: [PATCH 1/2] feat(eslint-plugin): [no-unnesc-cond] check array predicates by default --- .../docs/rules/no-unnecessary-condition.md | 21 ++++++--------- .../src/rules/no-unnecessary-condition.ts | 12 +-------- .../rules/no-unnecessary-condition.test.ts | 26 ++----------------- 3 files changed, 11 insertions(+), 48 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md b/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md index f2bea6f4fd8..5b24b4cc965 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md @@ -28,6 +28,12 @@ function bar(arg: string) { // arg can never be nullish, so ?. is unnecessary return arg?.length; } + +// Checks array predicate return types, where possible +[ + [1, 2], + [3, 4], +].filter(t => t); // number[] is always truthy ``` Examples of **correct** code for this rule: @@ -50,6 +56,8 @@ function bar(arg?: string | null) { // Necessary, since arg might be nullish return arg?.length; } + +[0, 1, 2, 3].filter(t => t); // number can be truthy or falsy ``` ## Options @@ -74,19 +82,6 @@ for (; true; ) {} do {} while (true); ``` -- `checkArrayPredicates` (default: `false`) - if set checks that the return value from certain array method callbacks (`filter`, `find`, `some`, `every`) is necessarily conditional. - -```ts -// Valid: numbers can be truthy or falsy. -[0, 1, 2, 3].filter(t => t); - -// Invalid: arrays are always falsy. -[ - [1, 2], - [3, 4], -].filter(t => t); -``` - ## When Not To Use It The main downside to using this rule is the need for type information. diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 22a9fed5ba6..0f3fb3d4b92 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -61,7 +61,6 @@ export type Options = [ { allowConstantLoopConditions?: boolean; ignoreRhs?: boolean; - checkArrayPredicates?: boolean; }, ]; @@ -96,9 +95,6 @@ export default createRule({ ignoreRhs: { type: 'boolean', }, - checkArrayPredicates: { - type: 'boolean', - }, }, additionalProperties: false, }, @@ -125,13 +121,9 @@ export default createRule({ { allowConstantLoopConditions: false, ignoreRhs: false, - checkArrayPredicates: false, }, ], - create( - context, - [{ allowConstantLoopConditions, checkArrayPredicates, ignoreRhs }], - ) { + create(context, [{ allowConstantLoopConditions, ignoreRhs }]) { const service = getParserServices(context); const checker = service.program.getTypeChecker(); const sourceCode = context.getSourceCode(); @@ -299,8 +291,6 @@ export default createRule({ function shouldCheckCallback(node: TSESTree.CallExpression): boolean { const { callee } = node; return ( - // option is on - !!checkArrayPredicates && // looks like `something.filter` or `something.find` callee.type === AST_NODE_TYPES.MemberExpression && callee.property.type === AST_NODE_TYPES.Identifier && diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 0728b9202b9..3e2f33a6935 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -91,20 +91,7 @@ function test(a: string) { /** * Predicate functions **/ - // valid, with the flag off ` -[1,3,5].filter(() => true); -[1,2,3].find(() => false); -function truthy() { - return []; -} -function falsy() {} -[1,3,5].filter(truthy); -[1,2,3].find(falsy); -`, - { - options: [{ checkArrayPredicates: true }], - code: ` // with literal arrow function [0,1,2].filter(x => x); @@ -117,13 +104,9 @@ function length(x: string) { // with non-literal array function nonEmptyStrings(x: string[]) { return x.filter(length); -} -`, - }, +}`, // Ignores non-array methods of the same name - { - options: [{ checkArrayPredicates: true }], - code: ` + ` const notArray = { filter: (func: () => boolean) => func(), find: (func: () => boolean) => func(), @@ -131,8 +114,6 @@ const notArray = { notArray.filter(() => true); notArray.find(() => true); `, - }, - // Nullish coalescing operator ` function test(a: string | null) { @@ -336,7 +317,6 @@ function test(a: never) { // Predicate functions { - options: [{ checkArrayPredicates: true }], code: ` [1,3,5].filter(() => true); [1,2,3].find(() => { return false; }); @@ -363,7 +343,6 @@ function nothing3(x: [string, string]) { ], }, { - options: [{ checkArrayPredicates: true }], code: ` function truthy() { return []; @@ -380,7 +359,6 @@ function falsy() {} // Supports generics // TODO: fix this // { - // options: [{ checkArrayPredicates: true }], // code: ` // const isTruthy = (t: T) => T; // // Valid: numbers can be truthy or falsy (0). From 90404234a7a70dad31b39e1d33352a3632527a1c Mon Sep 17 00:00:00 2001 From: Retsam Date: Fri, 7 Feb 2020 15:42:52 -0500 Subject: [PATCH 2/2] chore(eslint-plugin): slight cleanup of array predicate logic --- .../eslint-plugin/src/rules/no-unnecessary-condition.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 0f3fb3d4b92..c6157428b80 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -288,7 +288,7 @@ export default createRule({ 'some', 'every', ]); - function shouldCheckCallback(node: TSESTree.CallExpression): boolean { + function isArrayPredicateFunction(node: TSESTree.CallExpression): boolean { const { callee } = node; return ( // looks like `something.filter` or `something.find` @@ -300,10 +300,9 @@ export default createRule({ ); } function checkCallExpression(node: TSESTree.CallExpression): void { - const { - arguments: [callback], - } = node; - if (callback && shouldCheckCallback(node)) { + // If this is something like arr.filter(x => /*condition*/), check `condition` + if (isArrayPredicateFunction(node) && node.arguments.length) { + const callback = node.arguments[0]!; // Inline defined functions if ( (callback.type === AST_NODE_TYPES.ArrowFunctionExpression ||