Skip to content

Commit

Permalink
feat(eslint-plugin): [no-floating-promises] add option 'allowForKnown…
Browse files Browse the repository at this point in the history
…SafePromises' (#9186)

* feat: [no-floating-promises] add an
'allowForKnownSafePromises' option

fixes: #7008

* chore: snapshot errors

* chore: add a test

* chore: add another test

* chore: rewrite

* chore: docs & test

* chore: add jsdoc comment

* chore: cspell

* chore: lint

* chore: address reviews

* chore: lint errors

* chore: remove invalid js/ts code

* chore: add thenables

* chore: fix failures

* chore: re-write tests

* chore: more tests

* chore: fix schemas

* chore: typo

* fix: docs

* chore: nits

* snapshots

* chore: address reviews

* chore: reduce tests

* chore: line numbers

* chore: docs

* chore: typos

* chore: address reviews

* fix: condition issues

* chore: update docs

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

* chore: tag function

* chore: address reviews

* chore: revert var name

* chore: address reviews

* chore: lint & tests

* chore: comments

* new logic

* make use of closure

* rename variable

* wording

* whoops, copypasta mistake

* remove test comment

* simplify comments

* remove bare string specifier

* docs

---------

Co-authored-by: arka1002 <arka.mcu700@gmail.com>
Co-authored-by: Arka Pratim Chaudhuri <105232141+arka1002@users.noreply.github.com>
Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
  • Loading branch information
4 people committed Jun 3, 2024
1 parent d4504c8 commit 04990d5
Show file tree
Hide file tree
Showing 8 changed files with 612 additions and 62 deletions.
57 changes: 57 additions & 0 deletions packages/eslint-plugin/docs/rules/no-floating-promises.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,63 @@ await (async function () {
})();
```

### `allowForKnownSafePromises`

This option allows marking specific types as "safe" to be floating. For example, you may need to do this in the case of libraries whose APIs return Promises whose rejections are safely handled by the library.

This option takes an array of type specifiers to consider safe.
Each item in the array must have one of the following forms:

- A type defined in a file (`{ from: "file", name: "Foo", path: "src/foo-file.ts" }` with `path` being an optional path relative to the project root directory)
- A type from the default library (`{ from: "lib", name: "PromiseLike" }`)
- A type from a package (`{ from: "package", name: "Foo", package: "foo-lib" }`, this also works for types defined in a typings package).

Examples of code for this rule with:

```json
{
"allowForKnownSafePromises": [
{ "from": "file", "name": "SafePromise" },
{ "from": "lib", "name": "PromiseLike" },
{ "from": "package", "name": "Bar", "package": "bar-lib" }
]
}
```

<Tabs>
<TabItem value="❌ Incorrect">

```ts option='{"allowForKnownSafePromises":[{"from":"file","name":"SafePromise"},{"from":"lib","name":"PromiseLike"},{"from":"package","name":"Bar","package":"bar-lib"}]}'
let promise: Promise<number> = Promise.resolve(2);
promise;

function returnsPromise(): Promise<number> {
return Promise.resolve(42);
}

returnsPromise();
```

</TabItem>
<TabItem value="✅ Correct">

```ts option='{"allowForKnownSafePromises":[{"from":"file","name":"SafePromise"},{"from":"lib","name":"PromiseLike"},{"from":"package","name":"Bar","package":"bar-lib"}]}'
// promises can be marked as safe by using branded types
type SafePromise = Promise<number> & { __linterBrands?: string };

let promise: SafePromise = Promise.resolve(2);
promise;

function returnsSafePromise(): SafePromise {
return Promise.resolve(42);
}

returnsSafePromise();
```

</TabItem>
</Tabs>

## When Not To Use It

This rule can be difficult to enable on large existing projects that set up many floating Promises.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,12 @@ interface Foo {

Some complex types cannot easily be made readonly, for example the `HTMLElement` type or the `JQueryStatic` type from `@types/jquery`. This option allows you to globally disable reporting of such types.

Each item must be one of:
This option takes an array of type specifiers to ignore.
Each item in the array must have one of the following forms:

- A type defined in a file (`{from: "file", name: "Foo", path: "src/foo-file.ts"}` with `path` being an optional path relative to the project root directory)
- A type from the default library (`{from: "lib", name: "Foo"}`)
- A type from a package (`{from: "package", name: "Foo", package: "foo-lib"}`, this also works for types defined in a typings package).
- A type defined in a file (`{ from: "file", name: "Foo", path: "src/foo-file.ts" }` with `path` being an optional path relative to the project root directory)
- A type from the default library (`{ from: "lib", name: "Foo" }`)
- A type from a package (`{ from: "package", name: "Foo", package: "foo-lib" }`, this also works for types defined in a typings package).

Additionally, a type may be defined just as a simple string, which then matches the type independently of its origin.

Expand Down
118 changes: 67 additions & 51 deletions packages/eslint-plugin/src/rules/no-floating-promises.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,22 @@ import { AST_NODE_TYPES } from '@typescript-eslint/utils';
import * as tsutils from 'ts-api-utils';
import * as ts from 'typescript';

import type { TypeOrValueSpecifier } from '../util';
import {
createRule,
getOperatorPrecedence,
getParserServices,
OperatorPrecedence,
readonlynessOptionsDefaults,
readonlynessOptionsSchema,
typeMatchesSpecifier,
} from '../util';

type Options = [
{
ignoreVoid?: boolean;
ignoreIIFE?: boolean;
allowForKnownSafePromises?: TypeOrValueSpecifier[];
},
];

Expand Down Expand Up @@ -79,6 +84,7 @@ export default createRule<Options, MessageId>({
'Whether to ignore async IIFEs (Immediately Invoked Function Expressions).',
type: 'boolean',
},
allowForKnownSafePromises: readonlynessOptionsSchema.properties.allow,
},
additionalProperties: false,
},
Expand All @@ -89,12 +95,16 @@ export default createRule<Options, MessageId>({
{
ignoreVoid: true,
ignoreIIFE: false,
allowForKnownSafePromises: readonlynessOptionsDefaults.allow,
},
],

create(context, [options]) {
const services = getParserServices(context);
const checker = services.program.getTypeChecker();
// TODO: #5439
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const allowForKnownSafePromises = options.allowForKnownSafePromises!;

return {
ExpressionStatement(node): void {
Expand Down Expand Up @@ -253,11 +263,11 @@ export default createRule<Options, MessageId>({
// Check the type. At this point it can't be unhandled if it isn't a promise
// or array thereof.

if (isPromiseArray(checker, tsNode)) {
if (isPromiseArray(tsNode)) {
return { isUnhandled: true, promiseArray: true };
}

if (!isPromiseLike(checker, tsNode)) {
if (!isPromiseLike(tsNode)) {
return { isUnhandled: false };
}

Expand Down Expand Up @@ -322,63 +332,69 @@ export default createRule<Options, MessageId>({
// we just can't tell.
return { isUnhandled: false };
}
},
});

function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean {
const type = checker.getTypeAtLocation(node);
for (const ty of tsutils
.unionTypeParts(type)
.map(t => checker.getApparentType(t))) {
if (checker.isArrayType(ty)) {
const arrayType = checker.getTypeArguments(ty)[0];
if (isPromiseLike(checker, node, arrayType)) {
return true;
}
}
function isPromiseArray(node: ts.Node): boolean {
const type = checker.getTypeAtLocation(node);
for (const ty of tsutils
.unionTypeParts(type)
.map(t => checker.getApparentType(t))) {
if (checker.isArrayType(ty)) {
const arrayType = checker.getTypeArguments(ty)[0];
if (isPromiseLike(node, arrayType)) {
return true;
}
}

if (checker.isTupleType(ty)) {
for (const tupleElementType of checker.getTypeArguments(ty)) {
if (isPromiseLike(checker, node, tupleElementType)) {
return true;
if (checker.isTupleType(ty)) {
for (const tupleElementType of checker.getTypeArguments(ty)) {
if (isPromiseLike(node, tupleElementType)) {
return true;
}
}
}
}
return false;
}
}
return false;
}

// Modified from tsutils.isThenable() to only consider thenables which can be
// rejected/caught via a second parameter. Original source (MIT licensed):
//
// https://github.com/ajafff/tsutils/blob/49d0d31050b44b81e918eae4fbaf1dfe7b7286af/util/type.ts#L95-L125
function isPromiseLike(
checker: ts.TypeChecker,
node: ts.Node,
type?: ts.Type,
): boolean {
type ??= checker.getTypeAtLocation(node);
for (const ty of tsutils.unionTypeParts(checker.getApparentType(type))) {
const then = ty.getProperty('then');
if (then === undefined) {
continue;
}
// Modified from tsutils.isThenable() to only consider thenables which can be
// rejected/caught via a second parameter. Original source (MIT licensed):
//
// https://github.com/ajafff/tsutils/blob/49d0d31050b44b81e918eae4fbaf1dfe7b7286af/util/type.ts#L95-L125
function isPromiseLike(node: ts.Node, type?: ts.Type): boolean {
type ??= checker.getTypeAtLocation(node);

const thenType = checker.getTypeOfSymbolAtLocation(then, node);
if (
hasMatchingSignature(
thenType,
signature =>
signature.parameters.length >= 2 &&
isFunctionParam(checker, signature.parameters[0], node) &&
isFunctionParam(checker, signature.parameters[1], node),
)
) {
return true;
// Ignore anything specified by `allowForKnownSafePromises` option.
if (
allowForKnownSafePromises.some(allowedType =>
typeMatchesSpecifier(type, allowedType, services.program),
)
) {
return false;
}

for (const ty of tsutils.unionTypeParts(checker.getApparentType(type))) {
const then = ty.getProperty('then');
if (then === undefined) {
continue;
}

const thenType = checker.getTypeOfSymbolAtLocation(then, node);
if (
hasMatchingSignature(
thenType,
signature =>
signature.parameters.length >= 2 &&
isFunctionParam(checker, signature.parameters[0], node) &&
isFunctionParam(checker, signature.parameters[1], node),
)
) {
return true;
}
}
return false;
}
}
return false;
}
},
});

function hasMatchingSignature(
type: ts.Type,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 04990d5

Please sign in to comment.