Skip to content

Commit 797caee

Browse files
fiskerfisker
authored
no-array-method-this-argument: Check Array.from() (#2262)
1 parent 19ffd54 commit 797caee

4 files changed

+427
-88
lines changed

‎rules/no-array-method-this-argument.js

+112-76
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ const {isNodeMatches} = require('./utils/is-node-matches.js');
77
const {isNodeValueNotFunction} = require('./utils/index.js');
88
const {isMethodCall} = require('./ast/index.js');
99

10-
const ERROR = 'error';
10+
const ERROR_PROTOTYPE_METHOD = 'error-prototype-method';
11+
const ERROR_STATIC_METHOD = 'error-static-method';
1112
const SUGGESTION_BIND = 'suggestion-bind';
1213
const SUGGESTION_REMOVE = 'suggestion-remove';
1314
const messages = {
14-
[ERROR]: 'Do not use the `this` argument in `Array#{{method}}()`.',
15+
[ERROR_PROTOTYPE_METHOD]: 'Do not use the `this` argument in `Array#{{method}}()`.',
16+
[ERROR_STATIC_METHOD]: 'Do not use the `this` argument in `Array.{{method}}()`.',
1517
[SUGGESTION_REMOVE]: 'Remove the second argument.',
1618
[SUGGESTION_BIND]: 'Use a bound function.',
1719
};
@@ -70,107 +72,141 @@ const ignored = [
7072
'underscore.some',
7173
];
7274

73-
function removeThisArgument(callExpression, sourceCode) {
74-
return fixer => removeArgument(fixer, callExpression.arguments[1], sourceCode);
75+
function removeThisArgument(thisArgumentNode, sourceCode) {
76+
return fixer => removeArgument(fixer, thisArgumentNode, sourceCode);
7577
}
7678

77-
function useBoundFunction(callExpression, sourceCode) {
79+
function useBoundFunction(callbackNode, thisArgumentNode, sourceCode) {
7880
return function * (fixer) {
79-
yield removeThisArgument(callExpression, sourceCode)(fixer);
81+
yield removeThisArgument(thisArgumentNode, sourceCode)(fixer);
8082

81-
const [callback, thisArgument] = callExpression.arguments;
82-
83-
const callbackParentheses = getParentheses(callback, sourceCode);
83+
const callbackParentheses = getParentheses(callbackNode, sourceCode);
8484
const isParenthesized = callbackParentheses.length > 0;
8585
const callbackLastToken = isParenthesized
8686
? callbackParentheses.at(-1)
87-
: callback;
87+
: callbackNode;
8888
if (
8989
!isParenthesized
90-
&& shouldAddParenthesesToMemberExpressionObject(callback, sourceCode)
90+
&& shouldAddParenthesesToMemberExpressionObject(callbackNode, sourceCode)
9191
) {
9292
yield fixer.insertTextBefore(callbackLastToken, '(');
9393
yield fixer.insertTextAfter(callbackLastToken, ')');
9494
}
9595

96-
const thisArgumentText = getParenthesizedText(thisArgument, sourceCode);
96+
const thisArgumentText = getParenthesizedText(thisArgumentNode, sourceCode);
9797
// `thisArgument` was a argument, no need add extra parentheses
9898
yield fixer.insertTextAfter(callbackLastToken, `.bind(${thisArgumentText})`);
9999
};
100100
}
101101

102-
/** @param {import('eslint').Rule.RuleContext} context */
103-
const create = context => {
104-
const {sourceCode} = context;
105-
106-
return {
107-
CallExpression(callExpression) {
108-
if (
109-
!isMethodCall(callExpression, {
110-
methods: [
111-
'every',
112-
'filter',
113-
'find',
114-
'findLast',
115-
'findIndex',
116-
'findLastIndex',
117-
'flatMap',
118-
'forEach',
119-
'map',
120-
'some',
121-
],
122-
argumentsLength: 2,
123-
optionalCall: false,
124-
optionalMember: false,
125-
})
126-
|| isNodeMatches(callExpression.callee, ignored)
127-
|| isNodeValueNotFunction(callExpression.arguments[0])
128-
) {
129-
return;
130-
}
131-
132-
const {callee} = callExpression;
133-
const method = callee.property.name;
134-
const [callback, thisArgument] = callExpression.arguments;
135-
136-
const problem = {
137-
node: thisArgument,
138-
messageId: ERROR,
139-
data: {method},
140-
};
141-
142-
const thisArgumentHasSideEffect = hasSideEffect(thisArgument, sourceCode);
143-
const isArrowCallback = callback.type === 'ArrowFunctionExpression';
144-
145-
if (isArrowCallback) {
146-
if (thisArgumentHasSideEffect) {
147-
problem.suggest = [
148-
{
149-
messageId: SUGGESTION_REMOVE,
150-
fix: removeThisArgument(callExpression, sourceCode),
151-
},
152-
];
153-
} else {
154-
problem.fix = removeThisArgument(callExpression, sourceCode);
155-
}
156-
157-
return problem;
158-
}
102+
function getProblem({
103+
sourceCode,
104+
callExpression,
105+
callbackNode,
106+
thisArgumentNode,
107+
messageId,
108+
}) {
109+
const problem = {
110+
node: thisArgumentNode,
111+
messageId,
112+
data: {
113+
method: callExpression.callee.property.name,
114+
},
115+
};
159116

117+
const isArrowCallback = callbackNode.type === 'ArrowFunctionExpression';
118+
if (isArrowCallback) {
119+
const thisArgumentHasSideEffect = hasSideEffect(thisArgumentNode, sourceCode);
120+
if (thisArgumentHasSideEffect) {
160121
problem.suggest = [
161122
{
162123
messageId: SUGGESTION_REMOVE,
163-
fix: removeThisArgument(callExpression, sourceCode),
164-
},
165-
{
166-
messageId: SUGGESTION_BIND,
167-
fix: useBoundFunction(callExpression, sourceCode),
124+
fix: removeThisArgument(thisArgumentNode, sourceCode),
168125
},
169126
];
127+
} else {
128+
problem.fix = removeThisArgument(thisArgumentNode, sourceCode);
129+
}
130+
131+
return problem;
132+
}
170133

171-
return problem;
134+
problem.suggest = [
135+
{
136+
messageId: SUGGESTION_REMOVE,
137+
fix: removeThisArgument(thisArgumentNode, sourceCode),
172138
},
173-
};
139+
{
140+
messageId: SUGGESTION_BIND,
141+
fix: useBoundFunction(callbackNode, thisArgumentNode, sourceCode),
142+
},
143+
];
144+
145+
return problem;
146+
}
147+
148+
/** @param {import('eslint').Rule.RuleContext} context */
149+
const create = context => {
150+
const {sourceCode} = context;
151+
152+
// Prototype methods
153+
context.on('CallExpression', callExpression => {
154+
if (
155+
!isMethodCall(callExpression, {
156+
methods: [
157+
'every',
158+
'filter',
159+
'find',
160+
'findLast',
161+
'findIndex',
162+
'findLastIndex',
163+
'flatMap',
164+
'forEach',
165+
'map',
166+
'some',
167+
],
168+
argumentsLength: 2,
169+
optionalCall: false,
170+
optionalMember: false,
171+
})
172+
|| isNodeMatches(callExpression.callee, ignored)
173+
|| isNodeValueNotFunction(callExpression.arguments[0])
174+
) {
175+
return;
176+
}
177+
178+
return getProblem({
179+
sourceCode,
180+
callExpression,
181+
callbackNode: callExpression.arguments[0],
182+
thisArgumentNode: callExpression.arguments[1],
183+
messageId: ERROR_PROTOTYPE_METHOD,
184+
});
185+
});
186+
187+
// `Array.from()`
188+
context.on('CallExpression', callExpression => {
189+
if (
190+
!isMethodCall(callExpression, {
191+
object: 'Array',
192+
method: 'from',
193+
argumentsLength: 3,
194+
optionalCall: false,
195+
optionalMember: false,
196+
})
197+
|| isNodeValueNotFunction(callExpression.arguments[1])
198+
) {
199+
return;
200+
}
201+
202+
return getProblem({
203+
sourceCode,
204+
callExpression,
205+
callbackNode: callExpression.arguments[1],
206+
thisArgumentNode: callExpression.arguments[2],
207+
messageId: ERROR_STATIC_METHOD,
208+
});
209+
});
174210
};
175211

176212
/** @type {import('eslint').Rule.RuleModule} */

‎test/no-array-method-this-argument.mjs

+36-1
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,26 @@ test.snapshot({
99
'new array.map(() => {}, thisArgument)',
1010
'array.map?.(() => {}, thisArgument)',
1111
'array?.map(() => {}, thisArgument)',
12+
'Array.unknownMethod(iterableOrArrayLike, () => {}, thisArgument)',
13+
'new Array.from(iterableOrArrayLike, () => {}, thisArgument)',
14+
'Array.from?.(iterableOrArrayLike, () => {}, thisArgument)',
15+
'Array?.from(iterableOrArrayLike, () => {}, thisArgument)',
16+
'NotArray.from(iterableOrArrayLike, () => {}, thisArgument)',
17+
1218
// More or less arguments
1319
'array.map()',
1420
'array.map(() => {},)',
1521
'array.map(() => {}, ...thisArgument)',
1622
'array.map(...() => {}, thisArgument)',
1723
'array.map(() => {}, thisArgument, extraArgument)',
24+
'Array.from()',
25+
'Array.from(iterableOrArrayLike)',
26+
'Array.from(iterableOrArrayLike, () => {},)',
27+
'Array.from(iterableOrArrayLike, () => {}, ...thisArgument)',
28+
'Array.from(iterableOrArrayLike, ...() => {}, thisArgument)',
29+
'Array.from(...iterableOrArrayLike, () => {}, thisArgument)',
30+
'Array.from(iterableOrArrayLike, () => {}, thisArgument, extraArgument)',
31+
1832
// Ignored
1933
'lodash.every(array, () => {})',
2034
'lodash.find(array, () => {})',
@@ -33,10 +47,13 @@ test.snapshot({
3347
// `jQuery.find` and `jQuery.filter` don't accept second argument
3448
'$( "li" ).filter( ":nth-child(2n)" ).css( "background-color", "red" );',
3549
'$( "li.item-ii" ).find( "li" ).css( "background-color", "red" );',
36-
// First argument is not function
50+
// Callback argument is not function
3751
'array.map(new Callback, thisArgument)',
3852
'array.map(1, thisArgument)',
3953
'async () => array.map(await callback, thisArgument)',
54+
'Array.from(iterableOrArrayLike, new Callback, thisArgument)',
55+
'Array.from(iterableOrArrayLike, 1, thisArgument)',
56+
'Array.from(iterableOrArrayLike, await callback, thisArgument)',
4057
],
4158
invalid: [
4259
'array.every(() => {}, thisArgument)',
@@ -48,11 +65,14 @@ test.snapshot({
4865
'array.flatMap(() => {}, thisArgument)',
4966
'array.forEach(() => {}, thisArgument)',
5067
'array.map(() => {}, thisArgument)',
68+
'Array.from(iterableOrArrayLike, () => {}, thisArgument)',
5169
// Comma
5270
'array.map(() => {}, thisArgument,)',
5371
'array.map(() => {}, (0, thisArgument),)',
72+
'Array.from(iterableOrArrayLike, () => {}, thisArgument,)',
5473
// Side effect
5574
'array.map(() => {}, thisArgumentHasSideEffect())',
75+
'Array.from(iterableOrArrayLike, () => {}, thisArgumentHasSideEffect())',
5676
],
5777
});
5878

@@ -61,21 +81,36 @@ test.snapshot({
6181
valid: [],
6282
invalid: [
6383
'array.map(callback, thisArgument)',
84+
'Array.from(iterableOrArrayLike, callback, thisArgument)',
6485
'array.map(callback, (0, thisArgument))',
86+
'Array.from(iterableOrArrayLike, callback, (0, thisArgument))',
6587
'array.map(function () {}, thisArgument)',
88+
'Array.from(iterableOrArrayLike, function () {}, thisArgument)',
6689
'array.map(function callback () {}, thisArgument)',
90+
'Array.from(iterableOrArrayLike, function callback () {}, thisArgument)',
6791
{
6892
code: 'array.map( foo as bar, (( thisArgument )),)',
6993
parser: parsers.typescript,
7094
},
95+
{
96+
code: 'Array.from(iterableOrArrayLike, foo as bar, (( thisArgument )),)',
97+
parser: parsers.typescript,
98+
},
7199
{
72100
code: 'array.map( (( foo as bar )), (( thisArgument )),)',
73101
parser: parsers.typescript,
74102
},
103+
{
104+
code: 'Array.from(iterableOrArrayLike, (( foo as bar )), (( thisArgument )),)',
105+
parser: parsers.typescript,
106+
},
75107
'array.map( (( 0, callback )), (( thisArgument )),)',
108+
'Array.from(iterableOrArrayLike, (( 0, callback )), (( thisArgument )),)',
76109
// This callback is actually arrow function, but we don't know
77110
'array.map((0, () => {}), thisArgument)',
111+
'Array.from(iterableOrArrayLike, (0, () => {}), thisArgument)',
78112
// This callback is a bound function, but we don't know
79113
'array.map(callback.bind(foo), thisArgument)',
114+
'Array.from(iterableOrArrayLike, callback.bind(foo), thisArgument)',
80115
],
81116
});

0 commit comments

Comments
 (0)