Skip to content

Commit 4f8a1ee

Browse files
ark120202Kai Cataldo
ark120202
authored and
Kai Cataldo
committed
Update: Add enforceForClassMembers option to no-useless-computed-key (#12110)
* Update: Add checkMethods option to no-useless-computed-key * Update: Don't flag `static ['constructor']` * Update: Flag static computed constructor * Update: Rename `checkMethods` to `enforceForClassMembers` * Chore: Valid class test cases don't have new option specified * Chore: Add tests with defaults and explicitly disabled option * Chore: Add class expression tests * Docs: Add `enforceForClassMembers` option to `no-useless-computed-key` * Chore: Resolve JSDoc lint warnings * Address review comments
1 parent 1a2eb99 commit 4f8a1ee

3 files changed

Lines changed: 294 additions & 37 deletions

File tree

‎docs/rules/no-useless-computed-key.md‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Disallow unnecessary computed property keys on objects (no-useless-computed-key)
1+
# Disallow unnecessary computed property keys in objects and classes (no-useless-computed-key)
22

33
It's unnecessary to use computed properties with literals such as:
44

@@ -16,8 +16,6 @@ var foo = {"a": "b"};
1616

1717
This rule disallows unnecessary usage of computed property keys.
1818

19-
## Examples
20-
2119
Examples of **incorrect** code for this rule:
2220

2321
```js
@@ -43,6 +41,35 @@ var c = { a: 0 };
4341
var c = { '0+1,234': 0 };
4442
```
4543

44+
## Options
45+
46+
This rule has an object option:
47+
48+
* `enforceForClassMembers` set to `true` additionally applies this rule to class members (Default `false`).
49+
50+
### enforceForClassMembers
51+
52+
By default, this rule does not check class declarations and class expressions,
53+
as the default value for `enforceForClassMembers` is `false`.
54+
55+
When `enforceForClassMembers` is set to `true`, the rule will also disallow unnecessary computed
56+
keys inside of class methods, getters and setters.
57+
58+
Examples of **incorrect** code for `{ "enforceForClassMembers": true }`:
59+
60+
```js
61+
/*eslint no-useless-computed-key: ["error", { "enforceForClassMembers": true }]*/
62+
63+
class Foo {
64+
[0]() {}
65+
['a']() {}
66+
get ['b']() {}
67+
set ['c'](value) {}
68+
69+
static ['a']() {}
70+
}
71+
```
72+
4673
## When Not To Use It
4774

4875
If you don't want to be notified about unnecessary computed property keys, you can safely disable this rule.

‎lib/rules/no-useless-computed-key.js‎

Lines changed: 60 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// Requirements
99
//------------------------------------------------------------------------------
1010

11+
const lodash = require("lodash");
1112
const astUtils = require("./utils/ast-utils");
1213

1314
//------------------------------------------------------------------------------
@@ -21,57 +22,83 @@ module.exports = {
2122
type: "suggestion",
2223

2324
docs: {
24-
description: "disallow unnecessary computed property keys in object literals",
25+
description: "disallow unnecessary computed property keys in objects and classes",
2526
category: "ECMAScript 6",
2627
recommended: false,
2728
url: "https://eslint.org/docs/rules/no-useless-computed-key"
2829
},
2930

30-
schema: [],
31+
schema: [{
32+
type: "object",
33+
properties: {
34+
enforceForClassMembers: {
35+
type: "boolean",
36+
default: false
37+
}
38+
},
39+
additionalProperties: false
40+
}],
3141
fixable: "code"
3242
},
3343
create(context) {
3444
const sourceCode = context.getSourceCode();
45+
const enforceForClassMembers = context.options[0] && context.options[0].enforceForClassMembers;
46+
47+
/**
48+
* Reports a given node if it violated this rule.
49+
* @param {ASTNode} node The node to check.
50+
* @returns {void}
51+
*/
52+
function check(node) {
53+
if (!node.computed) {
54+
return;
55+
}
3556

36-
return {
37-
Property(node) {
38-
if (!node.computed) {
39-
return;
40-
}
41-
42-
const key = node.key,
43-
nodeType = typeof key.value;
57+
const key = node.key,
58+
nodeType = typeof key.value;
4459

45-
if (key.type === "Literal" && (nodeType === "string" || nodeType === "number") && key.value !== "__proto__") {
46-
context.report({
47-
node,
48-
message: MESSAGE_UNNECESSARY_COMPUTED,
49-
data: { property: sourceCode.getText(key) },
50-
fix(fixer) {
51-
const leftSquareBracket = sourceCode.getFirstToken(node, astUtils.isOpeningBracketToken);
52-
const rightSquareBracket = sourceCode.getFirstTokenBetween(node.key, node.value, astUtils.isClosingBracketToken);
53-
const tokensBetween = sourceCode.getTokensBetween(leftSquareBracket, rightSquareBracket, 1);
60+
let allowedKey;
5461

55-
if (tokensBetween.slice(0, -1).some((token, index) =>
56-
sourceCode.getText().slice(token.range[1], tokensBetween[index + 1].range[0]).trim())) {
62+
if (node.type === "MethodDefinition") {
63+
allowedKey = node.static ? "prototype" : "constructor";
64+
} else {
65+
allowedKey = "__proto__";
66+
}
5767

58-
// If there are comments between the brackets and the property name, don't do a fix.
59-
return null;
60-
}
68+
if (key.type === "Literal" && (nodeType === "string" || nodeType === "number") && key.value !== allowedKey) {
69+
context.report({
70+
node,
71+
message: MESSAGE_UNNECESSARY_COMPUTED,
72+
data: { property: sourceCode.getText(key) },
73+
fix(fixer) {
74+
const leftSquareBracket = sourceCode.getFirstToken(node, astUtils.isOpeningBracketToken);
75+
const rightSquareBracket = sourceCode.getFirstTokenBetween(node.key, node.value, astUtils.isClosingBracketToken);
76+
const tokensBetween = sourceCode.getTokensBetween(leftSquareBracket, rightSquareBracket, 1);
77+
78+
if (tokensBetween.slice(0, -1).some((token, index) =>
79+
sourceCode.getText().slice(token.range[1], tokensBetween[index + 1].range[0]).trim())) {
80+
81+
// If there are comments between the brackets and the property name, don't do a fix.
82+
return null;
83+
}
6184

62-
const tokenBeforeLeftBracket = sourceCode.getTokenBefore(leftSquareBracket);
85+
const tokenBeforeLeftBracket = sourceCode.getTokenBefore(leftSquareBracket);
6386

64-
// Insert a space before the key to avoid changing identifiers, e.g. ({ get[2]() {} }) to ({ get2() {} })
65-
const needsSpaceBeforeKey = tokenBeforeLeftBracket.range[1] === leftSquareBracket.range[0] &&
66-
!astUtils.canTokensBeAdjacent(tokenBeforeLeftBracket, sourceCode.getFirstToken(key));
87+
// Insert a space before the key to avoid changing identifiers, e.g. ({ get[2]() {} }) to ({ get2() {} })
88+
const needsSpaceBeforeKey = tokenBeforeLeftBracket.range[1] === leftSquareBracket.range[0] &&
89+
!astUtils.canTokensBeAdjacent(tokenBeforeLeftBracket, sourceCode.getFirstToken(key));
6790

68-
const replacementKey = (needsSpaceBeforeKey ? " " : "") + key.raw;
91+
const replacementKey = (needsSpaceBeforeKey ? " " : "") + key.raw;
6992

70-
return fixer.replaceTextRange([leftSquareBracket.range[0], rightSquareBracket.range[1]], replacementKey);
71-
}
72-
});
73-
}
93+
return fixer.replaceTextRange([leftSquareBracket.range[0], rightSquareBracket.range[1]], replacementKey);
94+
}
95+
});
7496
}
97+
}
98+
99+
return {
100+
Property: check,
101+
MethodDefinition: enforceForClassMembers ? check : lodash.noop
75102
};
76103
}
77104
};

‎tests/lib/rules/no-useless-computed-key.js‎

Lines changed: 204 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,24 @@ ruleTester.run("no-useless-computed-key", rule, {
2323
"({ 'a': 0, b(){} })",
2424
"({ [x]: 0 });",
2525
"({ a: 0, [b](){} })",
26-
"({ ['__proto__']: [] })"
26+
"({ ['__proto__']: [] })",
27+
{ code: "class Foo { a() {} }", options: [{ enforceForClassMembers: true }] },
28+
{ code: "class Foo { 'a'() {} }", options: [{ enforceForClassMembers: true }] },
29+
{ code: "class Foo { [x]() {} }", options: [{ enforceForClassMembers: true }] },
30+
{ code: "class Foo { ['constructor']() {} }", options: [{ enforceForClassMembers: true }] },
31+
{ code: "class Foo { static ['prototype']() {} }", options: [{ enforceForClassMembers: true }] },
32+
{ code: "(class { 'a'() {} })", options: [{ enforceForClassMembers: true }] },
33+
{ code: "(class { [x]() {} })", options: [{ enforceForClassMembers: true }] },
34+
{ code: "(class { ['constructor']() {} })", options: [{ enforceForClassMembers: true }] },
35+
{ code: "(class { static ['prototype']() {} })", options: [{ enforceForClassMembers: true }] },
36+
"class Foo { ['x']() {} }",
37+
"(class { ['x']() {} })",
38+
"class Foo { static ['constructor']() {} }",
39+
"class Foo { ['prototype']() {} }",
40+
{ code: "class Foo { ['x']() {} }", options: [{ enforceForClassMembers: false }] },
41+
{ code: "(class { ['x']() {} })", options: [{ enforceForClassMembers: false }] },
42+
{ code: "class Foo { static ['constructor']() {} }", options: [{ enforceForClassMembers: false }] },
43+
{ code: "class Foo { ['prototype']() {} }", options: [{ enforceForClassMembers: false }] }
2744
],
2845
invalid: [
2946
{
@@ -168,6 +185,192 @@ ruleTester.run("no-useless-computed-key", rule, {
168185
errors: [{
169186
message: "Unnecessarily computed property [2] found.", type: "Property"
170187
}]
188+
}, {
189+
code: "class Foo { ['0']() {} }",
190+
output: "class Foo { '0'() {} }",
191+
options: [{ enforceForClassMembers: true }],
192+
errors: [{
193+
message: "Unnecessarily computed property ['0'] found.", type: "MethodDefinition"
194+
}]
195+
}, {
196+
code: "class Foo { ['0+1,234']() {} }",
197+
output: "class Foo { '0+1,234'() {} }",
198+
options: [{ enforceForClassMembers: true }],
199+
errors: [{
200+
message: "Unnecessarily computed property ['0+1,234'] found.", type: "MethodDefinition"
201+
}]
202+
}, {
203+
code: "class Foo { ['x']() {} }",
204+
output: "class Foo { 'x'() {} }",
205+
options: [{ enforceForClassMembers: true }],
206+
errors: [{
207+
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
208+
}]
209+
}, {
210+
code: "class Foo { [/* this comment prevents a fix */ 'x']() {} }",
211+
output: null,
212+
options: [{ enforceForClassMembers: true }],
213+
errors: [{
214+
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
215+
}]
216+
}, {
217+
code: "class Foo { ['x' /* this comment also prevents a fix */]() {} }",
218+
output: null,
219+
options: [{ enforceForClassMembers: true }],
220+
errors: [{
221+
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
222+
}]
223+
}, {
224+
code: "class Foo { [('x')]() {} }",
225+
output: "class Foo { 'x'() {} }",
226+
options: [{ enforceForClassMembers: true }],
227+
errors: [{
228+
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
229+
}]
230+
}, {
231+
code: "class Foo { *['x']() {} }",
232+
output: "class Foo { *'x'() {} }",
233+
options: [{ enforceForClassMembers: true }],
234+
errors: [{
235+
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
236+
}]
237+
}, {
238+
code: "class Foo { async ['x']() {} }",
239+
output: "class Foo { async 'x'() {} }",
240+
options: [{ enforceForClassMembers: true }],
241+
parserOptions: { ecmaVersion: 8 },
242+
errors: [{
243+
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
244+
}]
245+
}, {
246+
code: "class Foo { get[.2]() {} }",
247+
output: "class Foo { get.2() {} }",
248+
options: [{ enforceForClassMembers: true }],
249+
errors: [{
250+
message: "Unnecessarily computed property [.2] found.", type: "MethodDefinition"
251+
}]
252+
}, {
253+
code: "class Foo { set[.2](value) {} }",
254+
output: "class Foo { set.2(value) {} }",
255+
options: [{ enforceForClassMembers: true }],
256+
errors: [{
257+
message: "Unnecessarily computed property [.2] found.", type: "MethodDefinition"
258+
}]
259+
}, {
260+
code: "class Foo { async[.2]() {} }",
261+
output: "class Foo { async.2() {} }",
262+
options: [{ enforceForClassMembers: true }],
263+
parserOptions: { ecmaVersion: 8 },
264+
errors: [{
265+
message: "Unnecessarily computed property [.2] found.", type: "MethodDefinition"
266+
}]
267+
}, {
268+
code: "class Foo { [2]() {} }",
269+
output: "class Foo { 2() {} }",
270+
options: [{ enforceForClassMembers: true }],
271+
errors: [{
272+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
273+
}]
274+
}, {
275+
code: "class Foo { get [2]() {} }",
276+
output: "class Foo { get 2() {} }",
277+
options: [{ enforceForClassMembers: true }],
278+
errors: [{
279+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
280+
}]
281+
}, {
282+
code: "class Foo { set [2](value) {} }",
283+
output: "class Foo { set 2(value) {} }",
284+
options: [{ enforceForClassMembers: true }],
285+
errors: [{
286+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
287+
}]
288+
}, {
289+
code: "class Foo { async [2]() {} }",
290+
output: "class Foo { async 2() {} }",
291+
options: [{ enforceForClassMembers: true }],
292+
parserOptions: { ecmaVersion: 8 },
293+
errors: [{
294+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
295+
}]
296+
}, {
297+
code: "class Foo { get[2]() {} }",
298+
output: "class Foo { get 2() {} }",
299+
options: [{ enforceForClassMembers: true }],
300+
errors: [{
301+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
302+
}]
303+
}, {
304+
code: "class Foo { set[2](value) {} }",
305+
output: "class Foo { set 2(value) {} }",
306+
options: [{ enforceForClassMembers: true }],
307+
errors: [{
308+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
309+
}]
310+
}, {
311+
code: "class Foo { async[2]() {} }",
312+
output: "class Foo { async 2() {} }",
313+
options: [{ enforceForClassMembers: true }],
314+
parserOptions: { ecmaVersion: 8 },
315+
errors: [{
316+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
317+
}]
318+
}, {
319+
code: "class Foo { get['foo']() {} }",
320+
output: "class Foo { get'foo'() {} }",
321+
options: [{ enforceForClassMembers: true }],
322+
errors: [{
323+
message: "Unnecessarily computed property ['foo'] found.", type: "MethodDefinition"
324+
}]
325+
}, {
326+
code: "class Foo { *[2]() {} }",
327+
output: "class Foo { *2() {} }",
328+
options: [{ enforceForClassMembers: true }],
329+
errors: [{
330+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
331+
}]
332+
}, {
333+
code: "class Foo { async*[2]() {} }",
334+
output: "class Foo { async*2() {} }",
335+
options: [{ enforceForClassMembers: true }],
336+
errors: [{
337+
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
338+
}]
339+
}, {
340+
code: "class Foo { static ['constructor']() {} }",
341+
output: "class Foo { static 'constructor'() {} }",
342+
options: [{ enforceForClassMembers: true }],
343+
errors: [{
344+
message: "Unnecessarily computed property ['constructor'] found.", type: "MethodDefinition"
345+
}]
346+
}, {
347+
code: "class Foo { ['prototype']() {} }",
348+
output: "class Foo { 'prototype'() {} }",
349+
options: [{ enforceForClassMembers: true }],
350+
errors: [{
351+
message: "Unnecessarily computed property ['prototype'] found.", type: "MethodDefinition"
352+
}]
353+
}, {
354+
code: "(class { ['x']() {} })",
355+
output: "(class { 'x'() {} })",
356+
options: [{ enforceForClassMembers: true }],
357+
errors: [{
358+
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
359+
}]
360+
}, {
361+
code: "(class { static ['constructor']() {} })",
362+
output: "(class { static 'constructor'() {} })",
363+
options: [{ enforceForClassMembers: true }],
364+
errors: [{
365+
message: "Unnecessarily computed property ['constructor'] found.", type: "MethodDefinition"
366+
}]
367+
}, {
368+
code: "(class { ['prototype']() {} })",
369+
output: "(class { 'prototype'() {} })",
370+
options: [{ enforceForClassMembers: true }],
371+
errors: [{
372+
message: "Unnecessarily computed property ['prototype'] found.", type: "MethodDefinition"
373+
}]
171374
}
172375
]
173376
});

0 commit comments

Comments
 (0)