From b9d62729a7a1046b5c43dd6584c468f0986b46e0 Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 30 May 2020 00:51:13 +0800 Subject: [PATCH 1/5] Ignore more types on `no-fn-reference-in-iterator` and `no-reduce` rule --- rules/no-fn-reference-in-iterator.js | 14 ++++++------- rules/no-reduce.js | 28 +++++++++++++++----------- rules/utils/not-function.js | 23 +++++++++++++++++++++ test/no-fn-reference-in-iterator.js | 15 +++++++------- test/no-reduce.js | 16 +++++++-------- test/utils/not-function-types.js | 30 ++++++++++++++++++++++++++++ 6 files changed, 89 insertions(+), 37 deletions(-) create mode 100644 rules/utils/not-function.js create mode 100644 test/utils/not-function-types.js 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..318e945730 --- /dev/null +++ b/rules/utils/not-function.js @@ -0,0 +1,23 @@ +'use strict'; + +// AST Types: +// https://github.com/eslint/espree/blob/master/lib/ast-node-types.js#L18 +// Only types possible to be `callee` or `argument` are listed +const impossibleNodeTypes = [ + 'ArrayExpression', + 'ClassExpression', + 'Literal', + 'ObjectExpression', + 'TemplateLiteral', + // Technically `this` could be a function, but most likely not + 'ThisExpression' +]; + +const notFunctionSelector = node => [ + ...impossibleNodeTypes.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..dc215683c0 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,13 @@ 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)' + 'foo.map(function bar() {})' ], invalid: [ // Suggestions 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..b5369f7a08 --- /dev/null +++ b/test/utils/not-function-types.js @@ -0,0 +1,30 @@ +'use strict'; + +module.exports = [ + // ArrayExpression + '[]', + '[element]', + '[...elements]', + // ClassExpression + 'class Node {}', + // Literal + '0', + '1', + '0.1', + '""', + '"string"', + '/regex/', + 'null', + '0n', + '1n', + 'true', + 'false', + // ObjectExpression + '{}', + // TemplateLiteral + '`templateLiteral`', + // Undefined + 'undefined', + // ThisExpression + 'this' +]; From 87b32a682d15fc4b4218c30d848301562a1a446c Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 30 May 2020 01:03:03 +0800 Subject: [PATCH 2/5] Add test for 755 --- test/no-fn-reference-in-iterator.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/no-fn-reference-in-iterator.js b/test/no-fn-reference-in-iterator.js index dc215683c0..bf588ce44d 100644 --- a/test/no-fn-reference-in-iterator.js +++ b/test/no-fn-reference-in-iterator.js @@ -97,7 +97,20 @@ ruleTester.run('no-fn-reference-in-iterator', rule, { // Ignored 'foo.map(() => {})', 'foo.map(function() {})', - 'foo.map(function bar() {})' + 'foo.map(function bar() {})', + + // #755 + outdent` + const results = collection + .find({ + $and: [cursorQuery, params.query] + }, { + projection: params.projection + }) + .sort($sort) + .limit(params.limit + 1) + .toArray() + ` ], invalid: [ // Suggestions From 46f7956651a86d344339c30379be78f155ff2456 Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 30 May 2020 01:42:55 +0800 Subject: [PATCH 3/5] More types --- rules/utils/not-function.js | 19 ++++++++++++++++--- test/no-fn-reference-in-iterator.js | 9 --------- test/utils/not-function-types.js | 26 ++++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/rules/utils/not-function.js b/rules/utils/not-function.js index 318e945730..9796e64dd1 100644 --- a/rules/utils/not-function.js +++ b/rules/utils/not-function.js @@ -2,19 +2,32 @@ // AST Types: // https://github.com/eslint/espree/blob/master/lib/ast-node-types.js#L18 -// Only types possible to be `callee` or `argument` are listed +// Only types possible to be `argument` are listed const impossibleNodeTypes = [ 'ArrayExpression', + 'BinaryExpression', 'ClassExpression', 'Literal', 'ObjectExpression', 'TemplateLiteral', - // Technically `this` could be a function, but most likely not + '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.map(type => `[${node}.type!="${type}"]`), + ...[...impossibleNodeTypes, ...mostLikelyNotNodeTypes].map(type => `[${node}.type!="${type}"]`), `:not([${node}.type="Identifier"][${node}.name="undefined"])` ].join(''); diff --git a/test/no-fn-reference-in-iterator.js b/test/no-fn-reference-in-iterator.js index bf588ce44d..085763e26f 100644 --- a/test/no-fn-reference-in-iterator.js +++ b/test/no-fn-reference-in-iterator.js @@ -213,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/utils/not-function-types.js b/test/utils/not-function-types.js index b5369f7a08..7b58316ffd 100644 --- a/test/utils/not-function-types.js +++ b/test/utils/not-function-types.js @@ -5,6 +5,10 @@ module.exports = [ '[]', '[element]', '[...elements]', + // BinaryExpression + '1 + fn', + '"length" in fn', + 'fn instanceof Function', // ClassExpression 'class Node {}', // Literal @@ -25,6 +29,24 @@ module.exports = [ '`templateLiteral`', // Undefined 'undefined', - // ThisExpression - 'this' + // 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 + 'false || fn', // could be a function + 'new Fn()', // class constructor could return a function + 'new Function()', // class constructor could return a function + 'fn``', // same as `CallExpression` + 'this' // could be a function ]; From ef67f93e08e7826a248201ecb30ceec5384dad06 Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 30 May 2020 01:47:38 +0800 Subject: [PATCH 4/5] style --- rules/utils/not-function.js | 3 +-- test/utils/not-function-types.js | 16 ++++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/rules/utils/not-function.js b/rules/utils/not-function.js index 9796e64dd1..ba0edf9604 100644 --- a/rules/utils/not-function.js +++ b/rules/utils/not-function.js @@ -11,7 +11,7 @@ const impossibleNodeTypes = [ 'ObjectExpression', 'TemplateLiteral', 'UnaryExpression', - 'UpdateExpression', + 'UpdateExpression' ]; // Technically these nodes could be a function, but most likely not @@ -25,7 +25,6 @@ const mostLikelyNotNodeTypes = [ 'ThisExpression' ]; - const notFunctionSelector = node => [ ...[...impossibleNodeTypes, ...mostLikelyNotNodeTypes].map(type => `[${node}.type!="${type}"]`), `:not([${node}.type="Identifier"][${node}.name="undefined"])` diff --git a/test/utils/not-function-types.js b/test/utils/not-function-types.js index 7b58316ffd..76da9ce85e 100644 --- a/test/utils/not-function-types.js +++ b/test/utils/not-function-types.js @@ -41,12 +41,12 @@ module.exports = [ '-- 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 - 'false || fn', // could be a function - 'new Fn()', // class constructor could return a function - 'new Function()', // class constructor could return a function - 'fn``', // same as `CallExpression` - 'this' // could be a function + '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 + 'false || fn', // Could be a function + 'new Fn()', // `class` constructor could return a function + 'new Function()', // `function` + 'fn``', // Same as `CallExpression` + 'this' // Could be a function ]; From 657fece36e2e17f7ed3b3c0fa59b3cd25c41d057 Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 30 May 2020 01:58:47 +0800 Subject: [PATCH 5/5] Update some names --- test/utils/not-function-types.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/utils/not-function-types.js b/test/utils/not-function-types.js index 76da9ce85e..f4c3fc7a02 100644 --- a/test/utils/not-function-types.js +++ b/test/utils/not-function-types.js @@ -10,7 +10,7 @@ module.exports = [ '"length" in fn', 'fn instanceof Function', // ClassExpression - 'class Node {}', + 'class ClassCantUseAsFunction {}', // Literal '0', '1', @@ -44,8 +44,8 @@ module.exports = [ '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 - 'false || fn', // Could be a function - 'new Fn()', // `class` constructor could return 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