Skip to content

Commit 5159c24

Browse files
fiskerfisker
authored
Ignore more types in no-fn-reference-in-iterator and no-reduce rule (#756)
1 parent 6d36407 commit 5159c24

File tree

6 files changed

+135
-45
lines changed

6 files changed

+135
-45
lines changed

‎rules/no-fn-reference-in-iterator.js‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
const {isParenthesized} = require('eslint-utils');
33
const getDocumentationUrl = require('./utils/get-documentation-url');
44
const methodSelector = require('./utils/method-selector');
5+
const {notFunctionSelector} = require('./utils/not-function');
56

67
const ERROR_WITH_NAME_MESSAGE_ID = 'error-with-name';
78
const ERROR_WITHOUT_NAME_MESSAGE_ID = 'error-without-name';
@@ -127,14 +128,11 @@ function check(context, node, method, options) {
127128
context.report(problem);
128129
}
129130

130-
const ignoredFirstArgumentSelector = `:not(${
131-
[
132-
'[arguments.0.type="FunctionExpression"]',
133-
'[arguments.0.type="ArrowFunctionExpression"]',
134-
'[arguments.0.type="Literal"]',
135-
'[arguments.0.type="Identifier"][arguments.0.name="undefined"]'
136-
].join(',')
137-
})`;
131+
const ignoredFirstArgumentSelector = [
132+
notFunctionSelector('arguments.0'),
133+
'[arguments.0.type!="FunctionExpression"]',
134+
'[arguments.0.type!="ArrowFunctionExpression"]'
135+
].join('');
138136

139137
const create = context => {
140138
const sourceCode = context.getSourceCode();

‎rules/no-reduce.js‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,13 @@
11
'use strict';
22
const methodSelector = require('./utils/method-selector');
33
const getDocumentationUrl = require('./utils/get-documentation-url');
4+
const {notFunctionSelector} = require('./utils/not-function');
45

56
const MESSAGE_ID_REDUCE = 'reduce';
67
const MESSAGE_ID_REDUCE_RIGHT = 'reduceRight';
78

8-
const ignoredFirstArgumentSelector = `:not(${
9-
[
10-
'[arguments.0.type="Literal"]',
11-
'[arguments.0.type="Identifier"][arguments.0.name="undefined"]'
12-
].join(',')
13-
})`;
14-
15-
const PROTOTYPE_SELECTOR = [
16-
methodSelector({names: ['call', 'apply']}),
17-
ignoredFirstArgumentSelector,
9+
const prototypeSelector = method => [
10+
methodSelector({name: method}),
1811
'[callee.object.type="MemberExpression"]',
1912
'[callee.object.computed=false]',
2013
`:matches(${
@@ -45,9 +38,16 @@ const PROTOTYPE_SELECTOR = [
4538
})`
4639
].join('');
4740

41+
const PROTOTYPE_CALL_SELECTOR = [
42+
prototypeSelector('call'),
43+
notFunctionSelector('arguments.1')
44+
].join('');
45+
46+
const PROTOTYPE_APPLY_SELECTOR = prototypeSelector('apply');
47+
4848
const METHOD_SELECTOR = [
4949
methodSelector({names: ['reduce', 'reduceRight'], min: 1, max: 2}),
50-
ignoredFirstArgumentSelector
50+
notFunctionSelector('arguments.0')
5151
].join('');
5252

5353
const create = context => {
@@ -56,9 +56,13 @@ const create = context => {
5656
// For arr.reduce()
5757
context.report({node: node.callee.property, messageId: node.callee.property.name});
5858
},
59-
[PROTOTYPE_SELECTOR](node) {
59+
[PROTOTYPE_CALL_SELECTOR](node) {
6060
// For cases [].reduce.call() and Array.prototype.reduce.call()
6161
context.report({node: node.callee.object.property, messageId: node.callee.object.property.name});
62+
},
63+
[PROTOTYPE_APPLY_SELECTOR](node) {
64+
// For cases [].reduce.apply() and Array.prototype.reduce.apply()
65+
context.report({node: node.callee.object.property, messageId: node.callee.object.property.name});
6266
}
6367
};
6468
};

‎rules/utils/not-function.js‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
'use strict';
2+
3+
// AST Types:
4+
// https://github.com/eslint/espree/blob/master/lib/ast-node-types.js#L18
5+
// Only types possible to be `argument` are listed
6+
const impossibleNodeTypes = [
7+
'ArrayExpression',
8+
'BinaryExpression',
9+
'ClassExpression',
10+
'Literal',
11+
'ObjectExpression',
12+
'TemplateLiteral',
13+
'UnaryExpression',
14+
'UpdateExpression'
15+
];
16+
17+
// Technically these nodes could be a function, but most likely not
18+
const mostLikelyNotNodeTypes = [
19+
'AssignmentExpression',
20+
'AwaitExpression',
21+
'CallExpression',
22+
'LogicalExpression',
23+
'NewExpression',
24+
'TaggedTemplateExpression',
25+
'ThisExpression'
26+
];
27+
28+
const notFunctionSelector = node => [
29+
...[...impossibleNodeTypes, ...mostLikelyNotNodeTypes].map(type => `[${node}.type!="${type}"]`),
30+
`:not([${node}.type="Identifier"][${node}.name="undefined"])`
31+
].join('');
32+
33+
module.exports = {
34+
notFunctionSelector
35+
};

‎test/no-fn-reference-in-iterator.js‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import test from 'ava';
22
import avaRuleTester from 'eslint-ava-rule-tester';
33
import {outdent} from 'outdent';
44
import rule from '../rules/no-fn-reference-in-iterator';
5+
import notFunctionTypes from './utils/not-function-types';
56

67
const ERROR_WITH_NAME_MESSAGE_ID = 'error-with-name';
78
const ERROR_WITHOUT_NAME_MESSAGE_ID = 'error-without-name';
@@ -22,8 +23,8 @@ const reduceLikeMethods = [
2223
];
2324

2425
const ruleTester = avaRuleTester(test, {
25-
env: {
26-
es6: true
26+
parserOptions: {
27+
ecmaVersion: 2020
2728
}
2829
});
2930

@@ -90,15 +91,26 @@ ruleTester.run('no-fn-reference-in-iterator', rule, {
9091
'React.children.forEach(children, fn)',
9192
'Vue.filter(name, fn)',
9293

94+
// First argument is not a function
95+
...notFunctionTypes.map(data => `foo.map(${data})`),
96+
9397
// Ignored
9498
'foo.map(() => {})',
9599
'foo.map(function() {})',
96100
'foo.map(function bar() {})',
97-
'foo.map("string")',
98-
'foo.map(null)',
99-
'foo.map(1)',
100-
'foo.map(true)',
101-
'foo.map(undefined)'
101+
102+
// #755
103+
outdent`
104+
const results = collection
105+
.find({
106+
$and: [cursorQuery, params.query]
107+
}, {
108+
projection: params.projection
109+
})
110+
.sort($sort)
111+
.limit(params.limit + 1)
112+
.toArray()
113+
`
102114
],
103115
invalid: [
104116
// Suggestions
@@ -201,15 +213,6 @@ ruleTester.run('no-fn-reference-in-iterator', rule, {
201213
'foo.map((element, index, array) => (a ? b : c)(element, index, array))'
202214
]
203215
}),
204-
invalidTestCase({
205-
code: 'foo.map((() => _.map)())',
206-
method: 'map',
207-
suggestions: [
208-
'foo.map((element) => (() => _.map)()(element))',
209-
'foo.map((element, index) => (() => _.map)()(element, index))',
210-
'foo.map((element, index, array) => (() => _.map)()(element, index, array))'
211-
]
212-
}),
213216
// Note: `await` is not handled, not sure if this is needed
214217
// invalidTestCase({
215218
// code: `foo.map(await foo())`,

‎test/no-reduce.js‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import avaRuleTester from 'eslint-ava-rule-tester';
33
import {flatten} from 'lodash';
44
import rule from '../rules/no-reduce';
55
import {outdent} from 'outdent';
6+
import notFunctionTypes from './utils/not-function-types';
67

78
const MESSAGE_ID_REDUCE = 'reduce';
89
const MESSAGE_ID_REDUCE_RIGHT = 'reduceRight';
@@ -27,14 +28,7 @@ const tests = {
2728
'[1, 2].reduce.call(() => {}, 34)',
2829

2930
// First argument is not a function
30-
'a.reduce(123)',
31-
'a.reduce(\'abc\')',
32-
'a.reduce(null)',
33-
'a.reduce(undefined)',
34-
'a.reduce(123, initialValue)',
35-
'a.reduce(\'abc\', initialValue)',
36-
'a.reduce(null, initialValue)',
37-
'a.reduce(undefined, initialValue)',
31+
...notFunctionTypes.map(data => `foo.reduce(${data})`),
3832

3933
// Test `.reduce`
4034
// Not `CallExpression`
@@ -104,7 +98,11 @@ const tests = {
10498
'[].reducex.call(arr, foo)',
10599
'[].xreduce.call(arr, foo)',
106100
'Array.prototype.reducex.call(arr, foo)',
107-
'Array.prototype.xreduce.call(arr, foo)'
101+
'Array.prototype.xreduce.call(arr, foo)',
102+
103+
// Second argument is not a function
104+
...notFunctionTypes.map(data => `Array.prototype.reduce.call(foo, ${data})`)
105+
108106
].map(code => [code, code.replace('reduce', 'reduceRight')])),
109107
invalid: flatten([
110108
'arr.reduce((total, item) => total + item)',

‎test/utils/not-function-types.js‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
'use strict';
2+
3+
module.exports = [
4+
// ArrayExpression
5+
'[]',
6+
'[element]',
7+
'[...elements]',
8+
// BinaryExpression
9+
'1 + fn',
10+
'"length" in fn',
11+
'fn instanceof Function',
12+
// ClassExpression
13+
'class ClassCantUseAsFunction {}',
14+
// Literal
15+
'0',
16+
'1',
17+
'0.1',
18+
'""',
19+
'"string"',
20+
'/regex/',
21+
'null',
22+
'0n',
23+
'1n',
24+
'true',
25+
'false',
26+
// ObjectExpression
27+
'{}',
28+
// TemplateLiteral
29+
'`templateLiteral`',
30+
// Undefined
31+
'undefined',
32+
// UnaryExpression
33+
'- fn',
34+
'+ fn',
35+
'~ fn',
36+
'typeof fn',
37+
'void fn',
38+
'delete fn',
39+
// UpdateExpression
40+
'++ fn',
41+
'-- fn',
42+
43+
// Following are not safe
44+
'a = fn', // Could be a function
45+
// 'await fn', // This requires async function to test, ignore for now
46+
'fn()', // Could be a factory returns a function
47+
'fn1 || fn2', // Could be a function
48+
'new ClassReturnsFunction()', // `class` constructor could return a function
49+
'new Function()', // `function`
50+
'fn``', // Same as `CallExpression`
51+
'this' // Could be a function
52+
];

0 commit comments

Comments
 (0)