diff --git a/lib/utils.js b/lib/utils.js index face10b6..50efe4e4 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,6 +1,7 @@ 'use strict'; const { getStaticValue } = require('eslint-utils'); +const estraverse = require('estraverse'); /** * Determines whether a node is a 'normal' (i.e. non-async, non-generator) function expression. @@ -102,6 +103,24 @@ function collectInterestingProperties (properties, interestingKeys) { }, {}); } +/** + * Check if there is a return statement that returns an object somewhere inside the given node. + * @param {Node} node + * @returns {boolean} + */ +function hasObjectReturn (node) { + let foundMatch = false; + estraverse.traverse(node, { + enter (child) { + if (child.type === 'ReturnStatement' && child.argument && child.argument.type === 'ObjectExpression') { + foundMatch = true; + } + }, + fallback: 'iteration', // Don't crash on unexpected node types. + }); + return foundMatch; +} + /** * Helper for `getRuleInfo`. Handles ESM and TypeScript rules. */ @@ -114,8 +133,8 @@ function getRuleExportsESM (ast) { if (node.type === 'ObjectExpression') { // Check `export default { create() {}, meta: {} }` return collectInterestingProperties(node.properties, INTERESTING_RULE_KEYS); - } else if (isNormalFunctionExpression(node)) { - // Check `export default function() {}` + } else if (isNormalFunctionExpression(node) && hasObjectReturn(node)) { + // Check `export default function() { return { ... }; }` return { create: node, meta: null, isNewStyle: false }; } else if ( node.type === 'CallExpression' && @@ -156,8 +175,8 @@ function getRuleExportsCJS (ast) { node.left.property.type === 'Identifier' && node.left.property.name === 'exports' ) { exportsVarOverridden = true; - if (isNormalFunctionExpression(node.right)) { - // Check `module.exports = function () {}` + if (isNormalFunctionExpression(node.right) && hasObjectReturn(node.right)) { + // Check `module.exports = function () { return {}; }` exportsIsFunction = true; return { create: node.right, meta: null, isNewStyle: false }; diff --git a/package.json b/package.json index 5010fd5f..1dc41b32 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,8 @@ }, "homepage": "https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin#readme", "dependencies": { - "eslint-utils": "^3.0.0" + "eslint-utils": "^3.0.0", + "estraverse": "^5.2.0" }, "nyc": { "branches": 98, @@ -51,7 +52,6 @@ "eslint-plugin-unicorn": "^37.0.0", "eslint-scope": "^6.0.0", "espree": "^9.0.0", - "estraverse": "^5.0.0", "lodash": "^4.17.2", "markdownlint-cli": "^0.28.1", "mocha": "^9.1.2", diff --git a/tests/lib/rules/no-deprecated-context-methods.js b/tests/lib/rules/no-deprecated-context-methods.js index 09d99099..b46c22fb 100644 --- a/tests/lib/rules/no-deprecated-context-methods.js +++ b/tests/lib/rules/no-deprecated-context-methods.js @@ -30,8 +30,8 @@ ruleTester.run('no-deprecated-context-methods', rule, { ` module.exports = context => { const sourceCode = context.getSourceCode(); - sourceCode.getFirstToken(); + return {}; } `, ], @@ -70,12 +70,12 @@ ruleTester.run('no-deprecated-context-methods', rule, { { code: ` module.exports = myRuleContext => { - myRuleContext.getFirstToken; + myRuleContext.getFirstToken; return {}; } `, output: ` module.exports = myRuleContext => { - myRuleContext.getSourceCode().getFirstToken; + myRuleContext.getSourceCode().getFirstToken; return {}; } `, errors: [ diff --git a/tests/lib/rules/no-deprecated-report-api.js b/tests/lib/rules/no-deprecated-report-api.js index d71e3df3..b8a101bb 100644 --- a/tests/lib/rules/no-deprecated-report-api.js +++ b/tests/lib/rules/no-deprecated-report-api.js @@ -39,12 +39,12 @@ ruleTester.run('no-deprecated-report-api', rule, { `, ` module.exports = function(context) { - context.report({node, message: "Foo"}); + context.report({node, message: "Foo"}); return {}; }; `, ` module.exports = (context) => { - context.report({node, message: "Foo"}); + context.report({node, message: "Foo"}); return {}; }; `, ` diff --git a/tests/lib/rules/no-missing-placeholders.js b/tests/lib/rules/no-missing-placeholders.js index 997f236f..9e9129b1 100644 --- a/tests/lib/rules/no-missing-placeholders.js +++ b/tests/lib/rules/no-missing-placeholders.js @@ -84,12 +84,12 @@ ruleTester.run('no-missing-placeholders', rule, { `, ` module.exports = context => { - context.report(node, 'foo {{bar}}', { bar: 'baz' }); + context.report(node, 'foo {{bar}}', { bar: 'baz' }); return {}; }; `, ` module.exports = context => { - context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' }); + context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' }); return {}; }; `, ` @@ -118,14 +118,14 @@ ruleTester.run('no-missing-placeholders', rule, { ` const MESSAGE = 'foo {{bar}}'; module.exports = context => { - context.report(node, MESSAGE, { bar: 'baz' }); + context.report(node, MESSAGE, { bar: 'baz' }); return {}; }; `, // Message in variable but cannot statically determine its type. ` const MESSAGE = getMessage(); module.exports = context => { - context.report(node, MESSAGE, { baz: 'qux' }); + context.report(node, MESSAGE, { baz: 'qux' }); return {}; }; `, // Suggestion with placeholder @@ -193,7 +193,7 @@ ruleTester.run('no-missing-placeholders', rule, { { code: ` module.exports = context => { - context.report(node, 'foo {{bar}}', { baz: 'qux' }); + context.report(node, 'foo {{bar}}', { baz: 'qux' }); return {}; }; `, errors: [error('bar')], @@ -203,7 +203,7 @@ ruleTester.run('no-missing-placeholders', rule, { code: ` const MESSAGE = 'foo {{bar}}'; module.exports = context => { - context.report(node, MESSAGE, { baz: 'qux' }); + context.report(node, MESSAGE, { baz: 'qux' }); return {}; }; `, errors: [error('bar', 'Identifier')], @@ -211,7 +211,7 @@ ruleTester.run('no-missing-placeholders', rule, { { code: ` module.exports = context => { - context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { baz: 'baz' }); + context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { baz: 'baz' }); return {}; }; `, errors: [error('bar')], diff --git a/tests/lib/rules/no-unused-placeholders.js b/tests/lib/rules/no-unused-placeholders.js index 87b6f063..5900eef8 100644 --- a/tests/lib/rules/no-unused-placeholders.js +++ b/tests/lib/rules/no-unused-placeholders.js @@ -85,26 +85,26 @@ ruleTester.run('no-unused-placeholders', rule, { `, ` module.exports = context => { - context.report(node, 'foo {{bar}}', { bar: 'baz' }); + context.report(node, 'foo {{bar}}', { bar: 'baz' }); return {}; }; `, // With message as variable. ` const MESSAGE = 'foo {{bar}}'; module.exports = context => { - context.report(node, MESSAGE, { bar: 'baz' }); + context.report(node, MESSAGE, { bar: 'baz' }); return {}; }; `, // With message as variable but cannot statically determine its type. ` const MESSAGE = getMessage(); module.exports = context => { - context.report(node, MESSAGE, { bar: 'baz' }); + context.report(node, MESSAGE, { bar: 'baz' }); return {}; }; `, ` module.exports = context => { - context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' }); + context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' }); return {}; }; `, // Suggestion diff --git a/tests/lib/rules/prefer-object-rule.js b/tests/lib/rules/prefer-object-rule.js index 77cc6f1e..43661656 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() {};', - output: 'export default {create() {}};', + code: 'export default function create() { return {}; };', + output: 'export default {create() { return {}; }};', parserOptions: { sourceType: 'module' }, errors: [{ messageId: 'preferObject', line: 1, column: 16 }], }, { - code: 'export default () => {};', - output: 'export default {create: () => {}};', + code: 'export default () => { return {}; };', + output: 'export default {create: () => { return {}; }};', parserOptions: { sourceType: 'module' }, errors: [{ messageId: 'preferObject', line: 1, column: 16 }], }, diff --git a/tests/lib/rules/report-message-format.js b/tests/lib/rules/report-message-format.js index 1f637d35..2dae34f1 100644 --- a/tests/lib/rules/report-message-format.js +++ b/tests/lib/rules/report-message-format.js @@ -22,7 +22,7 @@ ruleTester.run('report-message-format', rule, { valid: [ // with no configuration, everything is allowed - 'module.exports = context => context.report(node, "foo");', + 'module.exports = context => { context.report(node, "foo"); return {}; }', { code: ` module.exports = { diff --git a/tests/lib/rules/require-meta-docs-url.js b/tests/lib/rules/require-meta-docs-url.js index 0f755940..ae0aa9b7 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() {} + module.exports = function() { return {}; } `, output: null, errors: [{ messageId: 'missing', type: 'FunctionExpression' }], @@ -310,7 +310,7 @@ tester.run('require-meta-docs-url', rule, { // ------------------------------------------------------------------------- { code: ` - module.exports = function() {} + module.exports = function() { return {}; } `, output: null, options: [{ @@ -492,7 +492,7 @@ tester.run('require-meta-docs-url', rule, { { filename: 'test.js', code: ` - module.exports = function() {} + module.exports = function() { return {}; } `, output: null, options: [{ diff --git a/tests/lib/rules/require-meta-fixable.js b/tests/lib/rules/require-meta-fixable.js index 417cf42d..2d2dd245 100644 --- a/tests/lib/rules/require-meta-fixable.js +++ b/tests/lib/rules/require-meta-fixable.js @@ -25,7 +25,7 @@ ruleTester.run('require-meta-fixable', rule, { create(context) {} }; `, - 'module.exports = context => {};', + 'module.exports = context => { return {}; };', ` module.exports = { meta: { fixable: 'code' }, diff --git a/tests/lib/rules/require-meta-has-suggestions.js b/tests/lib/rules/require-meta-has-suggestions.js index 6218a4a7..d49e5bdc 100644 --- a/tests/lib/rules/require-meta-has-suggestions.js +++ b/tests/lib/rules/require-meta-has-suggestions.js @@ -14,7 +14,7 @@ const RuleTester = require('eslint').RuleTester; const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } }); ruleTester.run('require-meta-has-suggestions', rule, { valid: [ - 'module.exports = context => {};', + 'module.exports = context => { return {}; };', // No suggestions reported, no violations reported, no meta object. ` module.exports = { diff --git a/tests/lib/utils.js b/tests/lib/utils.js index 7dee2dc9..a80d1f84 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() {};', - 'exports = function() {};', - 'module.exports = function* () {};', - 'module.exports = async function () {};', + 'module.boop = function() { return {};};', + 'exports = function() { return {};};', + 'module.exports = function* () { return {}; };', + 'module.exports = async function () { return {};};', 'module.exports = {};', 'module.exports = { meta: {} }', 'module.exports = { create: {} }', @@ -27,7 +27,17 @@ describe('utils', () => { 'module.exports = { create: function* foo() {} }', 'module.exports = { create: async function foo() {} }', - // Correct TypeScript helper structure but missing parameterized types (note: we don't support CJS for TypeScript rules): + // 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 = {};', + + // Correct TypeScript helper structure but we don't support CJS for TypeScript rules: 'module.exports = createESLintRule({ create() {}, meta: {} });', 'module.exports = util.createRule({ create() {}, meta: {} });', 'module.exports = ESLintUtils.RuleCreator(docsUrl)({ create() {}, meta: {} });', @@ -46,6 +56,15 @@ describe('utils', () => { 'export default { foo: {} }', 'const foo = {}; export default foo', + // Exports function but not default export. + 'export function foo () { 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; }', + // Incorrect TypeScript helper structure: 'export default foo()({ create() {}, meta: {} });', 'export default foo().bar({ create() {}, meta: {} });', @@ -81,7 +100,7 @@ describe('utils', () => { describe('the file does not have a valid rule (TypeScript + TypeScript parser + CJS)', () => { [ - // Correct TypeScript helper structure but missing parameterized types (note: we don't support CJS for TypeScript rules): + // Correct TypeScript helper structure but we don't support CJS for TypeScript rules: 'module.exports = createESLintRule({ create() {}, meta: {} });', 'module.exports = util.createRule({ create() {}, meta: {} });', 'module.exports = ESLintUtils.RuleCreator(docsUrl)({ create() {}, meta: {} });', @@ -206,22 +225,27 @@ describe('utils', () => { meta: null, isNewStyle: true, }, - 'module.exports = function foo() {}': { + 'module.exports = function foo() { return {}; }': { create: { type: 'FunctionExpression', id: { name: 'foo' } }, meta: null, isNewStyle: false, }, - 'module.exports = () => {}': { + 'module.exports = () => { return {}; }': { + create: { type: 'ArrowFunctionExpression' }, + meta: null, + isNewStyle: false, + }, + 'module.exports = () => { if (foo) { return {}; } }': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, }, - 'exports.meta = {}; module.exports = () => {}': { + 'exports.meta = {}; module.exports = () => { return {}; }': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, }, - 'module.exports = () => {}; module.exports.meta = {};': { + 'module.exports = () => { return {}; }; module.exports.meta = {};': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, @@ -255,12 +279,17 @@ describe('utils', () => { }, // ESM (function style) - 'export default function () {}': { + 'export default function () { return {}; }': { create: { type: 'FunctionDeclaration' }, meta: null, isNewStyle: false, }, - 'export default () => {}': { + 'export default function () { if (foo) { return {}; } }': { + create: { type: 'FunctionDeclaration' }, + meta: null, + isNewStyle: false, + }, + 'export default () => { return {}; }': { create: { type: 'ArrowFunctionExpression' }, meta: null, isNewStyle: false, @@ -305,11 +334,47 @@ describe('utils', () => { }); } }); + + describe('the file has newer syntax', () => { + const CASES = [ + { + source: 'module.exports = function() { class Foo { @someDecorator() someProp }; return {}; };', + options: { sourceType: 'script' }, + expected: { + create: { type: 'FunctionExpression' }, + meta: null, + isNewStyle: false, + }, + }, + { + source: 'export default function() { class Foo { @someDecorator() someProp }; return {}; };', + options: { sourceType: 'module' }, + expected: { + create: { type: 'FunctionDeclaration' }, + meta: null, + isNewStyle: false, + }, + }, + ]; + for (const testCase of CASES) { + describe(testCase.source, () => { + it('does not throw with node type PropertyDefinition which is not handled by estraverse (estraverse is used for detecting the object return statement in a function-style rule).', () => { + const ast = typescriptEslintParser.parse(testCase.source, testCase.options); + const scopeManager = eslintScope.analyze(ast); + const ruleInfo = utils.getRuleInfo({ ast, scopeManager }); + assert( + lodash.isMatch(ruleInfo, testCase.expected), + `Expected \n${inspect(ruleInfo)}\nto match\n${inspect(testCase.expected)}` + ); + }); + }); + } + }); }); describe('getContextIdentifiers', () => { const CASES = { - 'module.exports = context => { context; context; context; }' (ast) { + 'module.exports = context => { context; context; context; return {}; }' (ast) { return [ ast.body[0].expression.right.body.body[0].expression, ast.body[0].expression.right.body.body[1].expression, @@ -382,7 +447,7 @@ describe('utils', () => { describe('the file does not have valid tests', () => { [ '', - 'module.exports = context => context.report(foo);', + 'module.exports = context => { context.report(foo); return {}; };', 'new (require("eslint").NotRuleTester).run(foo, bar, { valid: [] })', 'new NotRuleTester().run(foo, bar, { valid: [] })', 'new RuleTester()', @@ -519,9 +584,9 @@ describe('utils', () => { describe('getSourceCodeIdentifiers', () => { const CASES = { - 'module.exports = context => { const sourceCode = context.getSourceCode(); sourceCode; foo; }': 2, - 'module.exports = context => { const x = 1, sc = context.getSourceCode(); sc; sc; sc; sourceCode; }': 4, - 'module.exports = context => { const sourceCode = context.getNotSourceCode(); }': 0, + 'module.exports = context => { const sourceCode = context.getSourceCode(); sourceCode; foo; return {}; }': 2, + 'module.exports = context => { const x = 1, sc = context.getSourceCode(); sc; sc; sc; sourceCode; return {}; }': 4, + 'module.exports = context => { const sourceCode = context.getNotSourceCode(); return {}; }': 0, }; Object.keys(CASES).forEach(testSource => {