Skip to content

Commit d09645e

Browse files
bmishbmish
committed
Breaking: Reduce false positives by only detecting CJS function-style rules when function returns an object
1 parent 5239d69 commit d09645e

11 files changed

+53
-43
lines changed

‎lib/utils.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ function getRuleExportsCJS (ast) {
174174
node.left.property.type === 'Identifier' && node.left.property.name === 'exports'
175175
) {
176176
exportsVarOverridden = true;
177-
if (isNormalFunctionExpression(node.right)) {
178-
// Check `module.exports = function () {}`
177+
if (isNormalFunctionExpression(node.right) && hasObjectReturn(node.right)) {
178+
// Check `module.exports = function () { return {}; }`
179179

180180
exportsIsFunction = true;
181181
return { create: node.right, meta: null, isNewStyle: false };

‎tests/lib/rules/no-deprecated-context-methods.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ ruleTester.run('no-deprecated-context-methods', rule, {
3030
`
3131
module.exports = context => {
3232
const sourceCode = context.getSourceCode();
33-
3433
sourceCode.getFirstToken();
34+
return {};
3535
}
3636
`,
3737
],
@@ -70,12 +70,12 @@ ruleTester.run('no-deprecated-context-methods', rule, {
7070
{
7171
code: `
7272
module.exports = myRuleContext => {
73-
myRuleContext.getFirstToken;
73+
myRuleContext.getFirstToken; return {};
7474
}
7575
`,
7676
output: `
7777
module.exports = myRuleContext => {
78-
myRuleContext.getSourceCode().getFirstToken;
78+
myRuleContext.getSourceCode().getFirstToken; return {};
7979
}
8080
`,
8181
errors: [

‎tests/lib/rules/no-deprecated-report-api.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ ruleTester.run('no-deprecated-report-api', rule, {
3939
`,
4040
`
4141
module.exports = function(context) {
42-
context.report({node, message: "Foo"});
42+
context.report({node, message: "Foo"}); return {};
4343
};
4444
`,
4545
`
4646
module.exports = (context) => {
47-
context.report({node, message: "Foo"});
47+
context.report({node, message: "Foo"}); return {};
4848
};
4949
`,
5050
`

‎tests/lib/rules/no-missing-placeholders.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ ruleTester.run('no-missing-placeholders', rule, {
8484
`,
8585
`
8686
module.exports = context => {
87-
context.report(node, 'foo {{bar}}', { bar: 'baz' });
87+
context.report(node, 'foo {{bar}}', { bar: 'baz' }); return {};
8888
};
8989
`,
9090
`
9191
module.exports = context => {
92-
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' });
92+
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' }); return {};
9393
};
9494
`,
9595
`
@@ -118,14 +118,14 @@ ruleTester.run('no-missing-placeholders', rule, {
118118
`
119119
const MESSAGE = 'foo {{bar}}';
120120
module.exports = context => {
121-
context.report(node, MESSAGE, { bar: 'baz' });
121+
context.report(node, MESSAGE, { bar: 'baz' }); return {};
122122
};
123123
`,
124124
// Message in variable but cannot statically determine its type.
125125
`
126126
const MESSAGE = getMessage();
127127
module.exports = context => {
128-
context.report(node, MESSAGE, { baz: 'qux' });
128+
context.report(node, MESSAGE, { baz: 'qux' }); return {};
129129
};
130130
`,
131131
// Suggestion with placeholder
@@ -193,7 +193,7 @@ ruleTester.run('no-missing-placeholders', rule, {
193193
{
194194
code: `
195195
module.exports = context => {
196-
context.report(node, 'foo {{bar}}', { baz: 'qux' });
196+
context.report(node, 'foo {{bar}}', { baz: 'qux' }); return {};
197197
};
198198
`,
199199
errors: [error('bar')],
@@ -203,15 +203,15 @@ ruleTester.run('no-missing-placeholders', rule, {
203203
code: `
204204
const MESSAGE = 'foo {{bar}}';
205205
module.exports = context => {
206-
context.report(node, MESSAGE, { baz: 'qux' });
206+
context.report(node, MESSAGE, { baz: 'qux' }); return {};
207207
};
208208
`,
209209
errors: [error('bar', 'Identifier')],
210210
},
211211
{
212212
code: `
213213
module.exports = context => {
214-
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { baz: 'baz' });
214+
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { baz: 'baz' }); return {};
215215
};
216216
`,
217217
errors: [error('bar')],

‎tests/lib/rules/no-unused-placeholders.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,26 +85,26 @@ ruleTester.run('no-unused-placeholders', rule, {
8585
`,
8686
`
8787
module.exports = context => {
88-
context.report(node, 'foo {{bar}}', { bar: 'baz' });
88+
context.report(node, 'foo {{bar}}', { bar: 'baz' }); return {};
8989
};
9090
`,
9191
// With message as variable.
9292
`
9393
const MESSAGE = 'foo {{bar}}';
9494
module.exports = context => {
95-
context.report(node, MESSAGE, { bar: 'baz' });
95+
context.report(node, MESSAGE, { bar: 'baz' }); return {};
9696
};
9797
`,
9898
// With message as variable but cannot statically determine its type.
9999
`
100100
const MESSAGE = getMessage();
101101
module.exports = context => {
102-
context.report(node, MESSAGE, { bar: 'baz' });
102+
context.report(node, MESSAGE, { bar: 'baz' }); return {};
103103
};
104104
`,
105105
`
106106
module.exports = context => {
107-
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' });
107+
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' }); return {};
108108
};
109109
`,
110110
// Suggestion

‎tests/lib/rules/prefer-object-rule.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,14 +132,14 @@ ruleTester.run('prefer-object-rule', rule, {
132132
errors: [{ messageId: 'preferObject', line: 2, column: 24 }],
133133
},
134134
{
135-
code: 'export default function create() {};',
136-
output: 'export default {create() {}};',
135+
code: 'export default function create() { return {}; };',
136+
output: 'export default {create() { return {}; }};',
137137
parserOptions: { sourceType: 'module' },
138138
errors: [{ messageId: 'preferObject', line: 1, column: 16 }],
139139
},
140140
{
141-
code: 'export default () => {};',
142-
output: 'export default {create: () => {}};',
141+
code: 'export default () => { return {}; };',
142+
output: 'export default {create: () => { return {}; }};',
143143
parserOptions: { sourceType: 'module' },
144144
errors: [{ messageId: 'preferObject', line: 1, column: 16 }],
145145
},

‎tests/lib/rules/report-message-format.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ ruleTester.run('report-message-format', rule, {
2222

2323
valid: [
2424
// with no configuration, everything is allowed
25-
'module.exports = context => context.report(node, "foo");',
25+
'module.exports = context => { context.report(node, "foo"); return {}; }',
2626
{
2727
code: `
2828
module.exports = {

‎tests/lib/rules/require-meta-docs-url.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ tester.run('require-meta-docs-url', rule, {
125125
invalid: [
126126
{
127127
code: `
128-
module.exports = function() {}
128+
module.exports = function() { return {}; }
129129
`,
130130
output: null,
131131
errors: [{ messageId: 'missing', type: 'FunctionExpression' }],
@@ -310,7 +310,7 @@ tester.run('require-meta-docs-url', rule, {
310310
// -------------------------------------------------------------------------
311311
{
312312
code: `
313-
module.exports = function() {}
313+
module.exports = function() { return {}; }
314314
`,
315315
output: null,
316316
options: [{
@@ -492,7 +492,7 @@ tester.run('require-meta-docs-url', rule, {
492492
{
493493
filename: 'test.js',
494494
code: `
495-
module.exports = function() {}
495+
module.exports = function() { return {}; }
496496
`,
497497
output: null,
498498
options: [{

‎tests/lib/rules/require-meta-fixable.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ ruleTester.run('require-meta-fixable', rule, {
2525
create(context) {}
2626
};
2727
`,
28-
'module.exports = context => {};',
28+
'module.exports = context => { return {}; };',
2929
`
3030
module.exports = {
3131
meta: { fixable: 'code' },

‎tests/lib/rules/require-meta-has-suggestions.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const RuleTester = require('eslint').RuleTester;
1414
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
1515
ruleTester.run('require-meta-has-suggestions', rule, {
1616
valid: [
17-
'module.exports = context => {};',
17+
'module.exports = context => { return {}; };',
1818
// No suggestions reported, no violations reported, no meta object.
1919
`
2020
module.exports = {

‎tests/lib/utils.js

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,28 @@ describe('utils', () => {
1616
'',
1717
'module.exports;',
1818
'module.exports = foo;',
19-
'module.boop = function() {};',
20-
'exports = function() {};',
21-
'module.exports = function* () {};',
22-
'module.exports = async function () {};',
19+
'module.boop = function() { return {};};',
20+
'exports = function() { return {};};',
21+
'module.exports = function* () { return {}; };',
22+
'module.exports = async function () { return {};};',
2323
'module.exports = {};',
2424
'module.exports = { meta: {} }',
2525
'module.exports = { create: {} }',
2626
'module.exports = { create: foo }',
2727
'module.exports = { create: function* foo() {} }',
2828
'module.exports = { create: async function foo() {} }',
2929

30-
// Correct TypeScript helper structure but missing parameterized types (note: we don't support CJS for TypeScript rules):
30+
// Function-style rule but missing object return.
31+
'module.exports = () => { }',
32+
'module.exports = () => { return; }',
33+
'module.exports = () => { return 123; }',
34+
'module.exports = () => { return FOO; }',
35+
'module.exports = function foo() { }',
36+
'module.exports = () => { }',
37+
'exports.meta = {}; module.exports = () => { }',
38+
'module.exports = () => { }; module.exports.meta = {};',
39+
40+
// Correct TypeScript helper structure but we don't support CJS for TypeScript rules:
3141
'module.exports = createESLintRule({ create() {}, meta: {} });',
3242
'module.exports = util.createRule({ create() {}, meta: {} });',
3343
'module.exports = ESLintUtils.RuleCreator(docsUrl)({ create() {}, meta: {} });',
@@ -90,7 +100,7 @@ describe('utils', () => {
90100

91101
describe('the file does not have a valid rule (TypeScript + TypeScript parser + CJS)', () => {
92102
[
93-
// Correct TypeScript helper structure but missing parameterized types (note: we don't support CJS for TypeScript rules):
103+
// Correct TypeScript helper structure but we don't support CJS for TypeScript rules):
94104
'module.exports = createESLintRule<Options, MessageIds>({ create() {}, meta: {} });',
95105
'module.exports = util.createRule<Options, MessageIds>({ create() {}, meta: {} });',
96106
'module.exports = ESLintUtils.RuleCreator(docsUrl)<Options, MessageIds>({ create() {}, meta: {} });',
@@ -215,22 +225,22 @@ describe('utils', () => {
215225
meta: null,
216226
isNewStyle: true,
217227
},
218-
'module.exports = function foo() {}': {
228+
'module.exports = function foo() { return {}; }': {
219229
create: { type: 'FunctionExpression', id: { name: 'foo' } },
220230
meta: null,
221231
isNewStyle: false,
222232
},
223-
'module.exports = () => {}': {
233+
'module.exports = () => { return {}; }': {
224234
create: { type: 'ArrowFunctionExpression' },
225235
meta: null,
226236
isNewStyle: false,
227237
},
228-
'exports.meta = {}; module.exports = () => {}': {
238+
'exports.meta = {}; module.exports = () => { return {}; }': {
229239
create: { type: 'ArrowFunctionExpression' },
230240
meta: null,
231241
isNewStyle: false,
232242
},
233-
'module.exports = () => {}; module.exports.meta = {};': {
243+
'module.exports = () => { return {}; }; module.exports.meta = {};': {
234244
create: { type: 'ArrowFunctionExpression' },
235245
meta: null,
236246
isNewStyle: false,
@@ -323,7 +333,7 @@ describe('utils', () => {
323333

324334
describe('getContextIdentifiers', () => {
325335
const CASES = {
326-
'module.exports = context => { context; context; context; }' (ast) {
336+
'module.exports = context => { context; context; context; return {}; }' (ast) {
327337
return [
328338
ast.body[0].expression.right.body.body[0].expression,
329339
ast.body[0].expression.right.body.body[1].expression,
@@ -396,7 +406,7 @@ describe('utils', () => {
396406
describe('the file does not have valid tests', () => {
397407
[
398408
'',
399-
'module.exports = context => context.report(foo);',
409+
'module.exports = context => { context.report(foo); return {}; };',
400410
'new (require("eslint").NotRuleTester).run(foo, bar, { valid: [] })',
401411
'new NotRuleTester().run(foo, bar, { valid: [] })',
402412
'new RuleTester()',
@@ -533,9 +543,9 @@ describe('utils', () => {
533543

534544
describe('getSourceCodeIdentifiers', () => {
535545
const CASES = {
536-
'module.exports = context => { const sourceCode = context.getSourceCode(); sourceCode; foo; }': 2,
537-
'module.exports = context => { const x = 1, sc = context.getSourceCode(); sc; sc; sc; sourceCode; }': 4,
538-
'module.exports = context => { const sourceCode = context.getNotSourceCode(); }': 0,
546+
'module.exports = context => { const sourceCode = context.getSourceCode(); sourceCode; foo; return {}; }': 2,
547+
'module.exports = context => { const x = 1, sc = context.getSourceCode(); sc; sc; sc; sourceCode; return {}; }': 4,
548+
'module.exports = context => { const sourceCode = context.getNotSourceCode(); return {}; }': 0,
539549
};
540550

541551
Object.keys(CASES).forEach(testSource => {

0 commit comments

Comments
 (0)