8000 fix(eslint-plugin): [return-await] don't error for `in-try-catch` if … · raszi/typescript-eslint@efdd521 · GitHub 8000
[go: up one dir, main page]
Skip to content

Commit efdd521

Browse files
soobingsoobing
authored
fix(eslint-plugin): [return-await] don't error for in-try-catch if the return is in a catch without a finally (typescript-eslint#2356)
1 parent 7c6fcee commit efdd521

File tree

3 files changed

+143
-7
lines changed

3 files changed

+143
-7
lines changed

‎packages/eslint-plugin/docs/rules/return-await.md

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ const defaultOptions: Options = 'in-try-catch';
2828
### `in-try-catch`
2929

3030
Requires that a returned promise must be `await`ed in `try-catch-finally` blocks, and disallows it elsewhere.
31+
Specifically:
32+
33+
- if you `return` a promise within a `try`, then it must be `await`ed.
34+
- if you `return` a promise within a `catch`, and there **_is no_** `finally`, then it **_must not_** be `await`ed.
35+
- if you `return` a promise within a `catch`, and there **_is a_** `finally`, then it **_must_** be `await`ed.
36+
- if you `return` a promise within a `finally`, then it **_must not_** be `await`ed.
3137

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

@@ -39,10 +45,38 @@ async function invalidInTryCatch1() {
3945
}
4046

4147
async function invalidInTryCatch2() {
42-
return await Promise.resolve('try');
48+
try {
49+
throw new Error('error');
50+
} catch (e) {
51+
return await Promise.resolve('catch');
52+
}
4353
}
4454

4555
async function invalidInTryCatch3() {
56+
try {
57+
throw new Error('error');
58+
} catch (e) {
59+
return Promise.resolve('catch');
60+
} finally {
61+
console.log('cleanup');
62+
}
63+
}
64+
65+
async function invalidInTryCatch4() {
66+
try {
67+
throw new Error('error');
68+
} catch (e) {
69+
throw new Error('error2');
70+
} finally {
71+
return await Promise.resolve('finally');
72+
}
73+
}
74+
75+
async function invalidInTryCatch5() {
76+
return await Promise.resolve('try');
77+
}
78+
79+
async function invalidInTryCatch6() {
4680
return await 'value';
4781
}
4882
```
@@ -57,10 +91,38 @@ async function validInTryCatch1() {
5791
}
5892

5993
async function validInTryCatch2() {
60-
return Promise.resolve('try');
94+
try {
95+
throw new Error('error');
96+
} catch (e) {
97+
return Promise.resolve('catch');
98+
}
6199
}
62100

63101
async function validInTryCatch3() {
102+
try {
103+
throw new Error('error');
104+
} catch (e) {
105+
return await Promise.resolve('catch');
106+
} finally {
107+
console.log('cleanup');
108+
}
109+
}
110+
111+
async function validInTryCatch4() {
112+
try {
113+
throw new Error('error');
114+
} catch (e) {
115+
throw new Error('error2');
116+
} finally {
117+
return Promise.resolve('finally');
118+
}
119+
}
120+
121+
async function validInTryCatch5() {
122+
return Promise.resolve('try');
123+
}
124+
125+
async function validInTryCatch6() {
64126
return 'value';
65127
}
66128
```

‎packages/eslint-plugin/src/rules/return-await.ts

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,23 +57,63 @@ export default util.createRule({
5757
};
5858
}
5959

60-
function inTryCatch(node: ts.Node): boolean {
60+
function inTry(node: ts.Node): boolean {
61+
let ancestor = node.parent;
62+
63+
while (ancestor && !ts.isFunctionLike(ancestor)) {
64+
if (tsutils.isTryStatement(ancestor)) {
65+
return true;
66+
}
67+
68+
ancestor = ancestor.parent;
69+
}
70+
71+
return false;
72+
}
73+
74+
function inCatch(node: ts.Node): boolean {
75+
let ancestor = node.parent;
76+
77+
while (ancestor && !ts.isFunctionLike(ancestor)) {
78+
if (tsutils.isCatchClause(ancestor)) {
79+
return true;
80+
}
81+
82+
ancestor = ancestor.parent;
83+
}
84+
85+
return false;
86+
}
87+
88+
function isReturnPromiseInFinally(node: ts.Node): boolean {
6189
let ancestor = node.parent;
6290

6391
while (ancestor && !ts.isFunctionLike(ancestor)) {
6492
if (
65-
tsutils.isTryStatement(ancestor) ||
66-
tsutils.isCatchClause(ancestor)
93+
tsutils.isTryStatement(ancestor.parent) &&
94+
tsutils.isBlock(ancestor) &&
95+
ancestor.parent.end === ancestor.end
6796
) {
6897
return true;
6998
}
70-
7199
ancestor = ancestor.parent;
72100
}
73101

74102
return false;
75103
}
76104

105+
function hasFinallyBlock(node: ts.Node): boolean {
106+
let ancestor = node.parent;
107+
108+
while (ancestor && !ts.isFunctionLike(ancestor)) {
109+
if (tsutils.isTryStatement(ancestor)) {
110+
return !!ancestor.finallyBlock;
111+
}
112+
ancestor = ancestor.parent;
113+
}
114+
return false;
115+
}
116+
77117
// function findTokensToRemove()
78118

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

165205
if (option === 'in-try-catch') {
166-
const isInTryCatch = inTryCatch(expression);
206+
const isInTryCatch = inTry(expression) || inCatch(expression);
167207
if (isAwait && !isInTryCatch) {
168208
context.report({
169209
messageId: 'disallowedPromiseAwait',
170210
node,
171211
fix: fixer => removeAwait(fixer, node),
172212
});
173213
} else if (!isAwait && isInTryCatch) {
214+
if (inCatch(expression) && !hasFinallyBlock(expression)) {
215+
return;
216+
}
217+
218+
if (isReturnPromiseInFinally(expression)) {
219+
return;
220+
}
221+
174222
context.report({
175223
messageId: 'requiredPromiseAwait',
176224
node,

‎packages/eslint-plugin/tests/rules/return-await.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,32 @@ ruleTester.run('return-await', rule, {
112112
}
113113
`,
114114
},
115+
{
116+
options: ['in-try-catch'],
117+
code: `
118+
async function test() {
119+
try {
120+
throw 'foo';
121+
} catch (e) {
122+
return Promise.resolve(1);
123+
}
124+
}
125+
`,
126+
},
127+
{
128+
options: ['in-try-catch'],
129+
code: `
130+
async function test() {
131+
try {
132+
throw 'foo';
133+
} catch (e) {
134+
throw 'foo2';
135+
} finally {
136+
return Promise.resolve(1);
137+
}
138+
}
139+
`,
140+
},
115141
{
116142
options: ['in-try-catch'],
117143
code: `

0 commit comments

Comments
 (0)
0