diff --git a/docs/rules/no-useless-computed-key.md b/docs/rules/no-useless-computed-key.md index 9b3c592849b..2d3ddb3d50b 100644 --- a/docs/rules/no-useless-computed-key.md +++ b/docs/rules/no-useless-computed-key.md @@ -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 }; @@ -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: @@ -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. diff --git a/lib/rules/no-useless-computed-key.js b/lib/rules/no-useless-computed-key.js index 08df712657c..a6643061928 100644 --- a/lib/rules/no-useless-computed-key.js +++ b/lib/rules/no-useless-computed-key.js @@ -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 //------------------------------------------------------------------------------ @@ -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", diff --git a/tests/lib/rules/no-useless-computed-key.js b/tests/lib/rules/no-useless-computed-key.js index a7bf323cf38..37f31118878 100644 --- a/tests/lib/rules/no-useless-computed-key.js +++ b/tests/lib/rules/no-useless-computed-key.js @@ -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, @@ -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'() {} }", @@ -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'() {} })", @@ -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" + }] } ] });