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

feat: Allow void in rule no-promise-executor-return #17282

Merged
47 changes: 47 additions & 0 deletions docs/src/rules/no-promise-executor-return.md
Expand Up @@ -63,6 +63,8 @@ new Promise((resolve, reject) => getSomething((err, data) => {
new Promise(() => {
return 1;
});

new Promise(r => r(1));
```

:::
Expand All @@ -74,6 +76,7 @@ Examples of **correct** code for this rule:
```js
/*eslint no-promise-executor-return: "error"*/

// Turn return inline into two lines
new Promise((resolve, reject) => {
if (someCondition) {
resolve(defaultResult);
Expand All @@ -88,6 +91,7 @@ new Promise((resolve, reject) => {
});
});

// Add curly braces
new Promise((resolve, reject) => {
getSomething((err, data) => {
if (err) {
Expand All @@ -98,7 +102,50 @@ new Promise((resolve, reject) => {
});
});

new Promise(r => { r(1) });
// or just use Promise.resolve
Promise.resolve(1);
```

:::

## Options
nopeless marked this conversation as resolved.
Show resolved Hide resolved

This rule takes one option, an object, with the following properties:

* `allowVoid`: If set to `true` (`false` by default), this rule will allow returning void values.

### allowVoid

Examples of **correct** code for this rule with the `{ "allowVoid": true }` option:

::: correct

```js
nopeless marked this conversation as resolved.
Show resolved Hide resolved
/*eslint no-promise-executor-return: ["error", { allowVoid: true }]*/

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

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

new Promise(r => void r(1));
```

:::
170 changes: 154 additions & 16 deletions lib/rules/no-promise-executor-return.js
Expand Up @@ -10,6 +10,7 @@
//------------------------------------------------------------------------------

const { findVariable } = require("@eslint-community/eslint-utils");
const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -59,6 +60,78 @@ function isPromiseExecutor(node, scope) {
isGlobalReference(parent.callee, getOuterScope(scope));
}

/**
* Checks if the given node is a void expression.
* @param {ASTNode} node The node to check.
* @returns {boolean} - `true` if the node is a void expression
*/
function expressionIsVoid(node) {
return node.type === "UnaryExpression" && node.operator === "void";
}

/**
* Fixes the linting error by prepending "void " to the given node
* @param {Object} sourceCode context given by context.sourceCode
* @param {ASTNode} node The node to fix.
* @param {Object} fixer The fixer object provided by ESLint.
* @returns {Array<Object>} - An array of fix objects to apply to the node.
*/
function voidPrependFixer(sourceCode, node, fixer) {
nopeless marked this conversation as resolved.
Show resolved Hide resolved

const requiresParens =

// prepending `void ` will fail if the node has a lower precedence than void
astUtils.getPrecedence(node) < astUtils.getPrecedence({ type: "UnaryExpression", operator: "void" }) &&

// check if there are parentheses around the node to avoid redundant parentheses
!astUtils.isParenthesised(sourceCode, node);

// avoid parentheses issues
const returnOrArrowToken = sourceCode.getTokenBefore(
node,
node.parent.type === "ArrowFunctionExpression"
? astUtils.isArrowToken

// isReturnToken
: token => token.type === "Keyword" && token.value === "return"
);

const firstToken = sourceCode.getTokenAfter(returnOrArrowToken);

const prependSpace =

// is return token, as => allows void to be adjacent
returnOrArrowToken.value === "return" &&

// If two tokens (return and "(") are adjacent
returnOrArrowToken.range[1] === firstToken.range[0];

return [
fixer.insertTextBefore(firstToken, `${prependSpace ? " " : ""}void ${requiresParens ? "(" : ""}`),
fixer.insertTextAfter(node, requiresParens ? ")" : "")
];
}

/**
* Fixes the linting error by `wrapping {}` around the given node's body.
* @param {Object} sourceCode context given by context.sourceCode
* @param {ASTNode} node The node to fix.
* @param {Object} fixer The fixer object provided by ESLint.
* @returns {Array<Object>} - An array of fix objects to apply to the node.
*/
function curlyWrapFixer(sourceCode, node, fixer) {

// https://github.com/eslint/eslint/pull/17282#issuecomment-1592795923
const arrowToken = sourceCode.getTokenBefore(node.body, astUtils.isArrowToken);
const firstToken = sourceCode.getTokenAfter(arrowToken);
const lastToken = sourceCode.getLastToken(node);

return [
fixer.insertTextBefore(firstToken, "{"),
fixer.insertTextAfter(lastToken, "}")
];
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -74,37 +147,80 @@ module.exports = {
url: "https://eslint.org/docs/latest/rules/no-promise-executor-return"
},

schema: [],
hasSuggestions: true,

schema: [{
type: "object",
properties: {
allowVoid: {
type: "boolean",
default: false
}
},
additionalProperties: false
}],

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

// arrow and function suggestions
prependVoid: "Prepend `void` to the expression.",

// only arrow suggestions
wrapBraces: "Wrap the expression in `{}`."
}
},

create(context) {

let funcInfo = null;
const sourceCode = context.sourceCode;

/**
* Reports the given node.
* @param {ASTNode} node Node to report.
* @returns {void}
*/
function report(node) {
context.report({ node, messageId: "returnsValue" });
}
const {
allowVoid = false
} = context.options[0] || {};

return {

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

if (funcInfo.shouldCheck && node.type === "ArrowFunctionExpression" && node.expression) {
report(node.body);
if (// Is a Promise executor
funcInfo.shouldCheck &&
node.type === "ArrowFunctionExpression" &&
node.expression &&

// Except void
!(allowVoid && expressionIsVoid(node.body))
) {
const suggest = [];

// prevent useless refactors
if (allowVoid) {
suggest.push({
messageId: "prependVoid",
fix(fixer) {
return voidPrependFixer(sourceCode, node.body, fixer);
}
});
}

suggest.push({
messageId: "wrapBraces",
fix(fixer) {
return curlyWrapFixer(sourceCode, node, fixer);
}
});

context.report({
node: node.body,
messageId: "returnsValue",
suggest
});
}
},

Expand All @@ -113,9 +229,31 @@ module.exports = {
},

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

// node is `return <expression>`
if (!allowVoid) {
context.report({ node, messageId: "returnsValue" });
return;
}

if (expressionIsVoid(node.argument)) {
return;
}

// allowVoid && !expressionIsVoid
context.report({
node,
messageId: "returnsValue",
suggest: [{
messageId: "prependVoid",
fix(fixer) {
return voidPrependFixer(sourceCode, node.argument, fixer);
}
}]
});
}
};
}
Expand Down