From 238acd84cedfa90f0118054595aa9d74498010ea Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Wed, 23 Jun 2021 11:02:21 -0400 Subject: [PATCH] fix: update fixer-return rule to handle arrow function expressions --- lib/rules/fixer-return.js | 39 +++++++-- tests/lib/rules/fixer-return.js | 150 +++++++++++++++++++++++++------- 2 files changed, 147 insertions(+), 42 deletions(-) diff --git a/lib/rules/fixer-return.js b/lib/rules/fixer-return.js index e9113c95..5ea482fc 100644 --- a/lib/rules/fixer-return.js +++ b/lib/rules/fixer-return.js @@ -47,19 +47,17 @@ module.exports = { * If not, report the function as a violation. * * @param {ASTNode} node - A node to check. + * @param {Location} loc - Optional location to report violation on. * @returns {void} */ - function checkLastSegment (node) { + function ensureFunctionReturnedFix (node, loc = (node.id || node).loc.start) { if ( - funcInfo.shouldCheck && - ( - (node.generator && !funcInfo.hasYieldWithFixer) || // Generator function never yielded a fix - (!node.generator && !funcInfo.hasReturnWithFixer) // Non-generator function never returned a fix - ) + (node.generator && !funcInfo.hasYieldWithFixer) || // Generator function never yielded a fix + (!node.generator && !funcInfo.hasReturnWithFixer) // Non-generator function never returned a fix ) { context.report({ node, - loc: (node.id || node).loc.start, + loc, messageId: 'missingFix', }); } @@ -103,7 +101,7 @@ module.exports = { const parent = node.parent; // Whether we are inside the fixer function we care about. - const shouldCheck = node.type === 'FunctionExpression' && + const shouldCheck = ['FunctionExpression', 'ArrowFunctionExpression'].includes(node.type) && parent.parent.type === 'ObjectExpression' && parent.parent.parent.type === 'CallExpression' && contextIdentifiers.has(parent.parent.parent.callee.object) && @@ -140,7 +138,30 @@ module.exports = { }, // Ensure the current fixer function returned or yielded a fix. - 'FunctionExpression:exit': checkLastSegment, + 'FunctionExpression:exit' (node) { + if (funcInfo.shouldCheck) { + ensureFunctionReturnedFix(node); + } + }, + + // Ensure the current (arrow) fixer function returned a fix. + 'ArrowFunctionExpression:exit' (node) { + if (funcInfo.shouldCheck) { + const loc = context.getSourceCode().getTokenBefore(node.body).loc; // Show violation on arrow (=>). + if (node.expression) { + // When the return is implied (no curly braces around the body), we have to check the single body node directly. + if (!isFix(node.body)) { + context.report({ + node, + loc, + messageId: 'missingFix', + }); + } + } else { + ensureFunctionReturnedFix(node, loc); + } + } + }, }; }, }; diff --git a/tests/lib/rules/fixer-return.js b/tests/lib/rules/fixer-return.js index 69ed5d39..c9db0b9f 100644 --- a/tests/lib/rules/fixer-return.js +++ b/tests/lib/rules/fixer-return.js @@ -1,5 +1,5 @@ /** - * @fileoverview enforces always return from a fixer function + * @fileoverview Require fixer function to return a fix * @author 薛定谔的猫 */ @@ -12,8 +12,6 @@ const rule = require('../../../lib/rules/fixer-return'); const RuleTester = require('eslint').RuleTester; -const ERROR = { messageId: 'missingFix', type: 'FunctionExpression' }; - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ @@ -46,6 +44,38 @@ ruleTester.run('fixer-return', rule, { } }; `, + // Not the right fix function. + ` + module.exports = { + create: function(context) { + context.report( { + notFix: function(fixer) { + } + }); + } + }; + `, + // Not the right fix function (arrow function with implied return) + ` + module.exports = { + create: function(context) { + context.report( { + notFix: fixer => undefined + }); + } + }; + `, + // Not the right fix function (arrow function) + ` + module.exports = { + create: function(context) { + context.report( { + notFix: fixer => {} + }); + } + }; + `, + // Arrow function (expression) ` module.exports = { create: function(context) { @@ -55,6 +85,17 @@ ruleTester.run('fixer-return', rule, { } }; `, + // Arrow function (with return) + ` + module.exports = { + create: function(context) { + context.report({ + fix: fixer => {return fixer.foo();} + }); + } + }; + `, + // Generator ` module.exports = { create: function (context) { @@ -196,7 +237,50 @@ ruleTester.run('fixer-return', rule, { } }; `, - errors: [ERROR], + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 24 }], + }, + { + // Fix but missing return (arrow function, report on arrow) + code: ` + module.exports = { + create: function(context) { + context.report({ + fix: (fixer) => { + fixer.foo(); + } + }); + } + }; + `, + errors: [{ messageId: 'missingFix', type: 'ArrowFunctionExpression', line: 5, endLine: 5, column: 34, endColumn: 36 }], + }, + { + // With no autofix (arrow function, explicit return, report on arrow) + code: ` + module.exports = { + create: function(context) { + context.report({ + fix: (fixer) => { + return undefined; + } + }); + } + }; + `, + errors: [{ messageId: 'missingFix', type: 'ArrowFunctionExpression', line: 5, endLine: 5, column: 34, endColumn: 36 }], + }, + { + // With no autofix (arrow function, implied return, report on arrow) + code: ` + module.exports = { + create: function(context) { + context.report( { + fix: fixer => undefined + }); + } + }; + `, + errors: [{ messageId: 'missingFix', type: 'ArrowFunctionExpression', line: 5, endLine: 5, column: 32, endColumn: 34 }], }, { // Fix but missing yield (generator) @@ -211,22 +295,22 @@ ruleTester.run('fixer-return', rule, { } }; `, - errors: [ERROR], + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 25 }], }, { // With no autofix (only yield undefined) code: ` - module.exports = { - create: function(context) { - context.report({ - *fix(fixer) { - yield undefined; - } - }); - } - }; - `, - errors: [ERROR], + module.exports = { + create: function(context) { + context.report({ + *fix(fixer) { + yield undefined; + } + }); + } + }; + `, + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 25 }], }, { // With no autofix (only return null) @@ -241,7 +325,7 @@ ruleTester.run('fixer-return', rule, { } }; `, - errors: [ERROR], + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 26 }], }, { // With no autofix (only return undefined) @@ -256,23 +340,23 @@ ruleTester.run('fixer-return', rule, { } }; `, - errors: [ERROR], + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 26 }], }, { // With no autofix (only return undefined, but in variable) code: ` - module.exports = { - create: function(context) { - context.report( { - fix: function(fixer) { - const returnValue = undefined; - return returnValue; - } - }); - } - }; - `, - errors: [ERROR], + module.exports = { + create: function(context) { + context.report( { + fix: function(fixer) { + const returnValue = undefined; + return returnValue; + } + }); + } + }; + `, + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 26 }], }, { // With no autofix (only return implicit undefined) @@ -287,7 +371,7 @@ ruleTester.run('fixer-return', rule, { } }; `, - errors: [ERROR], + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 26 }], }, { // With no autofix (only return empty array) @@ -302,7 +386,7 @@ ruleTester.run('fixer-return', rule, { } }; `, - errors: [ERROR], + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 26 }], }, { // With no autofix (no return, empty function) @@ -316,7 +400,7 @@ ruleTester.run('fixer-return', rule, { } }; `, - errors: [ERROR], + errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 26 }], }, ], });