Skip to content

Commit

Permalink
fix(eslint-plugin): [no-floating-promises] finally should be transpar…
Browse files Browse the repository at this point in the history 
…ent to unhandled promises (#7092)

* finally is transparent to rejection

* Unwrap object

* split up test cases and put ignoreVoid: false on some

* remove duplicated line
  • Loading branch information
@kirkwaiblinger
kirkwaiblinger committed Jul 16, 2023
1 parent a9d7bb1 commit 2a4421c
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 36 deletions.
25 changes: 14 additions & 11 deletions packages/eslint-plugin/src/rules/no-floating-promises.ts
View file
Expand Up @@ -233,8 +233,8 @@ export default util.createRule<Options, MessageId>({
}

if (node.type === AST_NODE_TYPES.CallExpression) {
// If the outer expression is a call, it must be either a `.then()` or
// `.catch()` that handles the promise.
// If the outer expression is a call, a `.catch()` or `.then()` with
// rejection handler handles the promise.

const catchRejectionHandler = getRejectionHandlerFromCatchCall(node);
if (catchRejectionHandler) {
Expand All @@ -254,10 +254,14 @@ export default util.createRule<Options, MessageId>({
}
}

if (isPromiseFinallyCallWithHandler(node)) {
return { isUnhandled: false };
// `x.finally()` is transparent to resolution of the promise, so check `x`.
// ("object" in this context is the `x` in `x.finally()`)
const promiseFinallyObject = getObjectFromFinallyCall(node);
if (promiseFinallyObject) {
return isUnhandledPromise(checker, promiseFinallyObject);
}

// All other cases are unhandled.
return { isUnhandled: true };
} else if (node.type === AST_NODE_TYPES.ConditionalExpression) {
// We must be getting the promise-like value from one of the branches of the
Expand Down Expand Up @@ -381,13 +385,12 @@ function getRejectionHandlerFromThenCall(
}
}

function isPromiseFinallyCallWithHandler(
function getObjectFromFinallyCall(
expression: TSESTree.CallExpression,
): boolean {
return (
expression.callee.type === AST_NODE_TYPES.MemberExpression &&
): TSESTree.Expression | undefined {
return expression.callee.type === AST_NODE_TYPES.MemberExpression &&
expression.callee.property.type === AST_NODE_TYPES.Identifier &&
expression.callee.property.name === 'finally' &&
expression.arguments.length >= 1
);
expression.callee.property.name === 'finally'
? expression.callee.object
: undefined;
}
114 changes: 89 additions & 25 deletions packages/eslint-plugin/tests/rules/no-floating-promises.test.ts
View file
Expand Up @@ -31,7 +31,6 @@ async function test() {
.catch(() => {})
.finally(() => {});
Promise.resolve('value').catch(() => {});
Promise.resolve('value').finally(() => {});
return Promise.resolve('value');
}
`,
Expand All @@ -58,7 +57,6 @@ async function test() {
.catch(() => {})
.finally(() => {});
Promise.reject(new Error('message')).catch(() => {});
Promise.reject(new Error('message')).finally(() => {});
return Promise.reject(new Error('message'));
}
`,
Expand All @@ -77,7 +75,6 @@ async function test() {
.catch(() => {})
.finally(() => {});
(async () => true)().catch(() => {});
(async () => true)().finally(() => {});
return (async () => true)();
}
`,
Expand All @@ -97,7 +94,6 @@ async function test() {
.catch(() => {})
.finally(() => {});
returnsPromise().catch(() => {});
returnsPromise().finally(() => {});
return returnsPromise();
}
`,
Expand All @@ -106,7 +102,6 @@ async function test() {
const x = Promise.resolve();
const y = x.then(() => {});
y.catch(() => {});
y.finally(() => {});
}
`,
`
Expand All @@ -117,7 +112,6 @@ async function test() {
`
async function test() {
Promise.resolve().catch(() => {}), 123;
Promise.resolve().finally(() => {}), 123;
123,
Promise.resolve().then(
() => {},
Expand Down Expand Up @@ -160,7 +154,6 @@ async function test() {
.catch(() => {})
.finally(() => {});
promiseValue.catch(() => {});
promiseValue.finally(() => {});
return promiseValue;
}
`,
Expand Down Expand Up @@ -193,12 +186,7 @@ async function test() {
() => {},
);
promiseIntersection.then(() => {}).catch(() => {});
promiseIntersection
.then(() => {})
.catch(() => {})
.finally(() => {});
promiseIntersection.catch(() => {});
promiseIntersection.finally(() => {});
return promiseIntersection;
}
`,
Expand All @@ -218,7 +206,6 @@ async function test() {
.catch(() => {})
.finally(() => {});
canThen.catch(() => {});
canThen.finally(() => {});
return canThen;
}
`,
Expand Down Expand Up @@ -315,7 +302,6 @@ async function test() {
.catch(() => {})
.finally(() => {});
promise.catch(() => {});
promise.finally(() => {});
return promise;
}
`,
Expand All @@ -333,7 +319,6 @@ async function test() {
?.then(() => {})
?.catch(() => {});
returnsPromise()?.catch(() => {});
returnsPromise()?.finally(() => {});
return returnsPromise();
}
`,
Expand Down Expand Up @@ -465,6 +450,31 @@ Promise.reject().catch(definitelyCallable);
`,
options: [{ ignoreVoid: false }],
},
{
code: `
Promise.reject()
.catch(() => {})
.finally(() => {});
`,
},
{
code: `
Promise.reject()
.catch(() => {})
.finally(() => {})
.finally(() => {});
`,
options: [{ ignoreVoid: false }],
},
{
code: `
Promise.reject()
.catch(() => {})
.finally(() => {})
.finally(() => {})
.finally(() => {});
`,
},
],

invalid: [
Expand Down Expand Up @@ -612,7 +622,6 @@ async function test() {
(async () => true)();
(async () => true)().then(() => {});
(async () => true)().catch();
(async () => true)().finally();
}
`,
errors: [
Expand All @@ -628,10 +637,6 @@ async function test() {
line: 5,
messageId: 'floatingVoid',
},
{
line: 6,
messageId: 'floatingVoid',
},
],
},
{
Expand Down Expand Up @@ -940,7 +945,6 @@ async function test() {
promiseIntersection;
promiseIntersection.then(() => {});
promiseIntersection.catch();
promiseIntersection.finally();
}
`,
errors: [
Expand All @@ -956,10 +960,6 @@ async function test() {
line: 7,
messageId: 'floatingVoid',
},
{
line: 8,
messageId: 'floatingVoid',
},
],
},
{
Expand Down Expand Up @@ -1587,5 +1587,69 @@ Promise.reject() || 3;
},
],
},
{
code: `
Promise.reject().finally(() => {});
`,
errors: [{ line: 2, messageId: 'floatingVoid' }],
},
{
code: `
Promise.reject()
.finally(() => {})
.finally(() => {});
`,
options: [{ ignoreVoid: false }],
errors: [{ line: 2, messageId: 'floating' }],
},
{
code: `
Promise.reject()
.finally(() => {})
.finally(() => {})
.finally(() => {});
`,
errors: [{ line: 2, messageId: 'floatingVoid' }],
},
{
code: `
Promise.reject()
.then(() => {})
.finally(() => {});
`,
errors: [{ line: 2, messageId: 'floatingVoid' }],
},
{
code: `
declare const returnsPromise: () => Promise<void> | null;
returnsPromise()?.finally(() => {});
`,
errors: [{ line: 3, messageId: 'floatingVoid' }],
},
{
code: `
const promiseIntersection: Promise<number> & number;
promiseIntersection.finally(() => {});
`,
errors: [{ line: 3, messageId: 'floatingVoid' }],
},
{
code: `
Promise.resolve().finally(() => {}), 123;
`,
errors: [{ line: 2, messageId: 'floatingVoid' }],
},
{
code: `
(async () => true)().finally();
`,
errors: [{ line: 2, messageId: 'floatingVoid' }],
},
{
code: `
Promise.reject(new Error('message')).finally(() => {});
`,
errors: [{ line: 2, messageId: 'floatingVoid' }],
},
],
});

0 comments on commit 2a4421c

Please sign in to comment.