From 7f8a1c2d51bd722f27de8123499c4889c4423158 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Mar 2022 13:41:13 -0500 Subject: [PATCH 1/5] feat(eslint-plugin): [no-misused-promises] add opt-opt options within checksVoidReturns --- .../docs/rules/no-misused-promises.md | 34 ++++- .../src/rules/no-misused-promises.ts | 85 +++++++++++-- .../tests/rules/no-misused-promises.test.ts | 120 ++++++++++++++++++ 3 files changed, 223 insertions(+), 16 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-promises.md b/packages/eslint-plugin/docs/rules/no-misused-promises.md index a50cf28108c..cdacafd4f9f 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-promises.md +++ b/packages/eslint-plugin/docs/rules/no-misused-promises.md @@ -19,10 +19,18 @@ Both are enabled by default type Options = [ { checksConditionals?: boolean; - checksVoidReturn?: boolean; + checksVoidReturn?: boolean | ChecksVoidReturnOptions; }, ]; +interface ChecksVoidReturnOptions { + arguments?: boolean; + attributes?: boolean; + properties?: boolean; + returns?: boolean; + variables?: boolean; +} + const defaultOptions: Options = [ { checksConditionals: true, @@ -31,7 +39,21 @@ const defaultOptions: Options = [ ]; ``` -If you don't want functions that return promises where a void return is +If you don't want to check conditionals, you can configure the rule +like this: + +```json +{ + "@typescript-eslint/no-misused-promises": [ + "error", + { + "checksConditionals": false + } + ] +} +``` + +Likewise, if you don't want functions that return promises where a void return is expected to be checked, your configuration will look like this: ```json @@ -45,15 +67,17 @@ expected to be checked, your configuration will look like this: } ``` -Likewise, if you don't want to check conditionals, you can configure the rule -like this: +You can disable selective parts of the `checksVoidReturn` option by providing an +object that disables specific checks: ```json { "@typescript-eslint/no-misused-promises": [ "error", { - "checksConditionals": false + "checksVoidReturn": { + "arguments": false + } } ] } diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 7f08402334c..77e963ddda1 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -7,10 +7,18 @@ import * as util from '../util'; type Options = [ { checksConditionals?: boolean; - checksVoidReturn?: boolean; + checksVoidReturn?: boolean | ChecksVoidReturnOptions; }, ]; +interface ChecksVoidReturnOptions { + arguments?: boolean; + attributes?: boolean; + properties?: boolean; + returns?: boolean; + variables?: boolean; +} + type MessageId = | 'conditional' | 'voidReturnArgument' @@ -19,6 +27,34 @@ type MessageId = | 'voidReturnReturnValue' | 'voidReturnAttribute'; +function parseChecksVoidReturn( + checksVoidReturn: boolean | ChecksVoidReturnOptions | undefined, +): ChecksVoidReturnOptions | false { + switch (checksVoidReturn) { + case false: + return false; + + case true: + case undefined: + return { + arguments: true, + attributes: true, + properties: true, + returns: true, + variables: true, + }; + + default: + return { + arguments: checksVoidReturn.arguments ?? true, + attributes: checksVoidReturn.attributes ?? true, + properties: checksVoidReturn.properties ?? true, + returns: checksVoidReturn.returns ?? true, + variables: checksVoidReturn.variables ?? true, + }; + } +} + export default util.createRule({ name: 'no-misused-promises', meta: { @@ -48,7 +84,20 @@ export default util.createRule({ type: 'boolean', }, checksVoidReturn: { - type: 'boolean', + oneOf: [ + { type: 'boolean' }, + { + additionalProperties: false, + properties: { + arguments: { type: 'boolean' }, + attributes: { type: 'boolean' }, + properties: { type: 'boolean' }, + returns: { type: 'boolean' }, + variables: { type: 'boolean' }, + }, + type: 'object', + }, + ], }, }, }, @@ -80,15 +129,29 @@ export default util.createRule({ WhileStatement: checkTestConditional, }; - const voidReturnChecks: TSESLint.RuleListener = { - CallExpression: checkArguments, - NewExpression: checkArguments, - AssignmentExpression: checkAssignment, - VariableDeclarator: checkVariableDeclaration, - Property: checkProperty, - ReturnStatement: checkReturnStatement, - JSXAttribute: checkJSXAttribute, - }; + checksVoidReturn = parseChecksVoidReturn(checksVoidReturn); + + const voidReturnChecks: TSESLint.RuleListener = checksVoidReturn + ? { + ...(checksVoidReturn.arguments && { + CallExpression: checkArguments, + NewExpression: checkArguments, + }), + ...(checksVoidReturn.attributes && { + JSXAttribute: checkJSXAttribute, + }), + ...(checksVoidReturn.properties && { + Property: checkProperty, + }), + ...(checksVoidReturn.returns && { + ReturnStatement: checkReturnStatement, + }), + ...(checksVoidReturn.variables && { + AssignmentExpression: checkAssignment, + VariableDeclarator: checkVariableDeclaration, + }), + } + : {}; function checkTestConditional(node: { test: TSESTree.Expression | null; diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts index 961a21d4c89..690f76cc1ce 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -252,6 +252,51 @@ const Component: any = () => null; `, filename: 'react.tsx', }, + { + code: ` +type O = { + func: () => void; +}; +const Component = (obj: O) => null; + 0} />; + `, + filename: 'react.tsx', + options: [{ checksVoidReturn: { attributes: false } }], + }, + { + code: ` +[Promise.resolve(), Promise.reject()].forEach(async val => { + await val; +}); + `, + options: [{ checksVoidReturn: { arguments: false } }], + }, + { + code: ` +type O = { f: () => void }; +const obj: O = { + f: async () => 'foo', +}; + `, + options: [{ checksVoidReturn: { properties: false } }], + }, + { + code: ` +function f(): () => void { + return async () => 0; +} + `, + options: [{ checksVoidReturn: { returns: false } }], + }, + { + code: ` +let f: () => void; +f = async () => { + return 3; +}; + `, + options: [{ checksVoidReturn: { variables: false } }], + }, ], invalid: [ @@ -357,6 +402,20 @@ if (!Promise.resolve()) { }, { code: ` +[Promise.resolve(), Promise.reject()].forEach(async val => { + await val; +}); + `, + errors: [ + { + line: 2, + messageId: 'voidReturnArgument', + }, + ], + options: [{ checksVoidReturn: { arguments: true } }], + }, + { + code: ` new Promise(async (resolve, reject) => { await Promise.resolve(); resolve(); @@ -534,6 +593,21 @@ f = async () => { }, { code: ` +let f: () => void; +f = async () => { + return 3; +}; + `, + errors: [ + { + line: 3, + messageId: 'voidReturnVariable', + }, + ], + options: [{ checksVoidReturn: { variables: true } }], + }, + { + code: ` const f: () => void = async () => { return 0; }; @@ -584,6 +658,21 @@ const obj: O = { { code: ` type O = { f: () => void }; +const obj: O = { + f: async () => 'foo', +}; + `, + errors: [ + { + line: 4, + messageId: 'voidReturnProperty', + }, + ], + options: [{ checksVoidReturn: { properties: true } }], + }, + { + code: ` +type O = { f: () => void }; const f = async () => 0; const obj: O = { f, @@ -656,6 +745,36 @@ function f(): () => void { }, { code: ` +function f(): () => void { + return async () => 0; +} + `, + errors: [ + { + line: 3, + messageId: 'voidReturnReturnValue', + }, + ], + options: [{ checksVoidReturn: { returns: true } }], + }, + { + code: ` +type O = { + func: () => void; +}; +const Component = (obj: O) => null; + 0} />; + `, + filename: 'react.tsx', + errors: [ + { + line: 6, + messageId: 'voidReturnAttribute', + }, + ], + }, + { + code: ` type O = { func: () => void; }; @@ -669,6 +788,7 @@ const Component = (obj: O) => null; messageId: 'voidReturnAttribute', }, ], + options: [{ checksVoidReturn: { attributes: true } }], }, { code: ` From 42c324cb0779021c0551002fe5bfa0929219a9d5 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Mar 2022 13:58:43 -0500 Subject: [PATCH 2/5] chore: revert some endline changes --- .../tests/rules/no-misused-promises.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts index 3b719178844..c8d318ba66e 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -258,7 +258,9 @@ interface ItLike { (name: string, callback: () => Promise): void; (name: string, callback: () => void): void; } + declare const it: ItLike; + it('', async () => {}); `, }, @@ -268,7 +270,9 @@ interface ItLike { (name: string, callback: () => void): void; (name: string, callback: () => Promise): void; } + declare const it: ItLike; + it('', async () => {}); `, }, @@ -280,7 +284,9 @@ interface ItLike { interface ItLike { (name: string, callback: () => Promise): void; } + declare const it: ItLike; + it('', async () => {}); `, }, @@ -292,7 +298,9 @@ interface ItLike { interface ItLike { (name: string, callback: () => void): void; } + declare const it: ItLike; + it('', async () => {}); `, }, @@ -627,7 +635,9 @@ interface ItLike { (name: string, callback: () => number): void; (name: string, callback: () => void): void; } + declare const it: ItLike; + it('', async () => {}); `, errors: [ @@ -645,7 +655,9 @@ interface ItLike { interface ItLike { (name: string, callback: () => void): void; } + declare const it: ItLike; + it('', async () => {}); `, errors: [ @@ -663,7 +675,9 @@ interface ItLike { interface ItLike { (name: string, callback: () => number): void; } + declare const it: ItLike; + it('', async () => {}); `, errors: [ From f67cb02d043418cddae0dde4b01b95676bb4c513 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Mar 2022 14:15:06 -0500 Subject: [PATCH 3/5] chore: reordered tests for cleaner diff post-merge --- .../tests/rules/no-misused-promises.test.ts | 194 +++++++----------- 1 file changed, 77 insertions(+), 117 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts index c8d318ba66e..8478b9a6d27 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -304,51 +304,6 @@ declare const it: ItLike; it('', async () => {}); `, }, - { - code: ` -type O = { - func: () => void; -}; -const Component = (obj: O) => null; - 0} />; - `, - filename: 'react.tsx', - options: [{ checksVoidReturn: { attributes: false } }], - }, - { - code: ` -[Promise.resolve(), Promise.reject()].forEach(async val => { - await val; -}); - `, - options: [{ checksVoidReturn: { arguments: false } }], - }, - { - code: ` -type O = { f: () => void }; -const obj: O = { - f: async () => 'foo', -}; - `, - options: [{ checksVoidReturn: { properties: false } }], - }, - { - code: ` -function f(): () => void { - return async () => 0; -} - `, - options: [{ checksVoidReturn: { returns: false } }], - }, - { - code: ` -let f: () => void; -f = async () => { - return 3; -}; - `, - options: [{ checksVoidReturn: { variables: false } }], - }, ], invalid: [ @@ -454,20 +409,6 @@ if (!Promise.resolve()) { }, { code: ` -[Promise.resolve(), Promise.reject()].forEach(async val => { - await val; -}); - `, - errors: [ - { - line: 2, - messageId: 'voidReturnArgument', - }, - ], - options: [{ checksVoidReturn: { arguments: true } }], - }, - { - code: ` new Promise(async (resolve, reject) => { await Promise.resolve(); resolve(); @@ -631,64 +572,6 @@ function test(p: Promise | undefined) { }, { code: ` -interface ItLike { - (name: string, callback: () => number): void; - (name: string, callback: () => void): void; -} - -declare const it: ItLike; - -it('', async () => {}); - `, - errors: [ - { - line: 9, - messageId: 'voidReturnArgument', - }, - ], - }, - { - code: ` -interface ItLike { - (name: string, callback: () => number): void; -} -interface ItLike { - (name: string, callback: () => void): void; -} - -declare const it: ItLike; - -it('', async () => {}); - `, - errors: [ - { - line: 11, - messageId: 'voidReturnArgument', - }, - ], - }, - { - code: ` -interface ItLike { - (name: string, callback: () => void): void; -} -interface ItLike { - (name: string, callback: () => number): void; -} - -declare const it: ItLike; - -it('', async () => {}); - `, - errors: [ - { - line: 11, - messageId: 'voidReturnArgument', - }, - ], - }, - { - code: ` let f: () => void; f = async () => { return 3; @@ -917,5 +800,82 @@ const Component = (obj: O) => null; }, ], }, + { + code: ` +interface ItLike { + (name: string, callback: () => number): void; + (name: string, callback: () => void): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + errors: [ + { + line: 9, + messageId: 'voidReturnArgument', + }, + ], + }, + { + code: ` +interface ItLike { + (name: string, callback: () => number): void; + (name: string, callback: () => void): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + errors: [ + { + line: 9, + messageId: 'voidReturnArgument', + }, + ], + options: [{ checksVoidReturn: { arguments: true } }], + }, + { + code: ` +interface ItLike { + (name: string, callback: () => number): void; +} +interface ItLike { + (name: string, callback: () => void): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + errors: [ + { + line: 11, + messageId: 'voidReturnArgument', + }, + ], + }, + { + code: ` +interface ItLike { + (name: string, callback: () => void): void; +} +interface ItLike { + (name: string, callback: () => number): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + errors: [ + { + line: 11, + messageId: 'voidReturnArgument', + }, + ], + }, ], }); From 43ee6151138ab6314a88aeac28d43c3a52b50149 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Mar 2022 14:23:26 -0500 Subject: [PATCH 4/5] chore: one last dup test --- .../tests/rules/no-misused-promises.test.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts index 8478b9a6d27..f643f4d91e1 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -820,25 +820,6 @@ it('', async () => {}); }, { code: ` -interface ItLike { - (name: string, callback: () => number): void; - (name: string, callback: () => void): void; -} - -declare const it: ItLike; - -it('', async () => {}); - `, - errors: [ - { - line: 9, - messageId: 'voidReturnArgument', - }, - ], - options: [{ checksVoidReturn: { arguments: true } }], - }, - { - code: ` interface ItLike { (name: string, callback: () => number): void; } From f921726d99be2beb656ff363154c147923527403 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 3 Mar 2022 05:07:22 -0500 Subject: [PATCH 5/5] docs: more on the checksVoidReturn options --- .../docs/rules/no-misused-promises.md | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-promises.md b/packages/eslint-plugin/docs/rules/no-misused-promises.md index cdacafd4f9f..42063d7931f 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-promises.md +++ b/packages/eslint-plugin/docs/rules/no-misused-promises.md @@ -39,8 +39,9 @@ const defaultOptions: Options = [ ]; ``` -If you don't want to check conditionals, you can configure the rule -like this: +### `"checksConditionals"` + +If you don't want to check conditionals, you can configure the rule with `"checksConditionals": false`: ```json { @@ -53,6 +54,10 @@ like this: } ``` +Doing so prevents the rule from looking at code like `if (somePromise)`. + +### `"checksVoidReturn"` + Likewise, if you don't want functions that return promises where a void return is expected to be checked, your configuration will look like this: @@ -67,8 +72,16 @@ expected to be checked, your configuration will look like this: } ``` -You can disable selective parts of the `checksVoidReturn` option by providing an -object that disables specific checks: +You can disable selective parts of the `checksVoidReturn` option by providing an object that disables specific checks. +The following options are supported: + +- `arguments`: Disables checking an asynchronous function passed as argument where the parameter type expects a function that returns `void` +- `attributes`: Disables checking an asynchronous function passed as a JSX attribute expected to be a function that returns `void` +- `properties`: Disables checking an asynchronous function passed as an object property expected to be a function that returns `void` +- `returns`: Disables checking an asynchronous function returned in a function whose return type is a function that returns `void` +- `variables`: Disables checking an asynchronous function used as a variable whose return type is a function that returns `void` + +For example, if you don't mind that passing a `() => Promise` to a `() => void` parameter or JSX attribute can lead to a floating unhandled Promise: ```json { @@ -76,7 +89,8 @@ object that disables specific checks: "error", { "checksVoidReturn": { - "arguments": false + "arguments": false, + "attributes": false } } ]