Skip to content

Commit ea7383e

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

7 files changed

+40
-30
lines changed

‎lib/utils.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ function getRuleExportsCJS (ast) {
174174
node.left.property.type === 'Identifier' && node.left.property.name === 'exports'
175175
) {
176176
exportsVarOverridden = true;
177-
if (isNormalFunctionExpression(node.right)) {
177+
if (isNormalFunctionExpression(node.right) && hasObjectReturn(node.right)) {
178178
// Check `module.exports = function () {}`
179179

180180
exportsIsFunction = true;

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -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-missing-placeholders.js

+7-7
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

+4-4
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

+4-4
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/require-meta-docs-url.js

+3-3
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/utils.js

+19-9
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,17 @@ describe('utils', () => {
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,
@@ -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)