diff --git a/rules/no-fn-reference-in-iterator.js b/rules/no-fn-reference-in-iterator.js index b17ba1e896..767a403542 100644 --- a/rules/no-fn-reference-in-iterator.js +++ b/rules/no-fn-reference-in-iterator.js @@ -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'; @@ -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(); diff --git a/rules/no-reduce.js b/rules/no-reduce.js index 98f735f22f..260583646f 100644 --- a/rules/no-reduce.js +++ b/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(${ @@ -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 => { @@ -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}); } }; }; diff --git a/rules/utils/not-function.js b/rules/utils/not-function.js new file mode 100644 index 0000000000..ba0edf9604 --- /dev/null +++ b/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 +}; diff --git a/test/no-fn-reference-in-iterator.js b/test/no-fn-reference-in-iterator.js index 6f884c368a..085763e26f 100644 --- a/test/no-fn-reference-in-iterator.js +++ b/test/no-fn-reference-in-iterator.js @@ -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'; @@ -22,8 +23,8 @@ const reduceLikeMethods = [ ]; const ruleTester = avaRuleTester(test, { - env: { - es6: true + parserOptions: { + ecmaVersion: 2020 } }); @@ -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 @@ -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())`, diff --git a/test/no-reduce.js b/test/no-reduce.js index 4cbacc5201..e024942a7b 100644 --- a/test/no-reduce.js +++ b/test/no-reduce.js @@ -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'; @@ -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` @@ -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)', diff --git a/test/utils/not-function-types.js b/test/utils/not-function-types.js new file mode 100644 index 0000000000..f4c3fc7a02 --- /dev/null +++ b/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 +];