diff --git a/rules/no-array-callback-reference.js b/rules/no-array-callback-reference.js index 90f701fed0..ea9069268e 100644 --- a/rules/no-array-callback-reference.js +++ b/rules/no-array-callback-reference.js @@ -133,6 +133,8 @@ function getProblem(context, node, method, options) { const ignoredFirstArgumentSelector = [ notFunctionSelector('arguments.0'), + // Ignore all `CallExpression`s include `function.bind()` + '[arguments.0.type!="CallExpression"]', '[arguments.0.type!="FunctionExpression"]', '[arguments.0.type!="ArrowFunctionExpression"]' ].join(''); diff --git a/rules/no-array-method-this-argument.js b/rules/no-array-method-this-argument.js index daeeabc7bc..7ba86e6ea4 100644 --- a/rules/no-array-method-this-argument.js +++ b/rules/no-array-method-this-argument.js @@ -1,9 +1,10 @@ 'use strict'; const {hasSideEffect} = require('eslint-utils'); -const {methodCallSelector} = require('./selectors/index.js'); +const {methodCallSelector, notFunctionSelector} = require('./selectors/index.js'); const {removeArgument} = require('./fix/index.js'); const {getParentheses, getParenthesizedText} = require('./utils/parentheses.js'); const shouldAddParenthesesToMemberExpressionObject = require('./utils/should-add-parentheses-to-member-expression-object.js'); +const {isNodeMatches} = require('./utils/is-node-matches.js'); const ERROR = 'error'; const SUGGESTION_BIND = 'suggestion-bind'; @@ -14,19 +15,61 @@ const messages = { [SUGGESTION_BIND]: 'Use a bound function.' }; -const selector = methodCallSelector({ - names: [ - 'every', - 'filter', - 'find', - 'findIndex', - 'flatMap', - 'forEach', - 'map', - 'some' - ], - length: 2 -}); +const ignored = [ + 'lodash.every', + '_.every', + 'underscore.every', + + 'lodash.filter', + '_.filter', + 'underscore.filter', + 'Vue.filter', + + 'lodash.find', + '_.find', + 'underscore.find', + + 'lodash.findIndex', + '_.findIndex', + 'underscore.findIndex', + + 'lodash.flatMap', + '_.flatMap', + + 'lodash.forEach', + '_.forEach', + 'React.Children.forEach', + 'Children.forEach', + + 'lodash.map', + '_.map', + 'underscore.map', + 'React.Children.map', + 'Children.map', + 'jQuery.map', + '$.map', + + 'lodash.some', + '_.some', + 'underscore.some' +]; + +const selector = [ + methodCallSelector({ + names: [ + 'every', + 'filter', + 'find', + 'findIndex', + 'flatMap', + 'forEach', + 'map', + 'some' + ], + length: 2 + }), + notFunctionSelector('arguments.0') +].join(''); function removeThisArgument(callExpression, sourceCode) { return fixer => removeArgument(fixer, callExpression.arguments[1], sourceCode); @@ -63,7 +106,12 @@ const create = context => { return { [selector](callExpression) { - const method = callExpression.callee.property.name; + const {callee} = callExpression; + if (isNodeMatches(callee, ignored)) { + return; + } + + const method = callee.property.name; const [callback, thisArgument] = callExpression.arguments; const problem = { diff --git a/rules/selectors/not-function.js b/rules/selectors/not-function.js index ba0edf9604..8731af1bbc 100644 --- a/rules/selectors/not-function.js +++ b/rules/selectors/not-function.js @@ -1,4 +1,5 @@ 'use strict'; +const not = require('./negation.js'); // AST Types: // /~https://github.com/eslint/espree/blob/master/lib/ast-node-types.js#L18 @@ -18,17 +19,26 @@ const impossibleNodeTypes = [ 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(''); +const notFunctionSelector = node => not([ + [...impossibleNodeTypes, ...mostLikelyNotNodeTypes].map(type => `[${node}.type="${type}"]`), + `[${node}.type="Identifier"][${node}.name="undefined"]`, + [ + `[${node}.type="CallExpression"]`, + not([ + `[${node}.callee.type="MemberExpression"]`, + `[${node}.callee.optional!=true]`, + `[${node}.callee.computed!=true]`, + `[${node}.callee.property.type="Identifier"]`, + `[${node}.callee.property.name="bind"]` + ].join('')) + ].join('') +]); module.exports = { notFunctionSelector diff --git a/test/no-array-callback-reference.mjs b/test/no-array-callback-reference.mjs index 933c5c597d..c83136db61 100644 --- a/test/no-array-callback-reference.mjs +++ b/test/no-array-callback-reference.mjs @@ -96,6 +96,7 @@ test({ // Exclude await expressions ...simpleMethods.map(method => `(async () => await foo.${method}(bar))()`), + 'foo.map(function (a) {}.bind(bar))', // #813 outdent` diff --git a/test/no-array-method-this-argument.mjs b/test/no-array-method-this-argument.mjs index beb3d79884..93450a4ce6 100644 --- a/test/no-array-method-this-argument.mjs +++ b/test/no-array-method-this-argument.mjs @@ -1,4 +1,4 @@ -import {getTester} from './utils/test.mjs'; +import {getTester, parsers} from './utils/test.mjs'; const {test} = getTester(import.meta); @@ -14,7 +14,24 @@ test.snapshot({ 'array.map(() => {},)', 'array.map(() => {}, ...thisArgument)', 'array.map(...() => {}, thisArgument)', - 'array.map(() => {}, thisArgument, extraArgument)' + 'array.map(() => {}, thisArgument, extraArgument)', + // Ignored + 'lodash.every(array, () => {})', + 'lodash.find(array, () => {})', + 'jQuery.map(array, () => {})', + '$.map(array, () => {})', + 'React.Children.map(children, () => {})', + 'Children.map(children, () => {})', + 'React.Children.forEach(children, () => {})', + 'Children.forEach(children, () => {})', + 'Vue.filter("capitalize", () => {})', + // `jQuery.find` and `jQuery.filter` don't accept second argument + '$( "li" ).filter( ":nth-child(2n)" ).css( "background-color", "red" );', + '$( "li.item-ii" ).find( "li" ).css( "background-color", "red" );', + // First argument is not function + 'array.map(new Callback, thisArgument)', + 'array.map(1, thisArgument)', + 'async () => array.map(await callback, thisArgument)' ], invalid: [ 'array.every(() => {}, thisArgument)', @@ -40,12 +57,15 @@ test.snapshot({ 'array.map(callback, (0, thisArgument))', 'array.map(function () {}, thisArgument)', 'array.map(function callback () {}, thisArgument)', - 'array.map(new Callback, thisArgument)', - 'array.map(1, thisArgument)', - 'async () => array.map(await callback, thisArgument)', - 'array.map((new Callback), thisArgument)', - 'array.map(new Callback(), thisArgument)', - 'array.map( (( callback )), (( thisArgument )),)', + { + code: 'array.map( foo as bar, (( thisArgument )),)', + parser: parsers.typescript + }, + { + code: 'array.map( (( foo as bar )), (( thisArgument )),)', + parser: parsers.typescript + }, + 'array.map( (( 0, callback )), (( thisArgument )),)', // This callback is actually arrow function, but we don't know 'array.map((0, () => {}), thisArgument)', // This callback is a bound function, but we don't know diff --git a/test/snapshots/no-array-method-this-argument.mjs.md b/test/snapshots/no-array-method-this-argument.mjs.md index 797ce9da77..c4182e35e8 100644 --- a/test/snapshots/no-array-method-this-argument.mjs.md +++ b/test/snapshots/no-array-method-this-argument.mjs.md @@ -235,114 +235,60 @@ Generated by [AVA](https://avajs.dev). ` ## Invalid #5 - 1 | array.map(new Callback, thisArgument) + 1 | array.map( foo as bar, (( thisArgument )),) > Error 1/1 `␊ - > 1 | array.map(new Callback, thisArgument)␊ - | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ + > 1 | array.map( foo as bar, (( thisArgument )),)␊ + | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ ␊ --------------------------------------------------------------------------------␊ Suggestion 1/2: Remove the second argument.␊ - 1 | array.map(new Callback)␊ + 1 | array.map( foo as bar,)␊ ␊ --------------------------------------------------------------------------------␊ Suggestion 2/2: Use a bound function.␊ - 1 | array.map((new Callback).bind(thisArgument))␊ + 1 | array.map( (foo as bar).bind((( thisArgument ))),)␊ ` ## Invalid #6 - 1 | array.map(1, thisArgument) + 1 | array.map( (( foo as bar )), (( thisArgument )),) > Error 1/1 `␊ - > 1 | array.map(1, thisArgument)␊ - | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ + > 1 | array.map( (( foo as bar )), (( thisArgument )),)␊ + | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ ␊ --------------------------------------------------------------------------------␊ Suggestion 1/2: Remove the second argument.␊ - 1 | array.map(1)␊ + 1 | array.map( (( foo as bar )),)␊ ␊ --------------------------------------------------------------------------------␊ Suggestion 2/2: Use a bound function.␊ - 1 | array.map((1).bind(thisArgument))␊ + 1 | array.map( (( foo as bar )).bind((( thisArgument ))),)␊ ` ## Invalid #7 - 1 | async () => array.map(await callback, thisArgument) + 1 | array.map( (( 0, callback )), (( thisArgument )),) > Error 1/1 `␊ - > 1 | async () => array.map(await callback, thisArgument)␊ - | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ + > 1 | array.map( (( 0, callback )), (( thisArgument )),)␊ + | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ ␊ --------------------------------------------------------------------------------␊ Suggestion 1/2: Remove the second argument.␊ - 1 | async () => array.map(await callback)␊ + 1 | array.map( (( 0, callback )),)␊ ␊ --------------------------------------------------------------------------------␊ Suggestion 2/2: Use a bound function.␊ - 1 | async () => array.map((await callback).bind(thisArgument))␊ + 1 | array.map( (( 0, callback )).bind((( thisArgument ))),)␊ ` ## Invalid #8 - 1 | array.map((new Callback), thisArgument) - -> Error 1/1 - - `␊ - > 1 | array.map((new Callback), thisArgument)␊ - | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ - ␊ - --------------------------------------------------------------------------------␊ - Suggestion 1/2: Remove the second argument.␊ - 1 | array.map((new Callback))␊ - ␊ - --------------------------------------------------------------------------------␊ - Suggestion 2/2: Use a bound function.␊ - 1 | array.map((new Callback).bind(thisArgument))␊ - ` - -## Invalid #9 - 1 | array.map(new Callback(), thisArgument) - -> Error 1/1 - - `␊ - > 1 | array.map(new Callback(), thisArgument)␊ - | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ - ␊ - --------------------------------------------------------------------------------␊ - Suggestion 1/2: Remove the second argument.␊ - 1 | array.map(new Callback())␊ - ␊ - --------------------------------------------------------------------------------␊ - Suggestion 2/2: Use a bound function.␊ - 1 | array.map(new Callback().bind(thisArgument))␊ - ` - -## Invalid #10 - 1 | array.map( (( callback )), (( thisArgument )),) - -> Error 1/1 - - `␊ - > 1 | array.map( (( callback )), (( thisArgument )),)␊ - | ^^^^^^^^^^^^ Do not use the \`this\` argument in \`Array#map()\`.␊ - ␊ - --------------------------------------------------------------------------------␊ - Suggestion 1/2: Remove the second argument.␊ - 1 | array.map( (( callback )),)␊ - ␊ - --------------------------------------------------------------------------------␊ - Suggestion 2/2: Use a bound function.␊ - 1 | array.map( (( callback )).bind((( thisArgument ))),)␊ - ` - -## Invalid #11 1 | array.map((0, () => {}), thisArgument) > Error 1/1 @@ -360,7 +306,7 @@ Generated by [AVA](https://avajs.dev). 1 | array.map((0, () => {}).bind(thisArgument))␊ ` -## Invalid #12 +## Invalid #9 1 | array.map(callback.bind(foo), thisArgument) > Error 1/1 diff --git a/test/snapshots/no-array-method-this-argument.mjs.snap b/test/snapshots/no-array-method-this-argument.mjs.snap index 0caad9d16d..d7e7bc85ea 100644 Binary files a/test/snapshots/no-array-method-this-argument.mjs.snap and b/test/snapshots/no-array-method-this-argument.mjs.snap differ diff --git a/test/utils/snapshot-rule-tester.mjs b/test/utils/snapshot-rule-tester.mjs index 662e2ae79c..95c5c40825 100644 --- a/test/utils/snapshot-rule-tester.mjs +++ b/test/utils/snapshot-rule-tester.mjs @@ -59,7 +59,7 @@ function normalizeTests(tests) { const additionalProperties = getAdditionalProperties( testCase, - ['code', 'options', 'filename', 'parserOptions'] + ['code', 'options', 'filename', 'parserOptions', 'parser'] ); if (additionalProperties.length > 0) { @@ -72,9 +72,15 @@ function normalizeTests(tests) { } function getVerifyConfig(ruleId, testerConfig, testCase) { - const {options, parserOptions} = testCase; + const { + options, + parserOptions, + parser = testerConfig.parser + } = testCase; + return { ...testerConfig, + parser, parserOptions: { ...testerConfig.parserOptions, ...parserOptions