Skip to content

Commit

Permalink
Add isNodeValueNotFunction to replace selectors (#2095)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker committed May 12, 2023
1 parent 676e1c8 commit 7895e5d
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 91 deletions.
27 changes: 15 additions & 12 deletions rules/no-array-callback-reference.js
@@ -1,7 +1,8 @@
'use strict';
const {isParenthesized} = require('@eslint-community/eslint-utils');
const {methodCallSelector, notFunctionSelector} = require('./selectors/index.js');
const {methodCallSelector} = require('./selectors/index.js');
const {isNodeMatches} = require('./utils/is-node-matches.js');
const {isNodeValueNotFunction} = require('./utils/index.js');

const ERROR_WITH_NAME_MESSAGE_ID = 'error-with-name';
const ERROR_WITHOUT_NAME_MESSAGE_ID = 'error-without-name';
Expand Down Expand Up @@ -195,14 +196,6 @@ function getProblem(context, node, method, options) {
return problem;
}

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('');

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const rules = {};
Expand All @@ -216,7 +209,6 @@ const create = context => {
maximumArguments: 2,
}),
options.extraSelector,
ignoredFirstArgumentSelector,
].join('');

rules[selector] = node => {
Expand All @@ -228,8 +220,19 @@ const create = context => {
return;
}

const [iterator] = node.arguments;
return getProblem(context, iterator, method, options);
const [callback] = node.arguments;

if (
callback.type === 'FunctionExpression'
|| callback.type === 'ArrowFunctionExpression'
// Ignore all `CallExpression`s include `function.bind()`
|| callback.type === 'CallExpression'
|| isNodeValueNotFunction(callback)
) {
return;
}

return getProblem(context, callback, method, options);
};
}

Expand Down
49 changes: 26 additions & 23 deletions rules/no-array-method-this-argument.js
@@ -1,10 +1,11 @@
'use strict';
const {hasSideEffect} = require('@eslint-community/eslint-utils');
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 {isNodeValueNotFunction} = require('./utils/index.js');
const {isMethodCall} = require('./ast/index.js');

const ERROR = 'error';
const SUGGESTION_BIND = 'suggestion-bind';
Expand Down Expand Up @@ -69,25 +70,6 @@ const ignored = [
'underscore.some',
];

const selector = [
methodCallSelector({
methods: [
'every',
'filter',
'find',
'findLast',
'findIndex',
'findLastIndex',
'flatMap',
'forEach',
'map',
'some',
],
argumentsLength: 2,
}),
notFunctionSelector('arguments.0'),
].join('');

function removeThisArgument(callExpression, sourceCode) {
return fixer => removeArgument(fixer, callExpression.arguments[1], sourceCode);
}
Expand Down Expand Up @@ -122,12 +104,33 @@ const create = context => {
const {sourceCode} = context;

return {
[selector](callExpression) {
const {callee} = callExpression;
if (isNodeMatches(callee, ignored)) {
CallExpression(callExpression) {
if (
!isMethodCall(callExpression, {
methods: [
'every',
'filter',
'find',
'findLast',
'findIndex',
'findLastIndex',
'flatMap',
'forEach',
'map',
'some',
],
argumentsLength: 2,
optionalCall: false,
optionalMember: false,
computed: false,
})
|| isNodeMatches(callExpression.callee, ignored)
|| isNodeValueNotFunction(callExpression.arguments[0])
) {
return;
}

const {callee} = callExpression;
const method = callee.property.name;
const [callback, thisArgument] = callExpression.arguments;

Expand Down
21 changes: 11 additions & 10 deletions rules/no-array-reduce.js
@@ -1,6 +1,7 @@
'use strict';
const {methodCallSelector} = require('./selectors/index.js');
const {arrayPrototypeMethodSelector, notFunctionSelector} = require('./selectors/index.js');
const {arrayPrototypeMethodSelector} = require('./selectors/index.js');
const {isNodeValueNotFunction} = require('./utils/index.js');

const MESSAGE_ID = 'no-reduce';
const messages = {
Expand All @@ -17,10 +18,8 @@ const prototypeSelector = method => [
const cases = [
// `array.{reduce,reduceRight}()`
{
selector: [
methodCallSelector({methods: ['reduce', 'reduceRight'], minimumArguments: 1, maximumArguments: 2}),
notFunctionSelector('arguments.0'),
].join(''),
selector: methodCallSelector({methods: ['reduce', 'reduceRight'], minimumArguments: 1, maximumArguments: 2}),
test: callExpression => !isNodeValueNotFunction(callExpression.arguments[0]),
getMethodNode: callExpression => callExpression.callee.property,
isSimpleOperation(callExpression) {
const [callback] = callExpression.arguments;
Expand All @@ -45,10 +44,8 @@ const cases = [
},
// `[].{reduce,reduceRight}.call()` and `Array.{reduce,reduceRight}.call()`
{
selector: [
prototypeSelector('call'),
notFunctionSelector('arguments.1'),
].join(''),
selector: prototypeSelector('call'),
test: callExpression => !callExpression.arguments[1] || !isNodeValueNotFunction(callExpression.arguments[1]),
getMethodNode: callExpression => callExpression.callee.object.property,
},
// `[].{reduce,reduceRight}.apply()` and `Array.{reduce,reduceRight}.apply()`
Expand Down Expand Up @@ -76,8 +73,12 @@ const create = context => {
const {allowSimpleOperations} = {allowSimpleOperations: true, ...context.options[0]};
const listeners = {};

for (const {selector, getMethodNode, isSimpleOperation} of cases) {
for (const {selector, test, getMethodNode, isSimpleOperation} of cases) {
listeners[selector] = callExpression => {
if (test && !test(callExpression)) {
return;
}

if (allowSimpleOperations && isSimpleOperation?.(callExpression)) {
return;
}
Expand Down
1 change: 0 additions & 1 deletion rules/selectors/index.js
Expand Up @@ -12,7 +12,6 @@ module.exports = {
memberExpressionSelector: require('./member-expression-selector.js'),
methodCallSelector: require('./method-call-selector.js'),
notDomNodeSelector: require('./not-dom-node.js').notDomNodeSelector,
notFunctionSelector: require('./not-function.js').notFunctionSelector,
referenceIdentifierSelector: require('./reference-identifier-selector.js'),
callExpressionSelector: require('./call-or-new-expression-selector.js').callExpressionSelector,
newExpressionSelector: require('./call-or-new-expression-selector.js').newExpressionSelector,
Expand Down
45 changes: 0 additions & 45 deletions rules/selectors/not-function.js

This file was deleted.

5 changes: 5 additions & 0 deletions rules/utils/index.js
@@ -0,0 +1,5 @@
'use strict';

module.exports = {
isNodeValueNotFunction: require('./is-node-value-not-function.js'),
};
43 changes: 43 additions & 0 deletions rules/utils/is-node-value-not-function.js
@@ -0,0 +1,43 @@
'use strict';
const {isUndefined, isCallExpression, isMethodCall} = require('../ast/index.js');

// 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 = new Set([
'ArrayExpression',
'BinaryExpression',
'ClassExpression',
'Literal',
'ObjectExpression',
'TemplateLiteral',
'UnaryExpression',
'UpdateExpression',
]);

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

const isNodeValueNotFunction = node => (
impossibleNodeTypes.has(node.type)
|| mostLikelyNotNodeTypes.has(node.type)
|| isUndefined(node)
|| (
isCallExpression(node)
&& !(isMethodCall(node, {
method: 'bind',
optionalCall: false,
optionalMember: false,
computed: false,
}))
)
);

module.exports = isNodeValueNotFunction;

0 comments on commit 7895e5d

Please sign in to comment.