Skip to content

Commit

Permalink
Fix: no-useless-computed-key edge cases with class fields (refs #14857)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mdjermanovic
mdjermanovic committed Aug 8, 2021
1 parent ae6072b commit 317a069
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 19 deletions.
59 changes: 55 additions & 4 deletions docs/rules/no-useless-computed-key.md
View file
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
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
* 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
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"
}]
}
]
});

0 comments on commit 317a069

Please sign in to comment.