Skip to content

Commit

Permalink
Fix: handle try/catch correctly in no-return-await (fixes #7581) (#…
Browse files Browse the repository at this point in the history
…7582)

* Fix: handle try/catch correctly in `no-return-await` (fixes #7581)

* Use a while loop instead of recursion
  • Loading branch information
not-an-aardvark authored and btmills committed Nov 14, 2016
1 parent c4dd015 commit 8a0e92a
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 27 deletions.
24 changes: 23 additions & 1 deletion lib/rules/no-return-await.js
Expand Up @@ -4,6 +4,8 @@
*/
"use strict";

const astUtils = require("../ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -23,6 +25,26 @@ module.exports = {
},

create(context) {

/**
* Determines whether a thrown error from this node will be caught/handled within this function rather than immediately halting
* this function. For example, a statement in a `try` block will always have an error handler. A statement in
* a `catch` block will only have an error handler if there is also a `finally` block.
* @param {ASTNode} node A node representing a location where an could be thrown
* @returns {boolean} `true` if a thrown error will be caught/handled in this function
*/
function hasErrorHandler(node) {
let ancestor = node;

while (!astUtils.isFunction(ancestor) && ancestor.type !== "Program") {
if (ancestor.parent.type === "TryStatement" && (ancestor === ancestor.parent.block || ancestor === ancestor.parent.handler && ancestor.parent.finalizer)) {
return true;
}
ancestor = ancestor.parent;
}
return false;
}

return {
ArrowFunctionExpression(node) {
if (node.async && node.body.type === "AwaitExpression") {
Expand All @@ -39,7 +61,7 @@ module.exports = {
ReturnStatement(node) {
const argument = node.argument;

if (argument && argument.type === "AwaitExpression") {
if (argument && argument.type === "AwaitExpression" && !hasErrorHandler(node)) {
const sourceCode = context.getSourceCode();
const loc = argument.loc;

Expand Down
146 changes: 120 additions & 26 deletions tests/lib/rules/no-return-await.js
Expand Up @@ -16,54 +16,148 @@ const RuleTester = require("../../../lib/testers/rule-tester");
// Tests
//------------------------------------------------------------------------------

// pending https://github.com/eslint/espree/issues/304, the type should be "Keyword"
const errors = [{ message: "Redundant use of `await` on a return value.", type: "Identifier"}];

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2017 } });

ruleTester.run("no-return-await", rule, {

valid: [
{ code: "\nasync function foo() {\n\tawait bar(); return;\n}\n" },
{ code: "\nasync function foo() {\n\tconst x = await bar(); return x;\n}\n" },
{ code: "\nasync () => { return bar(); }\n" },
{ code: "\nasync () => bar()\n" },
{ code: "\nasync function foo() {\nif (a) {\n\t\tif (b) {\n\t\t\treturn bar();\n\t\t}\n\t}\n}\n" },
{ code: "\nasync () => {\nif (a) {\n\t\tif (b) {\n\t\t\treturn bar();\n\t\t}\n\t}\n}\n" },
"\nasync function foo() {\n\tawait bar(); return;\n}\n",
"\nasync function foo() {\n\tconst x = await bar(); return x;\n}\n",
"\nasync () => { return bar(); }\n",
"\nasync () => bar()\n",
"\nasync function foo() {\nif (a) {\n\t\tif (b) {\n\t\t\treturn bar();\n\t\t}\n\t}\n}\n",
"\nasync () => {\nif (a) {\n\t\tif (b) {\n\t\t\treturn bar();\n\t\t}\n\t}\n}\n",
`
async function foo() {
try {
return await bar();
} catch (e) {
baz();
}
}
`,
`
async function foo() {
try {
return await bar();
} finally {
baz();
}
}
`,
`
async function foo() {
try {}
catch (e) {
return await bar();
} finally {
baz();
}
}
`,
`
async function foo() {
try {
try {}
finally {
return await bar();
}
} finally {
baz();
}
}
`,
`
async function foo() {
try {
try {}
catch (e) {
return await bar();
}
} finally {
baz();
}
}
`
],

invalid: [
{
code: "\nasync function foo() {\n\treturn await bar();\n}\n",
errors: [{
message: "Redundant use of `await` on a return value.",
type: "Identifier", // pending https://github.com/eslint/espree/issues/304, should be "Keyword"
}],
errors,
},
{
code: "\nasync () => { return await bar(); }\n",
errors: [{
message: "Redundant use of `await` on a return value.",
type: "Identifier", // pending https://github.com/eslint/espree/issues/304, should be "Keyword"
}],
errors,
},
{
code: "\nasync () => await bar()\n",
errors: [{
message: "Redundant use of `await` on a return value.",
type: "Identifier", // pending https://github.com/eslint/espree/issues/304, should be "Keyword"
}],
errors,
},
{
code: "\nasync function foo() {\nif (a) {\n\t\tif (b) {\n\t\t\treturn await bar();\n\t\t}\n\t}\n}\n",
errors: [{
message: "Redundant use of `await` on a return value.",
type: "Identifier", // pending https://github.com/eslint/espree/issues/304, should be "Keyword"
}],
errors,
},
{
code: "\nasync () => {\nif (a) {\n\t\tif (b) {\n\t\t\treturn await bar();\n\t\t}\n\t}\n}\n",
errors: [{
message: "Redundant use of `await` on a return value.",
type: "Identifier", // pending https://github.com/eslint/espree/issues/304, should be "Keyword"
}],
errors,
},
{
code: `
async function foo() {
try {}
finally {
return await bar();
}
}
`,
errors
},
{
code: `
async function foo() {
try {}
catch (e) {
return await bar();
}
}
`,
errors
},
{
code: `
try {
async function foo() {
return await bar();
}
} catch (e) {}
`,
errors
},
{
code: `
try {
async () => await bar();
} catch (e) {}
`,
errors
},
{
code: `
async function foo() {
try {}
catch (e) {
try {}
catch (e) {
return await bar();
}
}
}
`,
errors
}
]
});

0 comments on commit 8a0e92a

Please sign in to comment.