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

* feat: Allow `void` in `no-promise-executor-return` (#17278) * feat: Autofix and suggestions for `no-promise-executor-return` * fix: update behavior for `no-promise-executor-return` * fix: update suggestion behavior * docs: update to match code * docs: misc fixes * fix: refactors * fix: parentheses issues * Update docs/src/rules/no-promise-executor-return.md Co-authored-by: Milos Djermanovic <[email protected]> * Update docs/src/rules/no-promise-executor-return.md Co-authored-by: Milos Djermanovic <[email protected]> * Update lib/rules/no-promise-executor-return.js Co-authored-by: Milos Djermanovic <[email protected]> * fix: adjacent token issue * fix comments --------- Co-authored-by: Milos Djermanovic <[email protected]>
eslint · Aug 20, 2023 · ee2f718 · ee2f718
1 parent 9e9edf9
commit ee2f718
Show file tree

Hide file tree

Showing 3 changed files with 518 additions and 50 deletions.
diff --git a/docs/src/rules/no-promise-executor-return.md b/docs/src/rules/no-promise-executor-return.md
@@ -63,6 +63,8 @@ new Promise((resolve, reject) => getSomething((err, data) => {
 new Promise(() => {
     return 1;
 });
+
+new Promise(r => r(1));
 ```
 
 :::
@@ -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);
@@ -88,6 +91,7 @@ new Promise((resolve, reject) => {
     });
 });
 
+// Add curly braces
 new Promise((resolve, reject) => {
     getSomething((err, data) => {
         if (err) {
@@ -98,7 +102,50 @@ new Promise((resolve, reject) => {
     });
 });
 
+new Promise(r => { r(1) });
+// or just use Promise.resolve
 Promise.resolve(1);
 ```
 
 :::
+
+## Options
+
+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
+/*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));
+```
+
+:::
diff --git a/lib/rules/no-promise-executor-return.js b/lib/rules/no-promise-executor-return.js
@@ -10,6 +10,7 @@
 //------------------------------------------------------------------------------
 
 const { findVariable } = require("@eslint-community/eslint-utils");
+const astUtils = require("./utils/ast-utils");
 
 //------------------------------------------------------------------------------
 // Helpers
@@ -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) {
+
+    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
 //------------------------------------------------------------------------------
@@ -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
+                    });
                 }
             },
 
@@ -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);
+                        }
+                    }]
+                });
             }
         };
     }