Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: Add no-promise-executor-return rule (fixes #12640) #12648

Merged
merged 3 commits into from Jun 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
96 changes: 96 additions & 0 deletions docs/rules/no-promise-executor-return.md
@@ -0,0 +1,96 @@
# Disallow returning values from Promise executor functions (no-promise-executor-return)

The `new Promise` constructor accepts a single argument, called an *executor*.

```js
const myPromise = new Promise(function executor(resolve, reject) {
readFile('foo.txt', function(err, result) {
if (err) {
reject(err);
} else {
resolve(result);
}
});
});
```

The executor function usually initiates some asynchronous operation. Once it is finished, the executor should call `resolve` with the result, or `reject` if an error occurred.

The return value of the executor is ignored. Returning a value from an executor function is a possible error because the returned value cannot be used and it doesn't affect the promise in any way.

## Rule Details

This rule disallows returning values from Promise executor functions.

Only `return` without a value is allowed, as it's a control flow statement.

Examples of **incorrect** code for this rule:

```js
/*eslint no-promise-executor-return: "error"*/

new Promise((resolve, reject) => {
if (someCondition) {
return defaultResult;
}
getSomething((err, result) => {
if (err) {
reject(err);
} else {
resolve(result);
}
});
});

new Promise((resolve, reject) => getSomething((err, data) => {
if (err) {
reject(err);
} else {
resolve(data);
}
}));

new Promise(() => {
return 1;
});
```

Examples of **correct** code for this rule:

```js
/*eslint no-promise-executor-return: "error"*/

new Promise((resolve, reject) => {
if (someCondition) {
resolve(defaultResult);
return;
}
getSomething((err, result) => {
if (err) {
reject(err);
} else {
resolve(result);
}
});
});

new Promise((resolve, reject) => {
getSomething((err, data) => {
if (err) {
reject(err);
} else {
resolve(data);
}
});
});

Promise.resolve(1);
```

## Further Reading

* [MDN Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise)

## Related Rules

* [no-async-promise-executor](no-async-promise-executor.md)
1 change: 1 addition & 0 deletions lib/rules/index.js
Expand Up @@ -174,6 +174,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
"no-plusplus": () => require("./no-plusplus"),
"no-process-env": () => require("./no-process-env"),
"no-process-exit": () => require("./no-process-exit"),
"no-promise-executor-return": () => require("./no-promise-executor-return"),
"no-proto": () => require("./no-proto"),
"no-prototype-builtins": () => require("./no-prototype-builtins"),
"no-redeclare": () => require("./no-redeclare"),
Expand Down
121 changes: 121 additions & 0 deletions lib/rules/no-promise-executor-return.js
@@ -0,0 +1,121 @@
/**
* @fileoverview Rule to disallow returning values from Promise executor functions
* @author Milos Djermanovic
*/

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const { findVariable } = require("eslint-utils");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

const functionTypesToCheck = new Set(["ArrowFunctionExpression", "FunctionExpression"]);

/**
* Determines whether the given identifier node is a reference to a global variable.
* @param {ASTNode} node `Identifier` node to check.
* @param {Scope} scope Scope to which the node belongs.
* @returns {boolean} True if the identifier is a reference to a global variable.
*/
function isGlobalReference(node, scope) {
const variable = findVariable(scope, node);

return variable !== null && variable.scope.type === "global" && variable.defs.length === 0;
}

/**
* Finds function's outer scope.
* @param {Scope} scope Function's own scope.
* @returns {Scope} Function's outer scope.
*/
function getOuterScope(scope) {
const upper = scope.upper;

if (upper.type === "function-expression-name") {
return upper.upper;
}
return upper;
}

/**
* Determines whether the given function node is used as a Promise executor.
* @param {ASTNode} node The node to check.
* @param {Scope} scope Function's own scope.
* @returns {boolean} `true` if the node is a Promise executor.
*/
function isPromiseExecutor(node, scope) {
const parent = node.parent;

return parent.type === "NewExpression" &&
parent.arguments[0] === node &&
parent.callee.type === "Identifier" &&
parent.callee.name === "Promise" &&
isGlobalReference(parent.callee, getOuterScope(scope));
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
type: "problem",

docs: {
description: "disallow returning values from Promise executor functions",
category: "Possible Errors",
recommended: false,
url: "https://eslint.org/docs/rules/no-promise-executor-return"
},

schema: [],

messages: {
returnsValue: "Return values from promise executor functions cannot be read."
}
},

create(context) {

let funcInfo = null;

/**
* Reports the given node.
* @param {ASTNode} node Node to report.
* @returns {void}
*/
function report(node) {
context.report({ node, messageId: "returnsValue" });
}

return {

onCodePathStart(_, node) {
funcInfo = {
upper: funcInfo,
shouldCheck: functionTypesToCheck.has(node.type) && isPromiseExecutor(node, context.getScope())
};

if (funcInfo.shouldCheck && node.type === "ArrowFunctionExpression" && node.expression) {
report(node.body);
}
},

onCodePathEnd() {
funcInfo = funcInfo.upper;
},

ReturnStatement(node) {
if (funcInfo.shouldCheck && node.argument) {
report(node);
}
}
};
}
};