From daa60397ba15c27c8252b8db2b3c50a29352015c Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Thu, 14 Oct 2021 12:27:25 -0400 Subject: [PATCH] Fix: Only consider functions with a single argument as function-style rules --- lib/utils.js | 21 +++++-- tests/lib/rules/prefer-object-rule.js | 8 +-- tests/lib/rules/require-meta-docs-url.js | 6 +- tests/lib/utils.js | 78 +++++++++++++++--------- 4 files changed, 72 insertions(+), 41 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 50efe4e4..77779e7c 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -121,6 +121,19 @@ function hasObjectReturn (node) { return foundMatch; } +/** + * Determine if the given node is likely to be a function-style rule. + * @param {*} node + * @returns {boolean} + */ +function isFunctionRule (node) { + return ( + isNormalFunctionExpression(node) && // Is a function definition. + node.params.length === 1 && // The function has a single `context` argument. + hasObjectReturn(node) // Returns an object containing the visitor functions. + ); +} + /** * Helper for `getRuleInfo`. Handles ESM and TypeScript rules. */ @@ -133,8 +146,8 @@ function getRuleExportsESM (ast) { if (node.type === 'ObjectExpression') { // Check `export default { create() {}, meta: {} }` return collectInterestingProperties(node.properties, INTERESTING_RULE_KEYS); - } else if (isNormalFunctionExpression(node) && hasObjectReturn(node)) { - // Check `export default function() { return { ... }; }` + } else if (isFunctionRule(node)) { + // Check `export default function(context) { return { ... }; }` return { create: node, meta: null, isNewStyle: false }; } else if ( node.type === 'CallExpression' && @@ -175,8 +188,8 @@ function getRuleExportsCJS (ast) { node.left.property.type === 'Identifier' && node.left.property.name === 'exports' ) { exportsVarOverridden = true; - if (isNormalFunctionExpression(node.right) && hasObjectReturn(node.right)) { - // Check `module.exports = function () { return {}; }` + if (isFunctionRule(node.right)) { + // Check `module.exports = function (context) { return { ... }; }` exportsIsFunction = true; return { create: node.right, meta: null, isNewStyle: false }; diff --git a/tests/lib/rules/prefer-object-rule.js b/tests/lib/rules/prefer-object-rule.js index 43661656..d7d7d78d 100644 --- a/tests/lib/rules/prefer-object-rule.js +++ b/tests/lib/rules/prefer-object-rule.js @@ -132,14 +132,14 @@ ruleTester.run('prefer-object-rule', rule, { errors: [{ messageId: 'preferObject', line: 2, column: 24 }], }, { - code: 'export default function create() { return {}; };', - output: 'export default {create() { return {}; }};', + code: 'export default function create(context) { return {}; };', + output: 'export default {create(context) { return {}; }};', parserOptions: { sourceType: 'module' }, errors: [{ messageId: 'preferObject', line: 1, column: 16 }], }, { - code: 'export default () => { return {}; };', - output: 'export default {create: () => { return {}; }};', + code: 'export default (context) => { return {}; };', + output: 'export default {create: (context) => { return {}; }};', parserOptions: { sourceType: 'module' }, errors: [{ messageId: 'preferObject', line: 1, column: 16 }], }, diff --git a/tests/lib/rules/require-meta-docs-url.js b/tests/lib/rules/require-meta-docs-url.js index ae0aa9b7..f7be9a8c 100644 --- a/tests/lib/rules/require-meta-docs-url.js +++ b/tests/lib/rules/require-meta-docs-url.js @@ -125,7 +125,7 @@ tester.run('require-meta-docs-url', rule, { invalid: [ { code: ` - module.exports = function() { return {}; } + module.exports = function(context) { return {}; } `, output: null, errors: [{ messageId: 'missing', type: 'FunctionExpression' }], @@ -310,7 +310,7 @@ tester.run('require-meta-docs-url', rule, { // ------------------------------------------------------------------------- { code: ` - module.exports = function() { return {}; } + module.exports = function(context) { return {}; } `, output: null, options: [{ @@ -492,7 +492,7 @@ tester.run('require-meta-docs-url', rule, { { filename: 'test.js', code: ` - module.exports = function() { return {}; } + module.exports = function(context) { return {}; } `, output: null, options: [{ diff --git a/tests/lib/utils.js b/tests/lib/utils.js index a80d1f84..e4d9240e 100644 --- a/tests/lib/utils.js +++ b/tests/lib/utils.js @@ -16,10 +16,10 @@ describe('utils', () => { '', 'module.exports;', 'module.exports = foo;', - 'module.boop = function() { return {};};', - 'exports = function() { return {};};', - 'module.exports = function* () { return {}; };', - 'module.exports = async function () { return {};};', + 'module.boop = function(context) { return {};};', + 'exports = function(context) { return {};};', + 'module.exports = function* (context) { return {}; };', + 'module.exports = async function (context) { return {};};', 'module.exports = {};', 'module.exports = { meta: {} }', 'module.exports = { create: {} }', @@ -28,14 +28,18 @@ describe('utils', () => { 'module.exports = { create: async function foo() {} }', // Function-style rule but missing object return. - 'module.exports = () => { }', - 'module.exports = () => { return; }', - 'module.exports = () => { return 123; }', - 'module.exports = () => { return FOO; }', - 'module.exports = function foo() { }', - 'module.exports = () => { }', - 'exports.meta = {}; module.exports = () => { }', - 'module.exports = () => { }; module.exports.meta = {};', + 'module.exports = (context) => { }', + 'module.exports = (context) => { return; }', + 'module.exports = (context) => { return 123; }', + 'module.exports = (context) => { return FOO; }', + 'module.exports = function foo(context) { }', + 'module.exports = (context) => { }', + 'exports.meta = {}; module.exports = (context) => { }', + 'module.exports = (context) => { }; module.exports.meta = {};', + + // Function-style rule but missing context parameter. + 'module.exports = () => { return {}; }', + 'module.exports = (foo, bar) => { return {}; }', // Correct TypeScript helper structure but we don't support CJS for TypeScript rules: 'module.exports = createESLintRule({ create() {}, meta: {} });', @@ -57,13 +61,17 @@ describe('utils', () => { 'const foo = {}; export default foo', // Exports function but not default export. - 'export function foo () { return {}; }', + 'export function foo (context) { return {}; }', // Exports function but no object return inside function. - 'export default function () { }', - 'export default function () { return; }', - 'export default function () { return 123; }', - 'export default function () { return FOO; }', + 'export default function (context) { }', + 'export default function (context) { return; }', + 'export default function (context) { return 123; }', + 'export default function (context) { return FOO; }', + + // Function-style rule but missing context parameter. + 'export default function () { return {}; }', + 'export default function (foo, bar) { return {}; }', // Incorrect TypeScript helper structure: 'export default foo()({ create() {}, meta: {} });', @@ -185,7 +193,7 @@ describe('utils', () => { meta: { type: 'ObjectExpression' }, isNewStyle: true, }, - 'module.exports.create = function foo() {}; module.exports.meta = {}': { + 'module.exports.create = function foo(context) {}; module.exports.meta = {}': { create: { type: 'FunctionExpression', id: { name: 'foo' } }, meta: { type: 'ObjectExpression' }, isNewStyle: true, @@ -220,32 +228,42 @@ describe('utils', () => { meta: { type: 'ObjectExpression' }, isNewStyle: true, }, - 'module.exports = { create: () => { } }; exports.meta = {};': { + 'module.exports = { create: (context) => { } }; exports.meta = {};': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: true, }, - 'module.exports = function foo() { return {}; }': { + 'module.exports = function foo(context) { return {}; }': { + create: { type: 'FunctionExpression', id: { name: 'foo' } }, + meta: null, + isNewStyle: false, + }, + 'module.exports = function foo(slightlyDifferentContextName) { return {}; }': { + create: { type: 'FunctionExpression', id: { name: 'foo' } }, + meta: null, + isNewStyle: false, + }, + 'module.exports = function foo({ report }) { return {}; }': { create: { type: 'FunctionExpression', id: { name: 'foo' } }, meta: null, isNewStyle: false, }, - 'module.exports = () => { return {}; }': { + 'module.exports = (context) => { return {}; }': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, }, - 'module.exports = () => { if (foo) { return {}; } }': { + 'module.exports = (context) => { if (foo) { return {}; } }': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, }, - 'exports.meta = {}; module.exports = () => { return {}; }': { + 'exports.meta = {}; module.exports = (context) => { return {}; }': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, }, - 'module.exports = () => { return {}; }; module.exports.meta = {};': { + 'module.exports = (context) => { return {}; }; module.exports.meta = {};': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, @@ -279,17 +297,17 @@ describe('utils', () => { }, // ESM (function style) - 'export default function () { return {}; }': { + 'export default function (context) { return {}; }': { create: { type: 'FunctionDeclaration' }, meta: null, isNewStyle: false, }, - 'export default function () { if (foo) { return {}; } }': { + 'export default function (context) { if (foo) { return {}; } }': { create: { type: 'FunctionDeclaration' }, meta: null, isNewStyle: false, }, - 'export default () => { return {}; }': { + 'export default (context) => { return {}; }': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, @@ -315,7 +333,7 @@ describe('utils', () => { { ignoreEval: true, ecmaVersion: 6, sourceType: 'module' }, ]) { const ast = espree.parse(` - const create = () => {}; + const create = (context) => {}; const meta = {}; module.exports = { create, meta }; `, { ecmaVersion: 6 }); @@ -338,7 +356,7 @@ describe('utils', () => { describe('the file has newer syntax', () => { const CASES = [ { - source: 'module.exports = function() { class Foo { @someDecorator() someProp }; return {}; };', + source: 'module.exports = function(context) { class Foo { @someDecorator() someProp }; return {}; };', options: { sourceType: 'script' }, expected: { create: { type: 'FunctionExpression' }, @@ -347,7 +365,7 @@ describe('utils', () => { }, }, { - source: 'export default function() { class Foo { @someDecorator() someProp }; return {}; };', + source: 'export default function(context) { class Foo { @someDecorator() someProp }; return {}; };', options: { sourceType: 'module' }, expected: { create: { type: 'FunctionDeclaration' },