Skip to content

Commit

Permalink
Breaking: Reduce false positives by only detecting CJS function-style…
Browse files Browse the repository at this point in the history 
… rules when function returns an object
  • Loading branch information
@bmish
bmish committed Oct 13, 2021
1 parent 5239d69 commit d09645e
Show file tree
Hide file tree
Showing 11 changed files with 53 additions and 43 deletions.
4 changes: 2 additions & 2 deletions lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,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 };
Expand Down
6 changes: 3 additions & 3 deletions tests/lib/rules/no-deprecated-context-methods.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ ruleTester.run('no-deprecated-context-methods', rule, {
`
module.exports = context => {
const sourceCode = context.getSourceCode();
sourceCode.getFirstToken();
return {};
}
`,
],
Expand Down Expand Up @@ -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: [
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/rules/no-deprecated-report-api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
};
`,
`
Expand Down
14 changes: 7 additions & 7 deletions tests/lib/rules/no-missing-placeholders.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
};
`,
`
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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')],
Expand All @@ -203,15 +203,15 @@ 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')],
},
{
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')],
Expand Down
8 changes: 4 additions & 4 deletions tests/lib/rules/no-unused-placeholders.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
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() {};',
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 }],
},
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/report-message-format.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
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() {}
module.exports = function() { 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() {}
module.exports = function() { 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() {}
module.exports = function() { return {}; }
`,
output: null,
options: [{
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/require-meta-fixable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ ruleTester.run('require-meta-fixable', rule, {
create(context) {}
};
`,
'module.exports = context => {};',
'module.exports = context => { return {}; };',
`
module.exports = {
meta: { fixable: 'code' },
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/require-meta-has-suggestions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
40 changes: 25 additions & 15 deletions tests/lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,28 @@ 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: {} }',
'module.exports = { create: foo }',
'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: {} });',
Expand Down Expand Up @@ -90,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<Options, MessageIds>({ create() {}, meta: {} });',
'module.exports = util.createRule<Options, MessageIds>({ create() {}, meta: {} });',
'module.exports = ESLintUtils.RuleCreator(docsUrl)<Options, MessageIds>({ create() {}, meta: {} });',
Expand Down Expand Up @@ -215,22 +225,22 @@ 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,
},
'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,
Expand Down Expand Up @@ -323,7 +333,7 @@ describe('utils', () => {

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,
Expand Down Expand Up @@ -396,7 +406,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()',
Expand Down Expand Up @@ -533,9 +543,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 => {
Expand Down

0 comments on commit d09645e

Please sign in to comment.