Skip to content

Commit 80c81bb

Browse files
authored
Refactor to use simple selectors (#2119)
1 parent e1b25d6 commit 80c81bb

8 files changed

+224
-133
lines changed

rules/no-useless-length-check.js

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
const {methodCallSelector, matches, memberExpressionSelector} = require('./selectors/index.js');
3-
const isSameReference = require('./utils/is-same-reference.js');
4-
const {getParenthesizedRange} = require('./utils/parentheses.js');
2+
const {isMethodCall} = require('./ast/index.js');
3+
const {matches, memberExpressionSelector} = require('./selectors/index.js');
4+
const {getParenthesizedRange, isSameReference} = require('./utils/index.js');
55

66
const messages = {
77
'non-zero': 'The non-empty check is useless as `Array#some()` returns `false` for an empty array.',
@@ -29,8 +29,6 @@ const nonZeroLengthCheckSelector = [
2929
lengthCompareZeroSelector,
3030
matches(['[operator=">"]', '[operator="!=="]']),
3131
].join('');
32-
const arraySomeCallSelector = methodCallSelector('some');
33-
const arrayEveryCallSelector = methodCallSelector('every');
3432

3533
function flatLogicalExpression(node) {
3634
return [node.left, node.right].flatMap(child =>
@@ -89,11 +87,21 @@ const create = context => {
8987
[nonZeroLengthCheckSelector](node) {
9088
nonZeroLengthChecks.add(node);
9189
},
92-
[arraySomeCallSelector](node) {
93-
arraySomeCalls.add(node);
94-
},
95-
[arrayEveryCallSelector](node) {
96-
arrayEveryCalls.add(node);
90+
CallExpression(node) {
91+
if (
92+
isMethodCall(node, {
93+
optionalCall: false,
94+
optionalMember: false,
95+
computed: false,
96+
})
97+
&& node.callee.property.type === 'Identifier'
98+
) {
99+
if (node.callee.property.name === 'some') {
100+
arraySomeCalls.add(node);
101+
} else if (node.callee.property.name === 'every') {
102+
arrayEveryCalls.add(node);
103+
}
104+
}
97105
},
98106
[logicalExpressionSelector](node) {
99107
logicalExpressions.push(node);

rules/prefer-array-some.js

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
'use strict';
2-
const {methodCallSelector, matches, memberExpressionSelector} = require('./selectors/index.js');
32
const {checkVueTemplate} = require('./utils/rule.js');
4-
const {isBooleanNode} = require('./utils/boolean.js');
5-
const {getParenthesizedRange} = require('./utils/parentheses.js');
6-
const {isNodeValueNotFunction} = require('./utils/index.js');
3+
const {
4+
isBooleanNode,
5+
getParenthesizedRange,
6+
isNodeValueNotFunction,
7+
} = require('./utils/index.js');
78
const {removeMemberExpressionProperty} = require('./fix/index.js');
8-
const {isLiteral, isUndefined} = require('./ast/index.js');
9+
const {isLiteral, isUndefined, isMethodCall, isMemberExpression} = require('./ast/index.js');
910

1011
const ERROR_ID_ARRAY_SOME = 'some';
1112
const SUGGESTION_ID_ARRAY_SOME = 'some-suggestion';
@@ -16,12 +17,6 @@ const messages = {
1617
[ERROR_ID_ARRAY_FILTER]: 'Prefer `.some(…)` over non-zero length check from `.filter(…)`.',
1718
};
1819

19-
const arrayFindOrFindLastCallSelector = methodCallSelector({
20-
methods: ['find', 'findLast'],
21-
minimumArguments: 1,
22-
maximumArguments: 2,
23-
});
24-
2520
const isCheckingUndefined = node =>
2621
node.parent.type === 'BinaryExpression'
2722
// Not checking yoda expression `null != foo.find()` and `undefined !== foo.find()
@@ -46,21 +41,19 @@ const isCheckingUndefined = node =>
4641
)
4742
);
4843

49-
const arrayFilterCallSelector = [
50-
'BinaryExpression',
51-
'[right.type="Literal"]',
52-
'[right.raw="0"]',
53-
// We assume the user already follows `unicorn/explicit-length-check`. These are allowed in that rule.
54-
matches(['[operator=">"]', '[operator="!=="]']),
55-
' > ',
56-
`${memberExpressionSelector('length')}.left`,
57-
' > ',
58-
`${methodCallSelector('filter')}.object`,
59-
].join('');
60-
6144
/** @param {import('eslint').Rule.RuleContext} context */
6245
const create = context => ({
63-
[arrayFindOrFindLastCallSelector](callExpression) {
46+
CallExpression(callExpression) {
47+
if (!isMethodCall(callExpression, {
48+
methods: ['find', 'findLast'],
49+
minimumArguments: 1,
50+
maximumArguments: 2,
51+
optionalCall: false,
52+
optionalMember: false,
53+
})) {
54+
return;
55+
}
56+
6457
const isCompare = isCheckingUndefined(callExpression);
6558
if (!isCompare && !isBooleanNode(callExpression)) {
6659
return;
@@ -94,7 +87,23 @@ const create = context => ({
9487
],
9588
};
9689
},
97-
[arrayFilterCallSelector](filterCall) {
90+
BinaryExpression(binaryExpression) {
91+
if (!(
92+
// We assume the user already follows `unicorn/explicit-length-check`. These are allowed in that rule.
93+
(binaryExpression.operator === '>' || binaryExpression.operator === '!==')
94+
&& binaryExpression.right.type === 'Literal'
95+
&& binaryExpression.right.raw === '0'
96+
&& isMemberExpression(binaryExpression.left, {property: 'length', optional: false})
97+
&& isMethodCall(binaryExpression.left.object, {
98+
method: 'filter',
99+
optionalCall: false,
100+
optionalMember: false,
101+
})
102+
)) {
103+
return;
104+
}
105+
106+
const filterCall = binaryExpression.left.object;
98107
const [firstArgument] = filterCall.arguments;
99108
if (!firstArgument || isNodeValueNotFunction(firstArgument)) {
100109
return;
@@ -109,23 +118,22 @@ const create = context => ({
109118
yield fixer.replaceText(filterProperty, 'some');
110119

111120
const {sourceCode} = context;
112-
const lengthNode = filterCall.parent;
121+
const lengthNode = binaryExpression.left;
113122
/*
114123
Remove `.length`
115124
`(( (( array.filter() )).length )) > (( 0 ))`
116125
------------------------^^^^^^^
117126
*/
118127
yield removeMemberExpressionProperty(fixer, lengthNode, sourceCode);
119128

120-
const compareNode = lengthNode.parent;
121129
/*
122130
Remove `> 0`
123131
`(( (( array.filter() )).length )) > (( 0 ))`
124132
----------------------------------^^^^^^^^^^
125133
*/
126134
yield fixer.removeRange([
127135
getParenthesizedRange(lengthNode, sourceCode)[1],
128-
compareNode.range[1],
136+
binaryExpression.range[1],
129137
]);
130138

131139
// The `BinaryExpression` always ends with a number or `)`, no need check for ASI

rules/prefer-modern-dom-apis.js

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
2-
const isValueNotUsable = require('./utils/is-value-not-usable.js');
3-
const {methodCallSelector} = require('./selectors/index.js');
2+
const {isValueNotUsable} = require('./utils/index.js');
3+
const {isMethodCall} = require('./ast/index.js');
44

55
const messages = {
66
replaceChildOrInsertBefore:
@@ -9,20 +9,6 @@ const messages = {
99
'Prefer `{{reference}}.{{preferredMethod}}({{content}})` over `{{reference}}.{{method}}({{position}}, {{content}})`.',
1010
};
1111

12-
const replaceChildOrInsertBeforeSelector = [
13-
methodCallSelector({
14-
methods: ['replaceChild', 'insertBefore'],
15-
argumentsLength: 2,
16-
}),
17-
// We only allow Identifier for now
18-
'[arguments.0.type="Identifier"]',
19-
'[arguments.0.name!="undefined"]',
20-
'[arguments.1.type="Identifier"]',
21-
'[arguments.1.name!="undefined"]',
22-
// This check makes sure that only the first method of chained methods with same identifier name e.g: parentNode.insertBefore(alfa, beta).insertBefore(charlie, delta); gets reported
23-
'[callee.object.type="Identifier"]',
24-
].join('');
25-
2612
const disallowedMethods = new Map([
2713
['replaceChild', 'replaceWith'],
2814
['insertBefore', 'before'],
@@ -55,19 +41,6 @@ const checkForReplaceChildOrInsertBefore = (context, node) => {
5541
};
5642
};
5743

58-
const insertAdjacentTextOrInsertAdjacentElementSelector = [
59-
methodCallSelector({
60-
methods: ['insertAdjacentText', 'insertAdjacentElement'],
61-
argumentsLength: 2,
62-
}),
63-
// Position argument should be `string`
64-
'[arguments.0.type="Literal"]',
65-
// TODO: remove this limits on second argument
66-
':matches([arguments.1.type="Literal"], [arguments.1.type="Identifier"])',
67-
// TODO: remove this limits on callee
68-
'[callee.object.type="Identifier"]',
69-
].join('');
70-
7144
const positionReplacers = new Map([
7245
['beforebegin', 'before'],
7346
['afterbegin', 'prepend'],
@@ -113,11 +86,41 @@ const checkForInsertAdjacentTextOrInsertAdjacentElement = (context, node) => {
11386

11487
/** @param {import('eslint').Rule.RuleContext} context */
11588
const create = context => ({
116-
[replaceChildOrInsertBeforeSelector](node) {
117-
return checkForReplaceChildOrInsertBefore(context, node);
118-
},
119-
[insertAdjacentTextOrInsertAdjacentElementSelector](node) {
120-
return checkForInsertAdjacentTextOrInsertAdjacentElement(context, node);
89+
CallExpression(node) {
90+
if (
91+
isMethodCall(node, {
92+
methods: ['replaceChild', 'insertBefore'],
93+
argumentsLength: 2,
94+
optionalCall: false,
95+
optionalMember: false,
96+
})
97+
// We only allow Identifier for now
98+
&& node.arguments.every(node => node.type === 'Identifier' && node.name !== 'undefined')
99+
// This check makes sure that only the first method of chained methods with same identifier name e.g: parentNode.insertBefore(alfa, beta).insertBefore(charlie, delta); gets reported
100+
&& node.callee.object.type === 'Identifier'
101+
) {
102+
return checkForReplaceChildOrInsertBefore(context, node);
103+
}
104+
105+
if (
106+
isMethodCall(node, {
107+
methods: ['insertAdjacentText', 'insertAdjacentElement'],
108+
argumentsLength: 2,
109+
optionalCall: false,
110+
optionalMember: false,
111+
})
112+
// Position argument should be `string`
113+
&& node.arguments[0].type === 'Literal'
114+
// TODO: remove this limits on second argument
115+
&& (
116+
node.arguments[1].type === 'Literal'
117+
|| node.arguments[1].type === 'Identifier'
118+
)
119+
// TODO: remove this limits on callee
120+
&& node.callee.object.type === 'Identifier'
121+
) {
122+
return checkForInsertAdjacentTextOrInsertAdjacentElement(context, node);
123+
}
121124
},
122125
});
123126

rules/prefer-modern-math-apis.js

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
'use strict';
2-
const {getParenthesizedText, getParenthesizedRange} = require('./utils/parentheses.js');
3-
const {methodCallSelector} = require('./selectors/index.js');
4-
const isSameReference = require('./utils/is-same-reference.js');
5-
const {isLiteral} = require('./ast/index.js');
2+
const {
3+
getParenthesizedText,
4+
getParenthesizedRange,
5+
isSameReference,
6+
} = require('./utils/index.js');
7+
const {isLiteral, isMethodCall} = require('./ast/index.js');
68
const {replaceNodeOrTokenAndSpacesBefore, removeParentheses} = require('./fix/index.js');
79

810
const MESSAGE_ID = 'prefer-modern-math-apis';
@@ -106,8 +108,6 @@ const checkFunctions = [
106108
createLogCallDivideConstantCheck({constantName: 'LN2', replacementMethod: 'log2'}),
107109
];
108110

109-
const mathSqrtCallSelector = methodCallSelector({object: 'Math', method: 'sqrt', argumentsLength: 1});
110-
111111
const isPlusExpression = node => node.type === 'BinaryExpression' && node.operator === '+';
112112

113113
const isPow2Expression = node =>
@@ -129,7 +129,17 @@ const create = context => {
129129
const nodes = [];
130130

131131
return {
132-
[mathSqrtCallSelector](callExpression) {
132+
CallExpression(callExpression) {
133+
if (!isMethodCall(callExpression, {
134+
object: 'Math',
135+
method: 'sqrt',
136+
argumentsLength: 1,
137+
optionalCall: false,
138+
optionalMember: false,
139+
})) {
140+
return;
141+
}
142+
133143
const expressions = flatPlusExpression(callExpression.arguments[0]);
134144
if (expressions.some(expression => !isPow2Expression(expression))) {
135145
return;

rules/prefer-reflect-apply.js

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,12 @@
11
'use strict';
22
const {getPropertyName} = require('@eslint-community/eslint-utils');
3-
const {not, methodCallSelector} = require('./selectors/index.js');
4-
const {isNullLiteral} = require('./ast/index.js');
3+
const {isNullLiteral, isMethodCall} = require('./ast/index.js');
54

65
const MESSAGE_ID = 'prefer-reflect-apply';
76
const messages = {
87
[MESSAGE_ID]: 'Prefer `Reflect.apply()` over `Function#apply()`.',
98
};
109

11-
const selector = [
12-
methodCallSelector({allowComputed: true}),
13-
not(['Literal', 'ArrayExpression', 'ObjectExpression'].map(type => `[callee.object.type=${type}]`)),
14-
].join('');
15-
1610
const isApplySignature = (argument1, argument2) => (
1711
(
1812
isNullLiteral(argument1)
@@ -64,7 +58,19 @@ const fixFunctionPrototypeCall = (node, sourceCode) => {
6458

6559
/** @param {import('eslint').Rule.RuleContext} context */
6660
const create = context => ({
67-
[selector](node) {
61+
CallExpression(node) {
62+
if (
63+
!isMethodCall(node, {
64+
optionalCall: false,
65+
optionalMember: false,
66+
})
67+
|| node.callee.object.type === 'Literal'
68+
|| node.callee.object.type === 'ArrayExpression'
69+
|| node.callee.object.type === 'ObjectExpression'
70+
) {
71+
return;
72+
}
73+
6874
const {sourceCode} = context;
6975
const fix = fixDirectApplyCall(node, sourceCode) || fixFunctionPrototypeCall(node, sourceCode);
7076
if (fix) {

0 commit comments

Comments
 (0)