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:

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

This rule disallows unnecessary usage of computed property keys.

## Examples

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

```js
Expand All @@ -43,6 +41,35 @@ var c = { a: 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

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
//------------------------------------------------------------------------------

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

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

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

schema: [],
schema: [{
type: "object",
properties: {
enforceForClassMembers: {
type: "boolean",
default: false
}
},
additionalProperties: false
}],
fixable: "code"
},
create(context) {
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 {
Property(node) {
if (!node.computed) {
return;
}

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

if (key.type === "Literal" && (nodeType === "string" || nodeType === "number") && key.value !== "__proto__") {
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);
let allowedKey;

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

// If there are comments between the brackets and the property name, don't do a fix.
return null;
}
if (key.type === "Literal" && (nodeType === "string" || nodeType === "number") && key.value !== 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) =>
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() {} })
const needsSpaceBeforeKey = tokenBeforeLeftBracket.range[1] === leftSquareBracket.range[0] &&
!astUtils.canTokensBeAdjacent(tokenBeforeLeftBracket, sourceCode.getFirstToken(key));
// Insert a space before the key to avoid changing identifiers, e.g. ({ get[2]() {} }) to ({ get2() {} })
const needsSpaceBeforeKey = tokenBeforeLeftBracket.range[1] === leftSquareBracket.range[0] &&
!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(){} })",
"({ [x]: 0 });",
"({ 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: [
{
Expand Down Expand Up @@ -168,6 +185,192 @@ ruleTester.run("no-useless-computed-key", rule, {
errors: [{
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.