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 ea7383e
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 30 deletions.
2 changes: 1 addition & 1 deletion lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ function getRuleExportsCJS (ast) {
node.left.property.type === 'Identifier' && node.left.property.name === 'exports'
) {
exportsVarOverridden = true;
if (isNormalFunctionExpression(node.right)) {
if (isNormalFunctionExpression(node.right) && hasObjectReturn(node.right)) {
// Check `module.exports = function () {}`

exportsIsFunction = true;
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/rules/no-deprecated-context-methods.js
View file
Original file line number Diff line number Diff line change
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
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
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
28 changes: 19 additions & 9 deletions tests/lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {} });',
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 @@ -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 ea7383e

Please sign in to comment.