Skip to content

Commit 7fdfd2c

Browse files
committed
[Fix] no-array-index-key: detect named-imported cloneElement/createElement
Fixes #3213
1 parent 625010a commit 7fdfd2c

File tree

4 files changed

+127
-18
lines changed

4 files changed

+127
-18
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
99
* [`jsx-curly-brace-presence`]: avoid warning on curlies containing quote characters ([#3214][] @ljharb)
1010
* [`jsx-indent`]: do not report on non-jsx-returning ternaries that contain null ([#3222][] @ljharb)
1111
* [`jsx-indent`]: properly report on returned ternaries with jsx ([#3222][] @ljharb)
12+
* [`no-array-index-key`]: detect named-imported `cloneElement`/`createElement` ([#3213][] @ljharb)
1213

1314
[#3222]: https://github.com/yannickcr/eslint-plugin-react/issues/3222
1415
[#3214]: https://github.com/yannickcr/eslint-plugin-react/issues/3214
16+
[#3213]: https://github.com/yannickcr/eslint-plugin-react/issues/3213
1517

1618
## [7.29.1] - 2022.02.25
1719

lib/rules/no-array-index-key.js

+23-6
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,33 @@ const astUtil = require('../util/ast');
1010
const docsUrl = require('../util/docsUrl');
1111
const pragma = require('../util/pragma');
1212
const report = require('../util/report');
13+
const variableUtil = require('../util/variable');
1314

1415
// ------------------------------------------------------------------------------
1516
// Rule Definition
1617
// ------------------------------------------------------------------------------
1718

19+
function isCreateCloneElement(node, context) {
20+
if (!node) {
21+
return false;
22+
}
23+
24+
if (node.type === 'MemberExpression' || node.type === 'OptionalMemberExpression') {
25+
return node.object
26+
&& node.object.name === pragma.getFromContext(context)
27+
&& ['createElement', 'cloneElement'].indexOf(node.property.name) !== -1;
28+
}
29+
30+
if (node.type === 'Identifier') {
31+
const variable = variableUtil.findVariableByName(context, node.name);
32+
if (variable && variable.type === 'ImportSpecifier') {
33+
return variable.parent.source.value === 'react';
34+
}
35+
}
36+
37+
return false;
38+
}
39+
1840
const messages = {
1941
noArrayIndex: 'Do not use Array index in keys',
2042
};
@@ -205,12 +227,7 @@ module.exports = {
205227

206228
return {
207229
'CallExpression, OptionalCallExpression'(node) {
208-
if (
209-
node.callee
210-
&& (node.callee.type === 'MemberExpression' || node.callee.type === 'OptionalMemberExpression')
211-
&& ['createElement', 'cloneElement'].indexOf(node.callee.property.name) !== -1
212-
&& node.arguments.length > 1
213-
) {
230+
if (isCreateCloneElement(node.callee, context) && node.arguments.length > 1) {
214231
// React.createElement
215232
if (!indexParamNames.length) {
216233
return;

lib/util/variable.js

+4
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ function findVariableByName(context, name) {
6969
return variable.defs[0].node.right;
7070
}
7171

72+
if (variable.defs[0].type === 'ImportBinding') {
73+
return variable.defs[0].node;
74+
}
75+
7276
return variable.defs[0].node.init;
7377
}
7478

tests/lib/rules/no-array-index-key.js

+98-12
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ ruleTester.run('no-array-index-key', rule, {
5454
{
5555
code: 'foo.map((baz, i) => React.cloneElement(someChild, { ...someChild.props }))',
5656
},
57+
{
58+
code: 'foo.map((baz, i) => cloneElement(someChild, { ...someChild.props }))',
59+
},
5760
{
5861
code: `
5962
foo.map((item, i) => {
@@ -63,6 +66,15 @@ ruleTester.run('no-array-index-key', rule, {
6366
})
6467
`,
6568
},
69+
{
70+
code: `
71+
foo.map((item, i) => {
72+
return cloneElement(someChild, {
73+
key: item.id
74+
})
75+
})
76+
`,
77+
},
6678
{
6779
code: 'foo.map((baz, i) => <Foo key />)',
6880
},
@@ -100,13 +112,27 @@ ruleTester.run('no-array-index-key', rule, {
100112
})
101113
`,
102114
},
115+
{
116+
code: `
117+
React.Children.map(this.props.children, (child, index, arr) => {
118+
return cloneElement(child, { key: child.id });
119+
})
120+
`,
121+
},
103122
{
104123
code: `
105124
Children.forEach(this.props.children, (child, index, arr) => {
106125
return React.cloneElement(child, { key: child.id });
107126
})
108127
`,
109128
},
129+
{
130+
code: `
131+
Children.forEach(this.props.children, (child, index, arr) => {
132+
return cloneElement(child, { key: child.id });
133+
})
134+
`,
135+
},
110136
{
111137
code: 'foo?.map(child => <Foo key={child.i} />)',
112138
features: ['optional chaining'],
@@ -145,6 +171,14 @@ ruleTester.run('no-array-index-key', rule, {
145171
code: 'foo.map((baz, i) => React.cloneElement(someChild, { ...someChild.props, key: i }))',
146172
errors: [{ messageId: 'noArrayIndex' }],
147173
},
174+
{
175+
code: `
176+
import { cloneElement } from 'react';
177+
178+
foo.map((baz, i) => cloneElement(someChild, { ...someChild.props, key: i }))
179+
`,
180+
errors: [{ messageId: 'noArrayIndex' }],
181+
},
148182
{
149183
code: `
150184
foo.map((item, i) => {
@@ -155,6 +189,18 @@ ruleTester.run('no-array-index-key', rule, {
155189
`,
156190
errors: [{ messageId: 'noArrayIndex' }],
157191
},
192+
{
193+
code: `
194+
import { cloneElement } from 'react';
195+
196+
foo.map((item, i) => {
197+
return cloneElement(someChild, {
198+
key: i
199+
})
200+
})
201+
`,
202+
errors: [{ messageId: 'noArrayIndex' }],
203+
},
158204
{
159205
code: 'foo.forEach((bar, i) => { baz.push(<Foo key={i} />); })',
160206
errors: [{ messageId: 'noArrayIndex' }],
@@ -229,33 +275,73 @@ ruleTester.run('no-array-index-key', rule, {
229275
},
230276
{
231277
code: `
232-
Children.map(this.props.children, (child, index) => {
233-
return React.cloneElement(child, { key: index });
234-
})
278+
Children.map(this.props.children, (child, index) => {
279+
return React.cloneElement(child, { key: index });
280+
})
281+
`,
282+
errors: [{ messageId: 'noArrayIndex' }],
283+
},
284+
{
285+
code: `
286+
import { cloneElement } from 'react';
287+
288+
Children.map(this.props.children, (child, index) => {
289+
return cloneElement(child, { key: index });
290+
})
235291
`,
236292
errors: [{ messageId: 'noArrayIndex' }],
237293
},
238294
{
239295
code: `
240-
React.Children.map(this.props.children, (child, index) => {
241-
return React.cloneElement(child, { key: index });
242-
})
296+
React.Children.map(this.props.children, (child, index) => {
297+
return React.cloneElement(child, { key: index });
298+
})
243299
`,
244300
errors: [{ messageId: 'noArrayIndex' }],
245301
},
246302
{
247303
code: `
248-
Children.forEach(this.props.children, (child, index) => {
249-
return React.cloneElement(child, { key: index });
250-
})
304+
import { cloneElement } from 'react';
305+
306+
React.Children.map(this.props.children, (child, index) => {
307+
return cloneElement(child, { key: index });
308+
})
251309
`,
252310
errors: [{ messageId: 'noArrayIndex' }],
253311
},
254312
{
255313
code: `
256-
React.Children.forEach(this.props.children, (child, index) => {
257-
return React.cloneElement(child, { key: index });
258-
})
314+
Children.forEach(this.props.children, (child, index) => {
315+
return React.cloneElement(child, { key: index });
316+
})
317+
`,
318+
errors: [{ messageId: 'noArrayIndex' }],
319+
},
320+
{
321+
code: `
322+
import { cloneElement } from 'react';
323+
324+
Children.forEach(this.props.children, (child, index) => {
325+
return cloneElement(child, { key: index });
326+
})
327+
`,
328+
errors: [{ messageId: 'noArrayIndex' }],
329+
},
330+
{
331+
code: `
332+
React.Children.forEach(this.props.children, (child, index) => {
333+
return React.cloneElement(child, { key: index });
334+
})
335+
`,
336+
errors: [{ messageId: 'noArrayIndex' }],
337+
},
338+
{
339+
code: `
340+
import { cloneElement } from 'react';
341+
342+
React.Children.forEach(this.props.children, (child, index) => {
343+
return cloneElement(child, { key: index });
344+
})
259345
`,
260346
errors: [{ messageId: 'noArrayIndex' }],
261347
},

0 commit comments

Comments
 (0)