Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
New: Add no-promise-executor-return rule (fixes #12640) (#12648)
* New: Add no-promise-executor-return rule (fixes #12640)

* Fix eslint comments in docs

* Change error message
  • Loading branch information
mdjermanovic committed Jun 19, 2020
1 parent 09cc0a2 commit 9e1414e
Show file tree
Hide file tree
Showing 5 changed files with 522 additions and 0 deletions.
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 @@ -176,6 +176,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);
}
}
};
}
};

2 comments on commit 9e1414e

@berere
Copy link

@berere berere commented on 9e1414e Jun 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@berere
Copy link

@berere berere commented on 9e1414e Jun 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.