Skip to content

Update fixer-return rule to handle arrow function expressions #144

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

Merged
merged 1 commit into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 30 additions & 9 deletions lib/rules/fixer-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});
}
Expand Down Expand Up @@ -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) &&
Expand Down Expand Up @@ -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);
}
}
},
};
},
};
150 changes: 117 additions & 33 deletions tests/lib/rules/fixer-return.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* @fileoverview enforces always return from a fixer function
* @fileoverview Require fixer function to return a fix
* @author 薛定谔的猫<[email protected]>
*/

Expand All @@ -12,8 +12,6 @@
const rule = require('../../../lib/rules/fixer-return');
const RuleTester = require('eslint').RuleTester;

const ERROR = { messageId: 'missingFix', type: 'FunctionExpression' };

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -316,7 +400,7 @@ ruleTester.run('fixer-return', rule, {
}
};
`,
errors: [ERROR],
errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 26 }],
},
],
});