Skip to content

Commit 4a39e6c

Browse files
lencionilencioni
committed
Add no-array-index-key rule
This rule will warn when using the array index as the `key`. As a first pass, I decided to implement this for `Array.prototype.map`, since that is likely the most common case, but it theoretically could be expanded to find other cases so I kept the naming more generic. Like many rules, this one is imperfect and is prone to some false positives and negatives. For instance, if someone defines a `.map()` function on an object that doesn't have the same signature as `Array.prototype.map`, this will likely end up warning in those cases. However, I think the value of this rule outweighs its hypothetical drawbacks.
1 parent 0a40c27 commit 4a39e6c

File tree

5 files changed

+328
-0
lines changed

5 files changed

+328
-0
lines changed

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
8383
* [react/display-name](docs/rules/display-name.md): Prevent missing `displayName` in a React component definition
8484
* [react/forbid-component-props](docs/rules/forbid-component-props.md): Forbid certain props on Components
8585
* [react/forbid-prop-types](docs/rules/forbid-prop-types.md): Forbid certain propTypes
86+
* [react/no-array-index-key](docs/rules/no-array-index-key.md): Prevent using Array index in `key` props
8687
* [react/no-children-prop](docs/rules/no-children-prop.md): Prevent passing children as props
8788
* [react/no-danger](docs/rules/no-danger.md): Prevent usage of dangerous JSX properties
8889
* [react/no-danger-with-children](docs/rules/no-danger-with-children.md): Prevent problem with children and props.dangerouslySetInnerHTML

‎docs/rules/no-array-index-key.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Prevent usage of Array index in keys
2+
3+
Warn if an element uses an Array index in its `key`.
4+
5+
The `key` is used by React to [identify which items have changed, are added, or are removed and should be stable](https://facebook.github.io/react/docs/lists-and-keys.html#keys).
6+
7+
## Rule Details
8+
9+
The following patterns are considered warnings:
10+
11+
```jsx
12+
things.map((thing, index) => (
13+
<Hello key={index} />
14+
));
15+
```
16+
17+
The following patterns are not considered warnings:
18+
19+
```jsx
20+
things.map((thing) => (
21+
<Hello key={thing.id} />
22+
));
23+
```

‎index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ var allRules = {
88
'jsx-wrap-multilines': require('./lib/rules/jsx-wrap-multilines'),
99
'self-closing-comp': require('./lib/rules/self-closing-comp'),
1010
'jsx-no-comment-textnodes': require('./lib/rules/jsx-no-comment-textnodes'),
11+
'no-array-index-key': require('./lib/rules/no-array-index-key'),
1112
'no-danger': require('./lib/rules/no-danger'),
1213
'no-set-state': require('./lib/rules/no-set-state'),
1314
'no-is-mounted': require('./lib/rules/no-is-mounted'),

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

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/**
2+
* @fileoverview Prevent usage of Array index in keys
3+
* @author Joe Lencioni
4+
*/
5+
'use strict';
6+
7+
// ------------------------------------------------------------------------------
8+
// Rule Definition
9+
// ------------------------------------------------------------------------------
10+
11+
module.exports = {
12+
meta: {
13+
docs: {
14+
description: 'Prevent usage of Array index in keys',
15+
category: 'Best Practices',
16+
recommended: false
17+
},
18+
19+
schema: []
20+
},
21+
22+
create: function(context) {
23+
// --------------------------------------------------------------------------
24+
// Public
25+
// --------------------------------------------------------------------------
26+
var indexParamNames = [];
27+
var ERROR_MESSAGE = 'Do not use Array index in keys';
28+
29+
function isArrayIndex(node) {
30+
return node.type === 'Identifier'
31+
&& indexParamNames.indexOf(node.name) !== -1;
32+
}
33+
34+
function getMapIndexParamName(node) {
35+
var callee = node.callee;
36+
if (callee.type !== 'MemberExpression') {
37+
return null;
38+
}
39+
if (callee.property.type !== 'Identifier') {
40+
return null;
41+
}
42+
if (callee.property.name !== 'map') {
43+
return null;
44+
}
45+
46+
var firstArg = node.arguments[0];
47+
if (!firstArg) {
48+
return null;
49+
}
50+
51+
var isFunction = [
52+
'ArrowFunctionExpression',
53+
'FunctionExpression'
54+
].indexOf(firstArg.type) !== -1;
55+
if (!isFunction) {
56+
return null;
57+
}
58+
59+
var params = firstArg.params;
60+
if (params.length < 2) {
61+
return null;
62+
}
63+
64+
return params[1].name;
65+
}
66+
67+
function getIdentifiersFromBinaryExpression(side) {
68+
if (side.type === 'Identifier') {
69+
return side;
70+
}
71+
72+
if (side.type === 'BinaryExpression') {
73+
// recurse
74+
var left = getIdentifiersFromBinaryExpression(side.left);
75+
var right = getIdentifiersFromBinaryExpression(side.right);
76+
return [].concat(left, right).filter(Boolean);
77+
}
78+
79+
return null;
80+
}
81+
82+
function checkPropValue(node) {
83+
if (isArrayIndex(node)) {
84+
// key={bar}
85+
context.report({
86+
node: node,
87+
message: ERROR_MESSAGE
88+
});
89+
return;
90+
}
91+
92+
if (node.type === 'TemplateLiteral') {
93+
// key={`foo-${bar}`}
94+
node.expressions.filter(isArrayIndex).forEach(function() {
95+
context.report({node: node, message: ERROR_MESSAGE});
96+
});
97+
98+
return;
99+
}
100+
101+
if (node.type === 'BinaryExpression') {
102+
// key={'foo' + bar}
103+
var identifiers = getIdentifiersFromBinaryExpression(node);
104+
105+
identifiers.filter(isArrayIndex).forEach(function() {
106+
context.report({node: node, message: ERROR_MESSAGE});
107+
});
108+
109+
return;
110+
}
111+
}
112+
113+
return {
114+
CallExpression: function(node) {
115+
if (
116+
node.callee
117+
&& node.callee.type === 'MemberExpression'
118+
&& node.callee.property.name === 'createElement'
119+
&& node.arguments.length > 1
120+
) {
121+
// React.createElement
122+
if (!indexParamNames.length) {
123+
return;
124+
}
125+
126+
var props = node.arguments[1];
127+
128+
if (props.type !== 'ObjectExpression') {
129+
return;
130+
}
131+
132+
props.properties.forEach(function (prop) {
133+
if (prop.key.name !== 'key') {
134+
// { foo: bar }
135+
return;
136+
}
137+
138+
checkPropValue(prop.value);
139+
});
140+
141+
return;
142+
}
143+
144+
var mapIndexParamName = getMapIndexParamName(node);
145+
if (!mapIndexParamName) {
146+
return;
147+
}
148+
149+
indexParamNames.push(mapIndexParamName);
150+
},
151+
152+
JSXAttribute: function(node) {
153+
if (node.name.name !== 'key') {
154+
// foo={bar}
155+
return;
156+
}
157+
158+
if (!indexParamNames.length) {
159+
// Not inside a call expression that we think has an index param.
160+
return;
161+
}
162+
163+
var value = node.value;
164+
if (value.type !== 'JSXExpressionContainer') {
165+
// key='foo'
166+
return;
167+
}
168+
169+
checkPropValue(value.expression);
170+
},
171+
172+
'CallExpression:exit': function(node) {
173+
var mapIndexParamName = getMapIndexParamName(node);
174+
if (!mapIndexParamName) {
175+
return;
176+
}
177+
178+
indexParamNames.pop();
179+
}
180+
};
181+
182+
}
183+
};

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

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/**
2+
* @fileoverview Tests for no-array-index-key
3+
* @author Joe Lencioni
4+
*/
5+
6+
'use strict';
7+
8+
// -----------------------------------------------------------------------------
9+
// Requirements
10+
// -----------------------------------------------------------------------------
11+
12+
var rule = require('../../../lib/rules/no-array-index-key');
13+
var RuleTester = require('eslint').RuleTester;
14+
15+
var parserOptions = {
16+
ecmaVersion: 6,
17+
ecmaFeatures: {
18+
jsx: true
19+
}
20+
};
21+
22+
// -----------------------------------------------------------------------------
23+
// Tests
24+
// -----------------------------------------------------------------------------
25+
26+
var ruleTester = new RuleTester();
27+
ruleTester.run('no-array-index-key', rule, {
28+
valid: [
29+
{code: '<Foo key="foo" />;', parserOptions: parserOptions},
30+
{code: '<Foo key={i} />;', parserOptions: parserOptions},
31+
{code: '<Foo key={`foo-${i}`} />;', parserOptions: parserOptions},
32+
{code: '<Foo key={\'foo-\' + i} />;', parserOptions: parserOptions},
33+
34+
{
35+
code: 'foo.bar((baz, i) => <Foo key={i} />)',
36+
parserOptions: parserOptions
37+
},
38+
39+
{
40+
code: 'foo.bar((bar, i) => <Foo key={`foo-${i}`} />)',
41+
parserOptions: parserOptions
42+
},
43+
44+
{
45+
code: 'foo.bar((bar, i) => <Foo key={\'foo-\' + i} />)',
46+
parserOptions: parserOptions
47+
},
48+
49+
{
50+
code: 'foo.map((baz) => <Foo key={baz.id} />)',
51+
parserOptions: parserOptions
52+
},
53+
54+
{
55+
code: 'foo.map((baz, i) => <Foo key={baz.id} />)',
56+
parserOptions: parserOptions
57+
},
58+
59+
{
60+
code: 'foo.map((baz, i) => <Foo key={\'foo\' + baz.id} />)',
61+
parserOptions: parserOptions
62+
}
63+
],
64+
65+
invalid: [
66+
{
67+
code: 'foo.map((bar, i) => <Foo key={i} />)',
68+
errors: [{message: 'Do not use Array index in keys'}],
69+
parserOptions: parserOptions
70+
},
71+
72+
{
73+
code: 'foo.map((bar, anything) => <Foo key={anything} />)',
74+
errors: [{message: 'Do not use Array index in keys'}],
75+
parserOptions: parserOptions
76+
},
77+
78+
{
79+
code: 'foo.map((bar, i) => <Foo key={`foo-${i}`} />)',
80+
errors: [{message: 'Do not use Array index in keys'}],
81+
parserOptions: parserOptions
82+
},
83+
84+
{
85+
code: 'foo.map((bar, i) => <Foo key={\'foo-\' + i} />)',
86+
errors: [{message: 'Do not use Array index in keys'}],
87+
parserOptions: parserOptions
88+
},
89+
90+
{
91+
code: 'foo.map((bar, i) => <Foo key={\'foo-\' + i + \'-bar\'} />)',
92+
errors: [{message: 'Do not use Array index in keys'}],
93+
parserOptions: parserOptions
94+
},
95+
96+
{
97+
code: 'foo.map((bar, i) => React.createElement(\'Foo\', { key: i }))',
98+
errors: [{message: 'Do not use Array index in keys'}],
99+
parserOptions: parserOptions
100+
},
101+
102+
{
103+
code: 'foo.map((bar, i) => React.createElement(\'Foo\', { key: `foo-${i}` }))',
104+
errors: [{message: 'Do not use Array index in keys'}],
105+
parserOptions: parserOptions
106+
},
107+
108+
{
109+
code: 'foo.map((bar, i) => React.createElement(\'Foo\', { key: \'foo-\' + i }))',
110+
errors: [{message: 'Do not use Array index in keys'}],
111+
parserOptions: parserOptions
112+
},
113+
114+
{
115+
code: 'foo.map((bar, i) => React.createElement(\'Foo\', { key: \'foo-\' + i + \'-bar\' }))',
116+
errors: [{message: 'Do not use Array index in keys'}],
117+
parserOptions: parserOptions
118+
}
119+
]
120+
});

0 commit comments

Comments
 (0)