Skip to content

Commit

Permalink
Fix: Only consider functions with a single argument as function-style…
Browse files Browse the repository at this point in the history 
… rules
  • Loading branch information
@bmish
bmish committed Oct 14, 2021
1 parent 1bd45d9 commit daa6039
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 41 deletions.
21 changes: 17 additions & 4 deletions lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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' &&
Expand Down Expand Up @@ -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 };
Expand Down
8 changes: 4 additions & 4 deletions tests/lib/rules/prefer-object-rule.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }],
},
Expand Down
6 changes: 3 additions & 3 deletions tests/lib/rules/require-meta-docs-url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' }],
Expand Down Expand Up @@ -310,7 +310,7 @@ tester.run('require-meta-docs-url', rule, {
// -------------------------------------------------------------------------
{
code: `
module.exports = function() { return {}; }
module.exports = function(context) { return {}; }
`,
output: null,
options: [{
Expand Down Expand Up @@ -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: [{
Expand Down
78 changes: 48 additions & 30 deletions tests/lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {} }',
Expand All @@ -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: {} });',
Expand All @@ -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: {} });',
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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 });
Expand All @@ -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' },
Expand All @@ -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' },
Expand Down

0 comments on commit daa6039

Please sign in to comment.