Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: no-useless-computed-key edge cases with class fields (refs #14857) #14903

Merged
merged 1 commit into from Aug 10, 2021
Merged

Fix: no-useless-computed-key edge cases with class fields (refs #14857) #14903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 55 additions & 4 deletions docs/rules/no-useless-computed-key.md
View file
Open in desktop
Expand Up @@ -20,7 +20,6 @@ Examples of **incorrect** code for this rule:

```js
/*eslint no-useless-computed-key: "error"*/
/*eslint-env es6*/

var a = { ['0']: 0 };
var a = { ['0+1,234']: 0 };
Expand All @@ -41,6 +40,18 @@ var c = { a: 0 };
var c = { '0+1,234': 0 };
```

Examples of additional **correct** code for this rule:

```js
/*eslint no-useless-computed-key: "error"*/

var c = {
"__proto__": foo, // defines object's prototype

["__proto__"]: bar // defines a property named "__proto__"
};
```

## Options

This rule has an object option:
Expand All @@ -52,24 +63,64 @@ This rule has an object option:
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.
When `enforceForClassMembers` is set to `true`, the rule will also disallow unnecessary computed keys inside of class fields, class methods, class getters, and class setters.

Examples of **incorrect** code for `{ "enforceForClassMembers": true }`:
Examples of **incorrect** code for this rule with the `{ "enforceForClassMembers": true }` option:

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

class Foo {
["foo"] = "bar";

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

static ["foo"] = "bar";

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

Examples of **correct** code for this rule with the `{ "enforceForClassMembers": true }` option:

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

class Foo {
"foo" = "bar";

0() {}
'a'() {}
get 'b'() {}
set 'c'(value) {}

static "foo" = "bar";

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

Examples of additional **correct** code for this rule with the `{ "enforceForClassMembers": true }` option:

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

class Foo {
["constructor"]; // instance field named "constructor"

"constructor"() {} // the constructor of this class

["constructor"]() {} // method named "constructor"

static ["constructor"]; // static field named "constructor"

static ["prototype"]; // runtime error, it would be a parsing error without `[]`
}
```

## When Not To Use It

If you don't want to be notified about unnecessary computed property keys, you can safely disable this rule.
87 changes: 72 additions & 15 deletions lib/rules/no-useless-computed-key.js
View file
Open in desktop
Expand Up @@ -10,6 +10,76 @@

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

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* Determines whether the computed key syntax is unnecessarily used for the given node.
* In particular, it determines whether removing the square brackets and using the content between them
Comment on lines +18 to +19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This documentation is helpful 👍🏻

* directly as the key (e.g. ['foo'] -> 'foo') would produce valid syntax and preserve the same behavior.
* Valid non-computed keys are only: identifiers, number literals and string literals.
* Only literals can preserve the same behavior, with a few exceptions for specific node types:
* Property
* - { ["__proto__"]: foo } defines a property named "__proto__"
* { "__proto__": foo } defines object's prototype
* PropertyDefinition
* - class C { ["constructor"]; } defines an instance field named "constructor"
* class C { "constructor"; } produces a parsing error
* - class C { static ["constructor"]; } defines a static field named "constructor"
* class C { static "constructor"; } produces a parsing error
* - class C { static ["prototype"]; } produces a runtime error (doesn't break the whole script)
* class C { static "prototype"; } produces a parsing error (breaks the whole script)
* MethodDefinition
* - class C { ["constructor"]() {} } defines a prototype method named "constructor"
* class C { "constructor"() {} } defines the constructor
* - class C { static ["prototype"]() {} } produces a runtime error (doesn't break the whole script)
* class C { static "prototype"() {} } produces a parsing error (breaks the whole script)
* @param {ASTNode} node The node to check. It can be `Property`, `PropertyDefinition` or `MethodDefinition`.
* @returns {void} `true` if the node has useless computed key.
*/
function hasUselessComputedKey(node) {
if (!node.computed) {
return false;
}

const { key } = node;

if (key.type !== "Literal") {
return false;
}

const { value } = key;

if (typeof value !== "number" && typeof value !== "string") {
return false;
}

switch (node.type) {
case "Property":
return value !== "__proto__";

case "PropertyDefinition":
if (node.static) {
return value !== "constructor" && value !== "prototype";
}

return value !== "constructor";

case "MethodDefinition":
if (node.static) {
return value !== "prototype";
}

return value !== "constructor";

/* istanbul ignore next */
default:
throw new Error(`Unexpected node type: ${node.type}`);
}

}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -51,22 +121,9 @@ module.exports = {
* @returns {void}
*/
function check(node) {
if (!node.computed) {
return;
}

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

let allowedKey;

if (node.type === "MethodDefinition") {
allowedKey = node.static ? "prototype" : "constructor";
} else {
allowedKey = "__proto__";
}
if (hasUselessComputedKey(node)) {
const { key } = node;

if (key.type === "Literal" && (nodeType === "string" || nodeType === "number") && key.value !== allowedKey) {
context.report({
node,
messageId: "unnecessarilyComputedProperty",
Expand Down
64 changes: 64 additions & 0 deletions tests/lib/rules/no-useless-computed-key.js
View file
Open in desktop
Expand Up @@ -42,6 +42,9 @@ ruleTester.run("no-useless-computed-key", rule, {
{ code: "class Foo { static ['constructor']() {} }", options: [{ enforceForClassMembers: false }] },
{ code: "class Foo { ['prototype']() {} }", options: [{ enforceForClassMembers: false }] },
{ code: "class Foo { a }", options: [{ enforceForClassMembers: true }] },
{ code: "class Foo { ['constructor'] }", options: [{ enforceForClassMembers: true }] },
{ code: "class Foo { static ['constructor'] }", options: [{ enforceForClassMembers: true }] },
{ code: "class Foo { static ['prototype'] }", options: [{ enforceForClassMembers: true }] },

/*
* Well-known browsers throw syntax error bigint literals on property names,
Expand Down Expand Up @@ -241,6 +244,22 @@ ruleTester.run("no-useless-computed-key", rule, {
data: { property: "2" },
type: "Property"
}]
}, {
code: "({ ['constructor']: 1 })",
output: "({ 'constructor': 1 })",
errors: [{
messageId: "unnecessarilyComputedProperty",
data: { property: "'constructor'" },
type: "Property"
}]
}, {
code: "({ ['prototype']: 1 })",
output: "({ 'prototype': 1 })",
errors: [{
messageId: "unnecessarilyComputedProperty",
data: { property: "'prototype'" },
type: "Property"
}]
}, {
code: "class Foo { ['0']() {} }",
output: "class Foo { '0'() {} }",
Expand Down Expand Up @@ -461,6 +480,24 @@ ruleTester.run("no-useless-computed-key", rule, {
data: { property: "'x'" },
type: "MethodDefinition"
}]
}, {
code: "(class { ['__proto__']() {} })",
output: "(class { '__proto__'() {} })",
options: [{ enforceForClassMembers: true }],
errors: [{
messageId: "unnecessarilyComputedProperty",
data: { property: "'__proto__'" },
type: "MethodDefinition"
}]
}, {
code: "(class { static ['__proto__']() {} })",
output: "(class { static '__proto__'() {} })",
options: [{ enforceForClassMembers: true }],
errors: [{
messageId: "unnecessarilyComputedProperty",
data: { property: "'__proto__'" },
type: "MethodDefinition"
}]
}, {
code: "(class { static ['constructor']() {} })",
output: "(class { static 'constructor'() {} })",
Expand Down Expand Up @@ -515,6 +552,33 @@ ruleTester.run("no-useless-computed-key", rule, {
data: { property: "'#foo'" },
type: "PropertyDefinition"
}]
}, {
code: "(class { ['__proto__'] })",
output: "(class { '__proto__' })",
options: [{ enforceForClassMembers: true }],
errors: [{
messageId: "unnecessarilyComputedProperty",
data: { property: "'__proto__'" },
type: "PropertyDefinition"
}]
}, {
code: "(class { static ['__proto__'] })",
output: "(class { static '__proto__' })",
options: [{ enforceForClassMembers: true }],
errors: [{
messageId: "unnecessarilyComputedProperty",
data: { property: "'__proto__'" },
type: "PropertyDefinition"
}]
}, {
code: "(class { ['prototype'] })",
output: "(class { 'prototype' })",
options: [{ enforceForClassMembers: true }],
errors: [{
messageId: "unnecessarilyComputedProperty",
data: { property: "'prototype'" },
type: "PropertyDefinition"
}]
}
]
});