From 1d3d64876cd6b51f2a6337ac2805966cd62407f6 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 3 Jun 2024 14:12:01 -0400 Subject: [PATCH 1/6] feat(eslint-plugin): [no-floating-promises] add 'allowForKnownSafePromiseReturns' option --- .../docs/rules/no-floating-promises.mdx | 38 +++++++++++++++++++ .../src/rules/no-floating-promises.ts | 25 +++++++++++- .../tests/rules/no-floating-promises.test.ts | 37 ++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-floating-promises.mdx b/packages/eslint-plugin/docs/rules/no-floating-promises.mdx index e3f047e6e4f0..2ac0540ad4c9 100644 --- a/packages/eslint-plugin/docs/rules/no-floating-promises.mdx +++ b/packages/eslint-plugin/docs/rules/no-floating-promises.mdx @@ -176,6 +176,44 @@ returnsSafePromise(); +### `allowForKnownSafePromiseReturns` + +This option allows marking specific functions as "safe" to be called to create floating Promises. +For example, you may need to do this in the case of libraries whose APIs may be called without handling the resultant Promises. + +This option takes the same array format as [`allowForKnownSafePromises`](#allowForKnownSafePromises). + +Examples of code for this rule with: + +```json +{ + "allowForKnownSafePromiseReturns": [ + { "from": "file", "name": "safe", "path": "input.ts" } + ] +} +``` + + + + +```ts option='{"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]}' +declare function unsafe(...args: unknown[]): Promise; + +unsafe('...', () => {}); +``` + + + + +```ts option='{"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]}' +declare function safe(...args: unknown[]): Promise; + +safe('...', () => {}); +``` + + + + ## When Not To Use It This rule can be difficult to enable on large existing projects that set up many floating Promises. diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 1ae5e602ae0e..653b06b13467 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -19,6 +19,7 @@ type Options = [ ignoreVoid?: boolean; ignoreIIFE?: boolean; allowForKnownSafePromises?: TypeOrValueSpecifier[]; + allowForKnownSafePromiseReturns?: TypeOrValueSpecifier[]; }, ]; @@ -85,6 +86,8 @@ export default createRule({ type: 'boolean', }, allowForKnownSafePromises: readonlynessOptionsSchema.properties.allow, + allowForKnownSafePromiseReturns: + readonlynessOptionsSchema.properties.allow, }, additionalProperties: false, }, @@ -96,6 +99,7 @@ export default createRule({ ignoreVoid: true, ignoreIIFE: false, allowForKnownSafePromises: readonlynessOptionsDefaults.allow, + allowForKnownSafePromiseReturns: readonlynessOptionsDefaults.allow, }, ], @@ -103,8 +107,11 @@ export default createRule({ const services = getParserServices(context); const checker = services.program.getTypeChecker(); // TODO: #5439 - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + /* eslint-disable @typescript-eslint/no-non-null-assertion */ const allowForKnownSafePromises = options.allowForKnownSafePromises!; + const allowForKnownSafePromiseReturns = + options.allowForKnownSafePromiseReturns!; + /* eslint-enable @typescript-eslint/no-non-null-assertion */ return { ExpressionStatement(node): void { @@ -118,6 +125,10 @@ export default createRule({ expression = expression.expression; } + if (isKnownSafePromiseReturn(expression)) { + return; + } + const { isUnhandled, nonFunctionHandler, promiseArray } = isUnhandledPromise(checker, expression); @@ -197,6 +208,18 @@ export default createRule({ }, }; + function isKnownSafePromiseReturn(node: TSESTree.Node): boolean { + if (node.type !== AST_NODE_TYPES.CallExpression) { + return false; + } + + const type = services.getTypeAtLocation(node.callee); + + return allowForKnownSafePromiseReturns.some(allowedType => + typeMatchesSpecifier(type, allowedType, services.program), + ); + } + function isHigherPrecedenceThanUnary(node: ts.Node): boolean { const operator = ts.isBinaryExpression(node) ? node.operatorToken.kind diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index e6087e512265..40b164142e0f 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -698,6 +698,24 @@ myTag\`abc\`; { allowForKnownSafePromises: [{ from: 'file', name: 'SafePromise' }] }, ], }, + { + code: ` + declare function it(...args: unknown[]): Promise; + + it('...', () => {}); + `, + options: [ + { + allowForKnownSafePromiseReturns: [ + { + from: 'file', + name: 'it', + path: 'tests/fixtures/file.ts', + }, + ], + }, + ], + }, { code: ` declare const myTag: (strings: TemplateStringsArray) => Promise; @@ -2181,5 +2199,24 @@ myTag\`abc\`; options: [{ allowForKnownSafePromises: [{ from: 'file', name: 'Foo' }] }], errors: [{ line: 4, messageId: 'floatingVoid' }], }, + { + code: ` + declare function unsafe(...args: unknown[]): Promise; + + unsafe('...', () => {}); + `, + errors: [{ line: 4, messageId: 'floatingVoid' }], + options: [ + { + allowForKnownSafePromiseReturns: [ + { + from: 'file', + name: 'it', + path: 'tests/fixtures/file.ts', + }, + ], + }, + ], + }, ], }); From 9c154790db2efd24e981928decfc656603becf72 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 3 Jun 2024 15:02:51 -0400 Subject: [PATCH 2/6] test: always with the snapshots --- .../docs/rules/no-floating-promises.mdx | 2 +- .../no-floating-promises.shot | 22 ++++ .../no-floating-promises.shot | 111 ++++++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-floating-promises.mdx b/packages/eslint-plugin/docs/rules/no-floating-promises.mdx index 2ac0540ad4c9..02e703dd808e 100644 --- a/packages/eslint-plugin/docs/rules/no-floating-promises.mdx +++ b/packages/eslint-plugin/docs/rules/no-floating-promises.mdx @@ -205,7 +205,7 @@ unsafe('...', () => {}); -```ts option='{"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]}' +```ts option='{"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]}' skipValidation declare function safe(...args: unknown[]): Promise; safe('...', () => {}); diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-floating-promises.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-floating-promises.shot index 44e303fc3131..e40c0c94a0f1 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-floating-promises.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-floating-promises.shot @@ -108,3 +108,25 @@ function returnsSafePromise(): SafePromise { returnsSafePromise(); " `; + +exports[`Validating rule docs no-floating-promises.mdx code examples ESLint output 7`] = ` +"Incorrect +Options: {"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]} + +declare function unsafe(...args: unknown[]): Promise; + +unsafe('...', () => {}); +~~~~~~~~~~~~~~~~~~~~~~~~ Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the \`void\` operator. +" +`; + +exports[`Validating rule docs no-floating-promises.mdx code examples ESLint output 8`] = ` +"Correct +Options: {"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]} + +declare function safe(...args: unknown[]): Promise; + +safe('...', () => {}); +~~~~~~~~~~~~~~~~~~~~~~ Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the \`void\` operator. +" +`; diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-floating-promises.shot b/packages/eslint-plugin/tests/schema-snapshots/no-floating-promises.shot index a708c7001d5b..534ba63fbae4 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-floating-promises.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-floating-promises.shot @@ -8,6 +8,100 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos { "additionalProperties": false, "properties": { + "allowForKnownSafePromiseReturns": { + "items": { + "oneOf": [ + { + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "from": { + "enum": ["file"], + "type": "string" + }, + "name": { + "oneOf": [ + { + "type": "string" + }, + { + "items": { + "type": "string" + }, + "minItems": 1, + "type": "array", + "uniqueItems": true + } + ] + }, + "path": { + "type": "string" + } + }, + "required": ["from", "name"], + "type": "object" + }, + { + "additionalProperties": false, + "properties": { + "from": { + "enum": ["lib"], + "type": "string" + }, + "name": { + "oneOf": [ + { + "type": "string" + }, + { + "items": { + "type": "string" + }, + "minItems": 1, + "type": "array", + "uniqueItems": true + } + ] + } + }, + "required": ["from", "name"], + "type": "object" + }, + { + "additionalProperties": false, + "properties": { + "from": { + "enum": ["package"], + "type": "string" + }, + "name": { + "oneOf": [ + { + "type": "string" + }, + { + "items": { + "type": "string" + }, + "minItems": 1, + "type": "array", + "uniqueItems": true + } + ] + }, + "package": { + "type": "string" + } + }, + "required": ["from", "name", "package"], + "type": "object" + } + ] + }, + "type": "array" + }, "allowForKnownSafePromises": { "items": { "oneOf": [ @@ -120,6 +214,23 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos type Options = [ { + allowForKnownSafePromiseReturns?: ( + | { + from: 'file'; + name: [string, ...string[]] | string; + path?: string; + } + | { + from: 'lib'; + name: [string, ...string[]] | string; + } + | { + from: 'package'; + name: [string, ...string[]] | string; + package: string; + } + | string + )[]; allowForKnownSafePromises?: ( | { from: 'file'; From 54d380c0ac991b3bd8e312692872959f0eefa1de Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 4 Jun 2024 15:30:53 -0400 Subject: [PATCH 3/6] fix unit test file relativity --- .../eslint-plugin/tests/rules/no-floating-promises.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 40b164142e0f..cc1d001d8e7e 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -710,7 +710,10 @@ myTag\`abc\`; { from: 'file', name: 'it', - path: 'tests/fixtures/file.ts', + // https://github.com/typescript-eslint/typescript-eslint/pull/9234/files#r1626465054 + path: process.env.TYPESCRIPT_ESLINT_PROJECT_SERVICE + ? 'file.ts' + : 'tests/fixtures/file.ts', }, ], }, From c6da9d99a6398af0cd8fefaec5ba2a13cc78ba2f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 5 Jun 2024 11:10:27 -0400 Subject: [PATCH 4/6] Add .then test --- .../tests/rules/no-floating-promises.test.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index cc1d001d8e7e..d0db896b15a8 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -2215,7 +2215,32 @@ myTag\`abc\`; { from: 'file', name: 'it', - path: 'tests/fixtures/file.ts', + // https://github.com/typescript-eslint/typescript-eslint/pull/9234/files#r1626465054 + path: process.env.TYPESCRIPT_ESLINT_PROJECT_SERVICE + ? 'file.ts' + : 'tests/fixtures/file.ts', + }, + ], + }, + ], + }, + { + code: ` + declare function safe(...args: unknown[]): Promise; + + safe('...', () => {}).then(() => {}); + `, + errors: [{ line: 4, messageId: 'floatingVoid' }], + options: [ + { + allowForKnownSafePromiseReturns: [ + { + from: 'file', + name: 'it', + // https://github.com/typescript-eslint/typescript-eslint/pull/9234/files#r1626465054 + path: process.env.TYPESCRIPT_ESLINT_PROJECT_SERVICE + ? 'file.ts' + : 'tests/fixtures/file.ts', }, ], }, From 7eb4c3df5c1c531861aa9e0a3da009dffba3a60f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 5 Jun 2024 11:11:56 -0400 Subject: [PATCH 5/6] Update option name: allowForKnownSafeCalls --- .../docs/rules/no-floating-promises.mdx | 8 ++++---- .../eslint-plugin/src/rules/no-floating-promises.ts | 12 +++++------- .../no-floating-promises.shot | 4 ++-- .../tests/rules/no-floating-promises.test.ts | 6 +++--- .../tests/schema-snapshots/no-floating-promises.shot | 4 ++-- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-floating-promises.mdx b/packages/eslint-plugin/docs/rules/no-floating-promises.mdx index 02e703dd808e..c3d051151d6f 100644 --- a/packages/eslint-plugin/docs/rules/no-floating-promises.mdx +++ b/packages/eslint-plugin/docs/rules/no-floating-promises.mdx @@ -176,7 +176,7 @@ returnsSafePromise(); -### `allowForKnownSafePromiseReturns` +### `allowForKnownSafeCalls` This option allows marking specific functions as "safe" to be called to create floating Promises. For example, you may need to do this in the case of libraries whose APIs may be called without handling the resultant Promises. @@ -187,7 +187,7 @@ Examples of code for this rule with: ```json { - "allowForKnownSafePromiseReturns": [ + "allowForKnownSafeCalls": [ { "from": "file", "name": "safe", "path": "input.ts" } ] } @@ -196,7 +196,7 @@ Examples of code for this rule with: -```ts option='{"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]}' +```ts option='{"allowForKnownSafeCalls":[{"from":"file","name":"safe","path":"input.ts"}]}' declare function unsafe(...args: unknown[]): Promise; unsafe('...', () => {}); @@ -205,7 +205,7 @@ unsafe('...', () => {}); -```ts option='{"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]}' skipValidation +```ts option='{"allowForKnownSafeCalls":[{"from":"file","name":"safe","path":"input.ts"}]}' skipValidation declare function safe(...args: unknown[]): Promise; safe('...', () => {}); diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 653b06b13467..ad25d241be55 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -19,7 +19,7 @@ type Options = [ ignoreVoid?: boolean; ignoreIIFE?: boolean; allowForKnownSafePromises?: TypeOrValueSpecifier[]; - allowForKnownSafePromiseReturns?: TypeOrValueSpecifier[]; + allowForKnownSafeCalls?: TypeOrValueSpecifier[]; }, ]; @@ -86,8 +86,7 @@ export default createRule({ type: 'boolean', }, allowForKnownSafePromises: readonlynessOptionsSchema.properties.allow, - allowForKnownSafePromiseReturns: - readonlynessOptionsSchema.properties.allow, + allowForKnownSafeCalls: readonlynessOptionsSchema.properties.allow, }, additionalProperties: false, }, @@ -99,7 +98,7 @@ export default createRule({ ignoreVoid: true, ignoreIIFE: false, allowForKnownSafePromises: readonlynessOptionsDefaults.allow, - allowForKnownSafePromiseReturns: readonlynessOptionsDefaults.allow, + allowForKnownSafeCalls: readonlynessOptionsDefaults.allow, }, ], @@ -109,8 +108,7 @@ export default createRule({ // TODO: #5439 /* eslint-disable @typescript-eslint/no-non-null-assertion */ const allowForKnownSafePromises = options.allowForKnownSafePromises!; - const allowForKnownSafePromiseReturns = - options.allowForKnownSafePromiseReturns!; + const allowForKnownSafeCalls = options.allowForKnownSafeCalls!; /* eslint-enable @typescript-eslint/no-non-null-assertion */ return { @@ -215,7 +213,7 @@ export default createRule({ const type = services.getTypeAtLocation(node.callee); - return allowForKnownSafePromiseReturns.some(allowedType => + return allowForKnownSafeCalls.some(allowedType => typeMatchesSpecifier(type, allowedType, services.program), ); } diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-floating-promises.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-floating-promises.shot index e40c0c94a0f1..e1f38e71fe3e 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-floating-promises.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-floating-promises.shot @@ -111,7 +111,7 @@ returnsSafePromise(); exports[`Validating rule docs no-floating-promises.mdx code examples ESLint output 7`] = ` "Incorrect -Options: {"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]} +Options: {"allowForKnownSafeCalls":[{"from":"file","name":"safe","path":"input.ts"}]} declare function unsafe(...args: unknown[]): Promise; @@ -122,7 +122,7 @@ unsafe('...', () => {}); exports[`Validating rule docs no-floating-promises.mdx code examples ESLint output 8`] = ` "Correct -Options: {"allowForKnownSafePromiseReturns":[{"from":"file","name":"safe","path":"input.ts"}]} +Options: {"allowForKnownSafeCalls":[{"from":"file","name":"safe","path":"input.ts"}]} declare function safe(...args: unknown[]): Promise; diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index d0db896b15a8..7b32969bf391 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -706,7 +706,7 @@ myTag\`abc\`; `, options: [ { - allowForKnownSafePromiseReturns: [ + allowForKnownSafeCalls: [ { from: 'file', name: 'it', @@ -2211,7 +2211,7 @@ myTag\`abc\`; errors: [{ line: 4, messageId: 'floatingVoid' }], options: [ { - allowForKnownSafePromiseReturns: [ + allowForKnownSafeCalls: [ { from: 'file', name: 'it', @@ -2233,7 +2233,7 @@ myTag\`abc\`; errors: [{ line: 4, messageId: 'floatingVoid' }], options: [ { - allowForKnownSafePromiseReturns: [ + allowForKnownSafeCalls: [ { from: 'file', name: 'it', diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-floating-promises.shot b/packages/eslint-plugin/tests/schema-snapshots/no-floating-promises.shot index 534ba63fbae4..b73c2638b92e 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-floating-promises.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-floating-promises.shot @@ -8,7 +8,7 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos { "additionalProperties": false, "properties": { - "allowForKnownSafePromiseReturns": { + "allowForKnownSafeCalls": { "items": { "oneOf": [ { @@ -214,7 +214,7 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos type Options = [ { - allowForKnownSafePromiseReturns?: ( + allowForKnownSafeCalls?: ( | { from: 'file'; name: [string, ...string[]] | string; From ed8b7d97b638f232c5a79b1c47037fa6b5360eb1 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 5 Jun 2024 17:51:15 -0400 Subject: [PATCH 6/6] Add .finally test --- .../tests/rules/no-floating-promises.test.ts | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 7b32969bf391..d5d62eeddf56 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -2226,9 +2226,31 @@ myTag\`abc\`; }, { code: ` - declare function safe(...args: unknown[]): Promise; + declare function it(...args: unknown[]): Promise; + + it('...', () => {}).then(() => {}); + `, + errors: [{ line: 4, messageId: 'floatingVoid' }], + options: [ + { + allowForKnownSafeCalls: [ + { + from: 'file', + name: 'it', + // https://github.com/typescript-eslint/typescript-eslint/pull/9234/files#r1626465054 + path: process.env.TYPESCRIPT_ESLINT_PROJECT_SERVICE + ? 'file.ts' + : 'tests/fixtures/file.ts', + }, + ], + }, + ], + }, + { + code: ` + declare function it(...args: unknown[]): Promise; - safe('...', () => {}).then(() => {}); + it('...', () => {}).finally(() => {}); `, errors: [{ line: 4, messageId: 'floatingVoid' }], options: [