Skip to content
Permalink
Browse files

fix(eslint-plugin): [return-await] don't error for `in-try-catch` if …

…the return is in a `catch` without a `finally` (#2356)
  • Loading branch information
soobing committed Sep 14, 2020
1 parent 7c6fcee commit efdd5213ceaef332cf0b2c26573176f844d22a09
@@ -28,6 +28,12 @@ const defaultOptions: Options = 'in-try-catch';
### `in-try-catch`

Requires that a returned promise must be `await`ed in `try-catch-finally` blocks, and disallows it elsewhere.
Specifically:

- if you `return` a promise within a `try`, then it must be `await`ed.
- if you `return` a promise within a `catch`, and there **_is no_** `finally`, then it **_must not_** be `await`ed.
- if you `return` a promise within a `catch`, and there **_is a_** `finally`, then it **_must_** be `await`ed.
- if you `return` a promise within a `finally`, then it **_must not_** be `await`ed.

Examples of **incorrect** code with `in-try-catch`:

@@ -39,10 +45,38 @@ async function invalidInTryCatch1() {
}
async function invalidInTryCatch2() {
return await Promise.resolve('try');
try {
throw new Error('error');
} catch (e) {
return await Promise.resolve('catch');
}
}
async function invalidInTryCatch3() {
try {
throw new Error('error');
} catch (e) {
return Promise.resolve('catch');
} finally {
console.log('cleanup');
}
}
async function invalidInTryCatch4() {
try {
throw new Error('error');
} catch (e) {
throw new Error('error2');
} finally {
return await Promise.resolve('finally');
}
}
async function invalidInTryCatch5() {
return await Promise.resolve('try');
}
async function invalidInTryCatch6() {
return await 'value';
}
```
@@ -57,10 +91,38 @@ async function validInTryCatch1() {
}
async function validInTryCatch2() {
return Promise.resolve('try');
try {
throw new Error('error');
} catch (e) {
return Promise.resolve('catch');
}
}
async function validInTryCatch3() {
try {
throw new Error('error');
} catch (e) {
return await Promise.resolve('catch');
} finally {
console.log('cleanup');
}
}
async function validInTryCatch4() {
try {
throw new Error('error');
} catch (e) {
throw new Error('error2');
} finally {
return Promise.resolve('finally');
}
}
async function validInTryCatch5() {
return Promise.resolve('try');
}
async function validInTryCatch6() {
return 'value';
}
```
@@ -57,23 +57,63 @@ export default util.createRule({
};
}

function inTryCatch(node: ts.Node): boolean {
function inTry(node: ts.Node): boolean {
let ancestor = node.parent;

while (ancestor && !ts.isFunctionLike(ancestor)) {
if (tsutils.isTryStatement(ancestor)) {
return true;
}

ancestor = ancestor.parent;
}

return false;
}

function inCatch(node: ts.Node): boolean {
let ancestor = node.parent;

while (ancestor && !ts.isFunctionLike(ancestor)) {
if (tsutils.isCatchClause(ancestor)) {
return true;
}

ancestor = ancestor.parent;
}

return false;
}

function isReturnPromiseInFinally(node: ts.Node): boolean {
let ancestor = node.parent;

while (ancestor && !ts.isFunctionLike(ancestor)) {
if (
tsutils.isTryStatement(ancestor) ||
tsutils.isCatchClause(ancestor)
tsutils.isTryStatement(ancestor.parent) &&
tsutils.isBlock(ancestor) &&
ancestor.parent.end === ancestor.end
) {
return true;
}

ancestor = ancestor.parent;
}

return false;
}

function hasFinallyBlock(node: ts.Node): boolean {
let ancestor = node.parent;

while (ancestor && !ts.isFunctionLike(ancestor)) {
if (tsutils.isTryStatement(ancestor)) {
return !!ancestor.finallyBlock;
}
ancestor = ancestor.parent;
}
return false;
}

// function findTokensToRemove()

function removeAwait(
@@ -163,14 +203,22 @@ export default util.createRule({
}

if (option === 'in-try-catch') {
const isInTryCatch = inTryCatch(expression);
const isInTryCatch = inTry(expression) || inCatch(expression);
if (isAwait && !isInTryCatch) {
context.report({
messageId: 'disallowedPromiseAwait',
node,
fix: fixer => removeAwait(fixer, node),
});
} else if (!isAwait && isInTryCatch) {
if (inCatch(expression) && !hasFinallyBlock(expression)) {
return;
}

if (isReturnPromiseInFinally(expression)) {
return;
}

context.report({
messageId: 'requiredPromiseAwait',
node,
@@ -112,6 +112,32 @@ ruleTester.run('return-await', rule, {
}
`,
},
{
options: ['in-try-catch'],
code: `
async function test() {
try {
throw 'foo';
} catch (e) {
return Promise.resolve(1);
}
}
`,
},
{
options: ['in-try-catch'],
code: `
async function test() {
try {
throw 'foo';
} catch (e) {
throw 'foo2';
} finally {
return Promise.resolve(1);
}
}
`,
},
{
options: ['in-try-catch'],
code: `

0 comments on commit efdd521

Please sign in to comment.
You can’t perform that action at this time.