From 27d5a9e57ad347982a68fcd0e75eafee42d344f0 Mon Sep 17 00:00:00 2001 From: Tanuj Kanti <86398394+Tanujkanti4441@users.noreply.github.com> Date: Sat, 23 Sep 2023 01:35:03 +0530 Subject: [PATCH] feat: add suggestions to array-callback-return (#17590) * feat: add suggestions to array-callback-return * fix: spacing errors * fix: spacing errors * feat: change meta type to problem again * feat: disable eslint-plugin/require-meta-has-suggestions * feat: update tests --- lib/rules/array-callback-return.js | 155 ++++++++++++++--- tests/lib/rules/array-callback-return.js | 202 +++++++++++++++++++++-- 2 files changed, 320 insertions(+), 37 deletions(-) diff --git a/lib/rules/array-callback-return.js b/lib/rules/array-callback-return.js index bda9ab139a6..6d8f258fa14 100644 --- a/lib/rules/array-callback-return.js +++ b/lib/rules/array-callback-return.js @@ -136,6 +136,76 @@ function getArrayMethodName(node) { return null; } +/** + * 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 isExpressionVoid(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} - 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} - An array of fix objects to apply to the node. + */ +function curlyWrapFixer(sourceCode, node, fixer) { + 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 //------------------------------------------------------------------------------ @@ -151,6 +221,9 @@ module.exports = { url: "https://eslint.org/docs/latest/rules/array-callback-return" }, + // eslint-disable-next-line eslint-plugin/require-meta-has-suggestions -- false positive + hasSuggestions: true, + schema: [ { type: "object", @@ -176,7 +249,9 @@ module.exports = { expectedAtEnd: "{{arrayMethodName}}() expects a value to be returned at the end of {{name}}.", expectedInside: "{{arrayMethodName}}() expects a return value from {{name}}.", expectedReturnValue: "{{arrayMethodName}}() expects a return value from {{name}}.", - expectedNoReturnValue: "{{arrayMethodName}}() expects no useless return value from {{name}}." + expectedNoReturnValue: "{{arrayMethodName}}() expects no useless return value from {{name}}.", + wrapBraces: "Wrap the expression in `{}`.", + prependVoid: "Prepend `void` to the expression." } }, @@ -209,32 +284,56 @@ module.exports = { return; } - let messageId = null; + const messageAndSuggestions = { messageId: "", suggest: [] }; if (funcInfo.arrayMethodName === "forEach") { if (options.checkForEach && node.type === "ArrowFunctionExpression" && node.expression) { - if (options.allowVoid && - node.body.type === "UnaryExpression" && - node.body.operator === "void") { - return; - } - messageId = "expectedNoReturnValue"; + if (options.allowVoid) { + if (isExpressionVoid(node.body)) { + return; + } + + messageAndSuggestions.messageId = "expectedNoReturnValue"; + messageAndSuggestions.suggest = [ + { + messageId: "wrapBraces", + fix(fixer) { + return curlyWrapFixer(sourceCode, node, fixer); + } + }, + { + messageId: "prependVoid", + fix(fixer) { + return voidPrependFixer(sourceCode, node.body, fixer); + } + } + ]; + } else { + messageAndSuggestions.messageId = "expectedNoReturnValue"; + messageAndSuggestions.suggest = [{ + messageId: "wrapBraces", + fix(fixer) { + return curlyWrapFixer(sourceCode, node, fixer); + } + }]; + } } } else { if (node.body.type === "BlockStatement" && isAnySegmentReachable(funcInfo.currentSegments)) { - messageId = funcInfo.hasReturn ? "expectedAtEnd" : "expectedInside"; + messageAndSuggestions.messageId = funcInfo.hasReturn ? "expectedAtEnd" : "expectedInside"; } } - if (messageId) { + if (messageAndSuggestions.messageId) { const name = astUtils.getFunctionNameWithKind(node); context.report({ node, loc: astUtils.getFunctionHeadLoc(node, sourceCode), - messageId, - data: { name, arrayMethodName: fullMethodName(funcInfo.arrayMethodName) } + messageId: messageAndSuggestions.messageId, + data: { name, arrayMethodName: fullMethodName(funcInfo.arrayMethodName) }, + suggest: messageAndSuggestions.suggest.length !== 0 ? messageAndSuggestions.suggest : null }); } } @@ -295,36 +394,46 @@ module.exports = { funcInfo.hasReturn = true; - let messageId = null; + const messageAndSuggestions = { messageId: "", suggest: [] }; if (funcInfo.arrayMethodName === "forEach") { // if checkForEach: true, returning a value at any path inside a forEach is not allowed if (options.checkForEach && node.argument) { - if (options.allowVoid && - node.argument.type === "UnaryExpression" && - node.argument.operator === "void") { - return; - } - messageId = "expectedNoReturnValue"; + if (options.allowVoid) { + if (isExpressionVoid(node.argument)) { + return; + } + + messageAndSuggestions.messageId = "expectedNoReturnValue"; + messageAndSuggestions.suggest = [{ + messageId: "prependVoid", + fix(fixer) { + return voidPrependFixer(sourceCode, node.argument, fixer); + } + }]; + } else { + messageAndSuggestions.messageId = "expectedNoReturnValue"; + } } } else { // if allowImplicit: false, should also check node.argument if (!options.allowImplicit && !node.argument) { - messageId = "expectedReturnValue"; + messageAndSuggestions.messageId = "expectedReturnValue"; } } - if (messageId) { + if (messageAndSuggestions.messageId) { context.report({ node, - messageId, + messageId: messageAndSuggestions.messageId, data: { name: astUtils.getFunctionNameWithKind(funcInfo.node), arrayMethodName: fullMethodName(funcInfo.arrayMethodName) - } + }, + suggest: messageAndSuggestions.suggest.length !== 0 ? messageAndSuggestions.suggest : null }); } }, diff --git a/tests/lib/rules/array-callback-return.js b/tests/lib/rules/array-callback-return.js index bc2e8555266..39114dbc582 100644 --- a/tests/lib/rules/array-callback-return.js +++ b/tests/lib/rules/array-callback-return.js @@ -209,10 +209,66 @@ ruleTester.run("array-callback-return", rule, { { code: "foo.forEach(function bar(x) { return x;})", options: allowImplicitCheckForEach, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function 'bar'", arrayMethodName: "Array.prototype.forEach" } }] }, // // options: { checkForEach: true } - { code: "foo.forEach(x => x)", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, - { code: "foo.forEach(val => y += val)", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, - { code: "[\"foo\",\"bar\"].forEach(x => ++x)", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, - { code: "foo.bar().forEach(x => x === y)", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, + { + code: "foo.forEach(x => x)", + options: checkForEachOptions, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach(x => {x})", messageId: "wrapBraces" } + ] + }] + }, + { + code: "foo.forEach(x => (x))", + options: checkForEachOptions, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach(x => {(x)})", messageId: "wrapBraces" } + ] + }] + }, + { + code: "foo.forEach(val => y += val)", + options: checkForEachOptions, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach(val => {y += val})", messageId: "wrapBraces" } + ] + }] + }, + { + code: "[\"foo\",\"bar\"].forEach(x => ++x)", + options: checkForEachOptions, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "[\"foo\",\"bar\"].forEach(x => {++x})", messageId: "wrapBraces" } + ] + }] + }, + { + code: "foo.bar().forEach(x => x === y)", + options: checkForEachOptions, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.bar().forEach(x => {x === y})", messageId: "wrapBraces" } + ] + }] + }, { code: "foo.forEach(function() {return function() { if (a == b) { return a; }}}())", options: checkForEachOptions, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function", arrayMethodName: "Array.prototype.forEach" } }] }, { code: "foo.forEach(function(x) { if (a == b) {return x;}})", options: checkForEachOptions, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function", arrayMethodName: "Array.prototype.forEach" } }] }, { code: "foo.forEach(function(x) { if (a == b) {return undefined;}})", options: checkForEachOptions, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function", arrayMethodName: "Array.prototype.forEach" } }] }, @@ -226,18 +282,136 @@ ruleTester.run("array-callback-return", rule, { { code: "foo.filter(function foo() {})", options: checkForEachOptions, errors: [{ messageId: "expectedInside", data: { name: "function 'foo'", arrayMethodName: "Array.prototype.filter" } }] }, { code: "foo.filter(function foo() { return; })", options: checkForEachOptions, errors: [{ messageId: "expectedReturnValue", data: { name: "function 'foo'", arrayMethodName: "Array.prototype.filter" } }] }, { code: "foo.every(cb || function() {})", options: checkForEachOptions, errors: ["Array.prototype.every() expects a return value from function."] }, - { code: "foo.forEach((x) => void x)", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, - { code: "foo.forEach((x) => void bar(x))", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, - { code: "foo.forEach((x) => { return void bar(x); })", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, - { code: "foo.forEach((x) => { if (a === b) { return void a; } bar(x) })", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, + { code: "foo.forEach((x) => void x)", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, + { code: "foo.forEach((x) => void bar(x))", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, + { code: "foo.forEach((x) => { return void bar(x); })", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, + { code: "foo.forEach((x) => { if (a === b) { return void a; } bar(x) })", options: checkForEachOptions, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, // options: { checkForEach: true, allowVoid: true } - { code: "foo.forEach((x) => x)", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, - { code: "foo.forEach((x) => !x)", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, - { code: "foo.forEach((x) => { return x; })", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, - { code: "foo.forEach((x) => { return !x; })", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, - { code: "foo.forEach((x) => { if (a === b) { return x; } })", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, - { code: "foo.forEach((x) => { if (a === b) { return !x } })", options: checkForEachAllowVoid, parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue" }] }, + + { + code: "foo.forEach(x => x)", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach(x => {x})", messageId: "wrapBraces" }, + { output: "foo.forEach(x => void x)", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach(x => !x)", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach(x => {!x})", messageId: "wrapBraces" }, + { output: "foo.forEach(x => void !x)", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach(x => (x))", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach(x => {(x)})", messageId: "wrapBraces" }, + { output: "foo.forEach(x => void (x))", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach((x) => { return x; })", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach((x) => { return void x; })", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach((x) => { return !x; })", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach((x) => { return void !x; })", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach((x) => { return(x); })", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach((x) => { return void (x); })", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach((x) => { return (x + 1); })", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach((x) => { return void (x + 1); })", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach((x) => { if (a === b) { return x; } })", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach((x) => { if (a === b) { return void x; } })", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach((x) => { if (a === b) { return !x; } })", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach((x) => { if (a === b) { return void !x; } })", messageId: "prependVoid" } + ] + }] + }, + { + code: "foo.forEach((x) => { if (a === b) { return (x + a); } })", + options: checkForEachAllowVoid, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "expectedNoReturnValue", + data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" }, + suggestions: [ + { output: "foo.forEach((x) => { if (a === b) { return void (x + a); } })", messageId: "prependVoid" } + ] + }] + }, // full location tests {