Skip to content

Commit

Permalink
Update: Add enforceForClassMembers option to no-useless-computed-key (#…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@ark120202 @kaicataldo
ark120202 authored and kaicataldo committed Nov 22, 2019
1 parent 1a2eb99 commit 4f8a1ee
Show file tree
Hide file tree
Showing 3 changed files with 294 additions and 37 deletions.
33 changes: 30 additions & 3 deletions docs/rules/no-useless-computed-key.md
View file
@@ -1,4 +1,4 @@
# Disallow unnecessary computed property keys on objects (no-useless-computed-key) # Disallow unnecessary computed property keys in objects and classes (no-useless-computed-key)


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


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


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


## Examples

Examples of **incorrect** code for this rule: Examples of **incorrect** code for this rule:


```js ```js
Expand All @@ -43,6 +41,35 @@ var c = { a: 0 };
var c = { '0+1,234': 0 }; var c = { '0+1,234': 0 };
``` ```


## Options

This rule has an object option:

* `enforceForClassMembers` set to `true` additionally applies this rule to class members (Default `false`).

### enforceForClassMembers

By default, this rule does not check class declarations and class expressions,
as the default value for `enforceForClassMembers` is `false`.

When `enforceForClassMembers` is set to `true`, the rule will also disallow unnecessary computed
keys inside of class methods, getters and setters.

Examples of **incorrect** code for `{ "enforceForClassMembers": true }`:

```js
/*eslint no-useless-computed-key: ["error", { "enforceForClassMembers": true }]*/

class Foo {
[0]() {}
['a']() {}
get ['b']() {}
set ['c'](value) {}

static ['a']() {}
}
```

## When Not To Use It ## When Not To Use It


If you don't want to be notified about unnecessary computed property keys, you can safely disable this rule. If you don't want to be notified about unnecessary computed property keys, you can safely disable this rule.
93 changes: 60 additions & 33 deletions lib/rules/no-useless-computed-key.js
View file
Expand Up @@ -8,6 +8,7 @@
// Requirements // Requirements
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------


const lodash = require("lodash");
const astUtils = require("./utils/ast-utils"); const astUtils = require("./utils/ast-utils");


//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
Expand All @@ -21,57 +22,83 @@ module.exports = {
type: "suggestion", type: "suggestion",


docs: { docs: {
description: "disallow unnecessary computed property keys in object literals", description: "disallow unnecessary computed property keys in objects and classes",
category: "ECMAScript 6", category: "ECMAScript 6",
recommended: false, recommended: false,
url: "https://eslint.org/docs/rules/no-useless-computed-key" url: "https://eslint.org/docs/rules/no-useless-computed-key"
}, },


schema: [], schema: [{
type: "object",
properties: {
enforceForClassMembers: {
type: "boolean",
default: false
}
},
additionalProperties: false
}],
fixable: "code" fixable: "code"
}, },
create(context) { create(context) {
const sourceCode = context.getSourceCode(); const sourceCode = context.getSourceCode();
const enforceForClassMembers = context.options[0] && context.options[0].enforceForClassMembers;

/**
* Reports a given node if it violated this rule.
* @param {ASTNode} node The node to check.
* @returns {void}
*/
function check(node) {
if (!node.computed) {
return;
}


return { const key = node.key,
Property(node) { nodeType = typeof key.value;
if (!node.computed) {
return;
}

const key = node.key,
nodeType = typeof key.value;


if (key.type === "Literal" && (nodeType === "string" || nodeType === "number") && key.value !== "__proto__") { let allowedKey;
context.report({
node,
message: MESSAGE_UNNECESSARY_COMPUTED,
data: { property: sourceCode.getText(key) },
fix(fixer) {
const leftSquareBracket = sourceCode.getFirstToken(node, astUtils.isOpeningBracketToken);
const rightSquareBracket = sourceCode.getFirstTokenBetween(node.key, node.value, astUtils.isClosingBracketToken);
const tokensBetween = sourceCode.getTokensBetween(leftSquareBracket, rightSquareBracket, 1);


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


// If there are comments between the brackets and the property name, don't do a fix. if (key.type === "Literal" && (nodeType === "string" || nodeType === "number") && key.value !== allowedKey) {
return null; context.report({
} node,
message: MESSAGE_UNNECESSARY_COMPUTED,
data: { property: sourceCode.getText(key) },
fix(fixer) {
const leftSquareBracket = sourceCode.getFirstToken(node, astUtils.isOpeningBracketToken);
const rightSquareBracket = sourceCode.getFirstTokenBetween(node.key, node.value, astUtils.isClosingBracketToken);
const tokensBetween = sourceCode.getTokensBetween(leftSquareBracket, rightSquareBracket, 1);

if (tokensBetween.slice(0, -1).some((token, index) =>
sourceCode.getText().slice(token.range[1], tokensBetween[index + 1].range[0]).trim())) {

// If there are comments between the brackets and the property name, don't do a fix.
return null;
}


const tokenBeforeLeftBracket = sourceCode.getTokenBefore(leftSquareBracket); const tokenBeforeLeftBracket = sourceCode.getTokenBefore(leftSquareBracket);


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


const replacementKey = (needsSpaceBeforeKey ? " " : "") + key.raw; const replacementKey = (needsSpaceBeforeKey ? " " : "") + key.raw;


return fixer.replaceTextRange([leftSquareBracket.range[0], rightSquareBracket.range[1]], replacementKey); return fixer.replaceTextRange([leftSquareBracket.range[0], rightSquareBracket.range[1]], replacementKey);
} }
}); });
}
} }
}

return {
Property: check,
MethodDefinition: enforceForClassMembers ? check : lodash.noop
}; };
} }
}; };
205 changes: 204 additions & 1 deletion tests/lib/rules/no-useless-computed-key.js
View file
Expand Up @@ -23,7 +23,24 @@ ruleTester.run("no-useless-computed-key", rule, {
"({ 'a': 0, b(){} })", "({ 'a': 0, b(){} })",
"({ [x]: 0 });", "({ [x]: 0 });",
"({ a: 0, [b](){} })", "({ a: 0, [b](){} })",
"({ ['__proto__']: [] })" "({ ['__proto__']: [] })",
{ code: "class Foo { a() {} }", options: [{ enforceForClassMembers: true }] },
{ code: "class Foo { 'a'() {} }", options: [{ enforceForClassMembers: true }] },
{ code: "class Foo { [x]() {} }", options: [{ enforceForClassMembers: true }] },
{ code: "class Foo { ['constructor']() {} }", options: [{ enforceForClassMembers: true }] },
{ code: "class Foo { static ['prototype']() {} }", options: [{ enforceForClassMembers: true }] },
{ code: "(class { 'a'() {} })", options: [{ enforceForClassMembers: true }] },
{ code: "(class { [x]() {} })", options: [{ enforceForClassMembers: true }] },
{ code: "(class { ['constructor']() {} })", options: [{ enforceForClassMembers: true }] },
{ code: "(class { static ['prototype']() {} })", options: [{ enforceForClassMembers: true }] },
"class Foo { ['x']() {} }",
"(class { ['x']() {} })",
"class Foo { static ['constructor']() {} }",
"class Foo { ['prototype']() {} }",
{ code: "class Foo { ['x']() {} }", options: [{ enforceForClassMembers: false }] },
{ code: "(class { ['x']() {} })", options: [{ enforceForClassMembers: false }] },
{ code: "class Foo { static ['constructor']() {} }", options: [{ enforceForClassMembers: false }] },
{ code: "class Foo { ['prototype']() {} }", options: [{ enforceForClassMembers: false }] }
], ],
invalid: [ invalid: [
{ {
Expand Down Expand Up @@ -168,6 +185,192 @@ ruleTester.run("no-useless-computed-key", rule, {
errors: [{ errors: [{
message: "Unnecessarily computed property [2] found.", type: "Property" message: "Unnecessarily computed property [2] found.", type: "Property"
}] }]
}, {
code: "class Foo { ['0']() {} }",
output: "class Foo { '0'() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['0'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { ['0+1,234']() {} }",
output: "class Foo { '0+1,234'() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['0+1,234'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { ['x']() {} }",
output: "class Foo { 'x'() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { [/* this comment prevents a fix */ 'x']() {} }",
output: null,
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { ['x' /* this comment also prevents a fix */]() {} }",
output: null,
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { [('x')]() {} }",
output: "class Foo { 'x'() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { *['x']() {} }",
output: "class Foo { *'x'() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { async ['x']() {} }",
output: "class Foo { async 'x'() {} }",
options: [{ enforceForClassMembers: true }],
parserOptions: { ecmaVersion: 8 },
errors: [{
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { get[.2]() {} }",
output: "class Foo { get.2() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [.2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { set[.2](value) {} }",
output: "class Foo { set.2(value) {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [.2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { async[.2]() {} }",
output: "class Foo { async.2() {} }",
options: [{ enforceForClassMembers: true }],
parserOptions: { ecmaVersion: 8 },
errors: [{
message: "Unnecessarily computed property [.2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { [2]() {} }",
output: "class Foo { 2() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { get [2]() {} }",
output: "class Foo { get 2() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { set [2](value) {} }",
output: "class Foo { set 2(value) {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { async [2]() {} }",
output: "class Foo { async 2() {} }",
options: [{ enforceForClassMembers: true }],
parserOptions: { ecmaVersion: 8 },
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { get[2]() {} }",
output: "class Foo { get 2() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { set[2](value) {} }",
output: "class Foo { set 2(value) {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { async[2]() {} }",
output: "class Foo { async 2() {} }",
options: [{ enforceForClassMembers: true }],
parserOptions: { ecmaVersion: 8 },
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { get['foo']() {} }",
output: "class Foo { get'foo'() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['foo'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { *[2]() {} }",
output: "class Foo { *2() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { async*[2]() {} }",
output: "class Foo { async*2() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property [2] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { static ['constructor']() {} }",
output: "class Foo { static 'constructor'() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['constructor'] found.", type: "MethodDefinition"
}]
}, {
code: "class Foo { ['prototype']() {} }",
output: "class Foo { 'prototype'() {} }",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['prototype'] found.", type: "MethodDefinition"
}]
}, {
code: "(class { ['x']() {} })",
output: "(class { 'x'() {} })",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['x'] found.", type: "MethodDefinition"
}]
}, {
code: "(class { static ['constructor']() {} })",
output: "(class { static 'constructor'() {} })",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['constructor'] found.", type: "MethodDefinition"
}]
}, {
code: "(class { ['prototype']() {} })",
output: "(class { 'prototype'() {} })",
options: [{ enforceForClassMembers: true }],
errors: [{
message: "Unnecessarily computed property ['prototype'] found.", type: "MethodDefinition"
}]
} }
] ]
}); });

0 comments on commit 4f8a1ee

Please sign in to comment.