Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore more types in no-fn-reference-in-iterator and no-reduce rule #756

Merged
merged 5 commits into from Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 6 additions & 8 deletions rules/no-fn-reference-in-iterator.js
Expand Up @@ -2,6 +2,7 @@
const {isParenthesized} = require('eslint-utils');
const getDocumentationUrl = require('./utils/get-documentation-url');
const methodSelector = require('./utils/method-selector');
const {notFunctionSelector} = require('./utils/not-function');

const ERROR_WITH_NAME_MESSAGE_ID = 'error-with-name';
const ERROR_WITHOUT_NAME_MESSAGE_ID = 'error-without-name';
Expand Down Expand Up @@ -127,14 +128,11 @@ function check(context, node, method, options) {
context.report(problem);
}

const ignoredFirstArgumentSelector = `:not(${
[
'[arguments.0.type="FunctionExpression"]',
'[arguments.0.type="ArrowFunctionExpression"]',
'[arguments.0.type="Literal"]',
'[arguments.0.type="Identifier"][arguments.0.name="undefined"]'
].join(',')
})`;
const ignoredFirstArgumentSelector = [
notFunctionSelector('arguments.0'),
'[arguments.0.type!="FunctionExpression"]',
'[arguments.0.type!="ArrowFunctionExpression"]'
].join('');

const create = context => {
const sourceCode = context.getSourceCode();
Expand Down
28 changes: 16 additions & 12 deletions rules/no-reduce.js
@@ -1,20 +1,13 @@
'use strict';
const methodSelector = require('./utils/method-selector');
const getDocumentationUrl = require('./utils/get-documentation-url');
const {notFunctionSelector} = require('./utils/not-function');

const MESSAGE_ID_REDUCE = 'reduce';
const MESSAGE_ID_REDUCE_RIGHT = 'reduceRight';

const ignoredFirstArgumentSelector = `:not(${
[
'[arguments.0.type="Literal"]',
'[arguments.0.type="Identifier"][arguments.0.name="undefined"]'
].join(',')
})`;

const PROTOTYPE_SELECTOR = [
methodSelector({names: ['call', 'apply']}),
ignoredFirstArgumentSelector,
const prototypeSelector = method => [
methodSelector({name: method}),
'[callee.object.type="MemberExpression"]',
'[callee.object.computed=false]',
`:matches(${
Expand Down Expand Up @@ -45,9 +38,16 @@ const PROTOTYPE_SELECTOR = [
})`
].join('');

const PROTOTYPE_CALL_SELECTOR = [
prototypeSelector('call'),
notFunctionSelector('arguments.1')
].join('');

const PROTOTYPE_APPLY_SELECTOR = prototypeSelector('apply');

const METHOD_SELECTOR = [
methodSelector({names: ['reduce', 'reduceRight'], min: 1, max: 2}),
ignoredFirstArgumentSelector
notFunctionSelector('arguments.0')
].join('');

const create = context => {
Expand All @@ -56,9 +56,13 @@ const create = context => {
// For arr.reduce()
context.report({node: node.callee.property, messageId: node.callee.property.name});
},
[PROTOTYPE_SELECTOR](node) {
[PROTOTYPE_CALL_SELECTOR](node) {
// For cases [].reduce.call() and Array.prototype.reduce.call()
context.report({node: node.callee.object.property, messageId: node.callee.object.property.name});
},
[PROTOTYPE_APPLY_SELECTOR](node) {
// For cases [].reduce.apply() and Array.prototype.reduce.apply()
context.report({node: node.callee.object.property, messageId: node.callee.object.property.name});
}
};
};
Expand Down
35 changes: 35 additions & 0 deletions rules/utils/not-function.js
@@ -0,0 +1,35 @@
'use strict';

// AST Types:
// https://github.com/eslint/espree/blob/master/lib/ast-node-types.js#L18
// Only types possible to be `argument` are listed
const impossibleNodeTypes = [
'ArrayExpression',
'BinaryExpression',
'ClassExpression',
'Literal',
'ObjectExpression',
'TemplateLiteral',
'UnaryExpression',
'UpdateExpression'
];

// Technically these nodes could be a function, but most likely not
const mostLikelyNotNodeTypes = [
'AssignmentExpression',
'AwaitExpression',
'CallExpression',
'LogicalExpression',
'NewExpression',
'TaggedTemplateExpression',
'ThisExpression'
];

const notFunctionSelector = node => [
...[...impossibleNodeTypes, ...mostLikelyNotNodeTypes].map(type => `[${node}.type!="${type}"]`),
`:not([${node}.type="Identifier"][${node}.name="undefined"])`
].join('');

module.exports = {
notFunctionSelector
};
35 changes: 19 additions & 16 deletions test/no-fn-reference-in-iterator.js
Expand Up @@ -2,6 +2,7 @@ import test from 'ava';
import avaRuleTester from 'eslint-ava-rule-tester';
import {outdent} from 'outdent';
import rule from '../rules/no-fn-reference-in-iterator';
import notFunctionTypes from './utils/not-function-types';

const ERROR_WITH_NAME_MESSAGE_ID = 'error-with-name';
const ERROR_WITHOUT_NAME_MESSAGE_ID = 'error-without-name';
Expand All @@ -22,8 +23,8 @@ const reduceLikeMethods = [
];

const ruleTester = avaRuleTester(test, {
env: {
es6: true
parserOptions: {
ecmaVersion: 2020
}
});

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

// First argument is not a function
...notFunctionTypes.map(data => `foo.map(${data})`),

// Ignored
'foo.map(() => {})',
'foo.map(function() {})',
'foo.map(function bar() {})',
'foo.map("string")',
'foo.map(null)',
'foo.map(1)',
'foo.map(true)',
'foo.map(undefined)'

// #755
outdent`
const results = collection
.find({
$and: [cursorQuery, params.query]
}, {
projection: params.projection
})
.sort($sort)
.limit(params.limit + 1)
.toArray()
`
],
invalid: [
// Suggestions
Expand Down Expand Up @@ -201,15 +213,6 @@ ruleTester.run('no-fn-reference-in-iterator', rule, {
'foo.map((element, index, array) => (a ? b : c)(element, index, array))'
]
}),
invalidTestCase({
code: 'foo.map((() => _.map)())',
method: 'map',
suggestions: [
'foo.map((element) => (() => _.map)()(element))',
'foo.map((element, index) => (() => _.map)()(element, index))',
'foo.map((element, index, array) => (() => _.map)()(element, index, array))'
]
}),
// Note: `await` is not handled, not sure if this is needed
// invalidTestCase({
// code: `foo.map(await foo())`,
Expand Down
16 changes: 7 additions & 9 deletions test/no-reduce.js
Expand Up @@ -3,6 +3,7 @@ import avaRuleTester from 'eslint-ava-rule-tester';
import {flatten} from 'lodash';
import rule from '../rules/no-reduce';
import {outdent} from 'outdent';
import notFunctionTypes from './utils/not-function-types';

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

// First argument is not a function
'a.reduce(123)',
'a.reduce(\'abc\')',
'a.reduce(null)',
'a.reduce(undefined)',
'a.reduce(123, initialValue)',
'a.reduce(\'abc\', initialValue)',
'a.reduce(null, initialValue)',
'a.reduce(undefined, initialValue)',
...notFunctionTypes.map(data => `foo.reduce(${data})`),

// Test `.reduce`
// Not `CallExpression`
Expand Down Expand Up @@ -104,7 +98,11 @@ const tests = {
'[].reducex.call(arr, foo)',
'[].xreduce.call(arr, foo)',
'Array.prototype.reducex.call(arr, foo)',
'Array.prototype.xreduce.call(arr, foo)'
'Array.prototype.xreduce.call(arr, foo)',

// Second argument is not a function
...notFunctionTypes.map(data => `Array.prototype.reduce.call(foo, ${data})`)

].map(code => [code, code.replace('reduce', 'reduceRight')])),
invalid: flatten([
'arr.reduce((total, item) => total + item)',
Expand Down
52 changes: 52 additions & 0 deletions test/utils/not-function-types.js
@@ -0,0 +1,52 @@
'use strict';

module.exports = [
// ArrayExpression
'[]',
'[element]',
'[...elements]',
// BinaryExpression
'1 + fn',
'"length" in fn',
'fn instanceof Function',
// ClassExpression
'class ClassCantUseAsFunction {}',
// Literal
'0',
'1',
'0.1',
'""',
'"string"',
'/regex/',
'null',
'0n',
'1n',
'true',
'false',
// ObjectExpression
'{}',
// TemplateLiteral
'`templateLiteral`',
// Undefined
'undefined',
// UnaryExpression
'- fn',
'+ fn',
'~ fn',
'typeof fn',
'void fn',
'delete fn',
// UpdateExpression
'++ fn',
'-- fn',

// Following are not safe
'a = fn', // Could be a function
// 'await fn', // This requires async function to test, ignore for now
'fn()', // Could be a factory returns a function
'fn1 || fn2', // Could be a function
'new ClassReturnsFunction()', // `class` constructor could return a function
'new Function()', // `function`
'fn``', // Same as `CallExpression`
'this' // Could be a function
];