Skip to content

Commit 1bd45d9

Browse files
bmishbmish
authored
Breaking: Reduce false positives by only detecting function-style rules when function returns an object (#211)
1 parent f79f6ac commit 1bd45d9

12 files changed

+133
-49
lines changed

‎lib/utils.js

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const { getStaticValue } = require('eslint-utils');
4+
const estraverse = require('estraverse');
45

56
/**
67
* Determines whether a node is a 'normal' (i.e. non-async, non-generator) function expression.
@@ -102,6 +103,24 @@ function collectInterestingProperties (properties, interestingKeys) {
102103
}, {});
103104
}
104105

106+
/**
107+
* Check if there is a return statement that returns an object somewhere inside the given node.
108+
* @param {Node} node
109+
* @returns {boolean}
110+
*/
111+
function hasObjectReturn (node) {
112+
let foundMatch = false;
113+
estraverse.traverse(node, {
114+
enter (child) {
115+
if (child.type === 'ReturnStatement' && child.argument && child.argument.type === 'ObjectExpression') {
116+
foundMatch = true;
117+
}
118+
},
119+
fallback: 'iteration', // Don't crash on unexpected node types.
120+
});
121+
return foundMatch;
122+
}
123+
105124
/**
106125
* Helper for `getRuleInfo`. Handles ESM and TypeScript rules.
107126
*/
@@ -114,8 +133,8 @@ function getRuleExportsESM (ast) {
114133
if (node.type === 'ObjectExpression') {
115134
// Check `export default { create() {}, meta: {} }`
116135
return collectInterestingProperties(node.properties, INTERESTING_RULE_KEYS);
117-
} else if (isNormalFunctionExpression(node)) {
118-
// Check `export default function() {}`
136+
} else if (isNormalFunctionExpression(node) && hasObjectReturn(node)) {
137+
// Check `export default function() { return { ... }; }`
119138
return { create: node, meta: null, isNewStyle: false };
120139
} else if (
121140
node.type === 'CallExpression' &&
@@ -156,8 +175,8 @@ function getRuleExportsCJS (ast) {
156175
node.left.property.type === 'Identifier' && node.left.property.name === 'exports'
157176
) {
158177
exportsVarOverridden = true;
159-
if (isNormalFunctionExpression(node.right)) {
160-
// Check `module.exports = function () {}`
178+
if (isNormalFunctionExpression(node.right) && hasObjectReturn(node.right)) {
179+
// Check `module.exports = function () { return {}; }`
161180

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

‎package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030
},
3131
"homepage": "https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin#readme",
3232
"dependencies": {
33-
"eslint-utils": "^3.0.0"
33+
"eslint-utils": "^3.0.0",
34+
"estraverse": "^5.2.0"
3435
},
3536
"nyc": {
3637
"branches": 98,
@@ -51,7 +52,6 @@
5152
"eslint-plugin-unicorn": "^37.0.0",
5253
"eslint-scope": "^6.0.0",
5354
"espree": "^9.0.0",
54-
"estraverse": "^5.0.0",
5555
"lodash": "^4.17.2",
5656
"markdownlint-cli": "^0.28.1",
5757
"mocha": "^9.1.2",

‎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' },

0 commit comments

Comments
 (0)