From 96872c35c6d32825e1a3bbd4a5337837355270e8 Mon Sep 17 00:00:00 2001 From: John Sanders Date: Mon, 13 Apr 2020 19:30:12 -0700 Subject: [PATCH 1/6] feat(eslint-plugin): add no-identifier-enum-member rule --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-identifier-enum-member.md | 56 +++++++++++++++++++ packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-identifier-enum-member.ts | 46 +++++++++++++++ .../rules/no-identifier-enum-member.test.ts | 44 +++++++++++++++ 6 files changed, 150 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-identifier-enum-member.md create mode 100644 packages/eslint-plugin/src/rules/no-identifier-enum-member.ts create mode 100644 packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 7ed22981926..1d0baef454a 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -117,6 +117,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces | | | | | [`@typescript-eslint/no-floating-promises`](./docs/rules/no-floating-promises.md) | Requires Promise-like values to be handled appropriately | | | :thought_balloon: | | [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop | :heavy_check_mark: | | :thought_balloon: | +| [`@typescript-eslint/no-identifier-enum-member`](./docs/rules/no-identifier-enum-member.md) | Disallow identifier (aka variable) enum members | | | | | [`@typescript-eslint/no-implied-eval`](./docs/rules/no-implied-eval.md) | Disallow the use of `eval()`-like methods | | | :thought_balloon: | | [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor` | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/no-identifier-enum-member.md b/packages/eslint-plugin/docs/rules/no-identifier-enum-member.md new file mode 100644 index 00000000000..5454e50d649 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-identifier-enum-member.md @@ -0,0 +1,56 @@ +# Disallow identifier (aka variable) enum members (`no-identifier-enum-member`) + +TypeScript allows the value of an enum member to be any valid JavaScript expression. However, because enums create their own scope whereby +each enum member becomes a variable in that scope, unexpected values could be used at runtime. Example: + +```ts +const imOutside = 2; +const b = 2; +enum Foo { + outer = imOutside, + a = 1, + b = a, + c = b, + // does c == Foo.b == Foo.c == 1? + // or does c == b == 2? +} +``` + +The answer is that `Foo.c` will be `1` at runtime. The [playground](https://www.typescriptlang.org/play/#src=const%20imOutside%20%3D%202%3B%0D%0Aconst%20b%20%3D%202%3B%0D%0Aenum%20Foo%20%7B%0D%0A%20%20%20%20outer%20%3D%20imOutside%2C%0D%0A%20%20%20%20a%20%3D%201%2C%0D%0A%20%20%20%20b%20%3D%20a%2C%0D%0A%20%20%20%20c%20%3D%20b%2C%0D%0A%20%20%20%20%2F%2F%20does%20c%20%3D%3D%20Foo.b%20%3D%3D%20Foo.c%20%3D%3D%201%3F%0D%0A%20%20%20%20%2F%2F%20or%20does%20c%20%3D%3D%20b%20%3D%3D%202%3F%0D%0A%7D) illustrates this quite nicely. + +## Rule Details + +This rule is meant to prevent unexpected results in code by prohibiting the use of identifiers (aka variables) as enum members to prevent unexpected runtime behavior. All other values should be allowed since they do not have this particular issue. + +Examples of **incorrect** code for this rule: + +```ts +const str = 'Test'; +enum Invalid { + A = str, // Variable assignment +} +``` + +Examples of **correct** code for this rule: + +```ts +enum Valid { + A, + B = 'TestStr', // A regular string + C = 4, // A number + D = null, + E = /some_regex/, + F = 2 + 2, // Expressions + G = {}, // Objects + H = [], // Arrays + I = new Set([1, 2, 3]), // Any constructable class +} +``` + +## Options + +There are no options. + +## When Not To Use It + +If you want the value of an enum member to be stored as a variable, do not use this rule. diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index c736840a3ff..1c6fbf38e76 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -44,6 +44,7 @@ "@typescript-eslint/no-extraneous-class": "error", "@typescript-eslint/no-floating-promises": "error", "@typescript-eslint/no-for-in-array": "error", + "@typescript-eslint/no-identifier-enum-member": "error", "@typescript-eslint/no-implied-eval": "error", "@typescript-eslint/no-inferrable-types": "error", "no-magic-numbers": "off", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 901d31a8b9f..12e0f1dbbc9 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -39,6 +39,7 @@ import noFloatingPromises from './no-floating-promises'; import noForInArray from './no-for-in-array'; import noImpliedEval from './no-implied-eval'; import noInferrableTypes from './no-inferrable-types'; +import noIdentifierEnumMember from './no-identifier-enum-member'; import noMagicNumbers from './no-magic-numbers'; import noMisusedNew from './no-misused-new'; import noMisusedPromises from './no-misused-promises'; @@ -169,6 +170,7 @@ export default { 'prefer-for-of': preferForOf, 'prefer-function-type': preferFunctionType, 'prefer-includes': preferIncludes, + 'no-identifier-enum-member': noIdentifierEnumMember, 'prefer-namespace-keyword': preferNamespaceKeyword, 'prefer-nullish-coalescing': preferNullishCoalescing, 'prefer-optional-chain': preferOptionalChain, diff --git a/packages/eslint-plugin/src/rules/no-identifier-enum-member.ts b/packages/eslint-plugin/src/rules/no-identifier-enum-member.ts new file mode 100644 index 00000000000..56e2dcc4cf0 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-identifier-enum-member.ts @@ -0,0 +1,46 @@ +import { createRule } from '../util'; +import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { TSEnumMember } from '../../../typescript-estree/dist/ts-estree/ts-estree'; + +export enum MessageId { + NoVariable = 'noVariable', +} + +type Options = []; + +export default createRule({ + name: 'no-identifier-enum-member', + meta: { + type: 'suggestion', + docs: { + description: 'Disallow identifier (aka variable) enum members', + category: 'Best Practices', + recommended: false, + requiresTypeChecking: false, + }, + messages: { + [MessageId.NoVariable]: `Enum member can not be an ${AST_NODE_TYPES.Identifier}.`, + }, + schema: [], + }, + defaultOptions: [], + create(context) { + return { + TSEnumMember(node: TSEnumMember): void { + // If there is no initializer, then this node is just the name of the member, so ignore. + if ( + node.initializer != null && + node.initializer.type === AST_NODE_TYPES.Identifier + ) { + context.report({ + node: node.id, + messageId: MessageId.NoVariable, + data: { + type: node.initializer.type, + }, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts b/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts new file mode 100644 index 00000000000..bcdcab4a31b --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts @@ -0,0 +1,44 @@ +import rule, { MessageId } from '../../src/rules/no-identifier-enum-member'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('prefer-literal-enum-member', rule, { + valid: [ + ` +enum Valid { + A = /test/, + B = 'test', + C = 42, + D = null, + E, + F = {}, + G = [], + H = new Set(), + I = 2 + 2, +} + `, + ], + invalid: [ + { + code: ` +const variable = 'Test', +enum Foo { + A = 'TestStr', + B = 2, + C, + V = variable, +} + `, + errors: [ + { + messageId: MessageId.NoVariable, + line: 7, + column: 3, + }, + ], + }, + ], +}); From 63d5a2a469000a5fea1cb36d3df580ebc8f3ac2e Mon Sep 17 00:00:00 2001 From: John Sanders Date: Mon, 13 Apr 2020 20:45:25 -0700 Subject: [PATCH 2/6] fix: ran lint-fix --- .../eslint-plugin/tests/rules/no-identifier-enum-member.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts b/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts index bcdcab4a31b..a3bd85b6781 100644 --- a/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts +++ b/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts @@ -24,7 +24,7 @@ enum Valid { invalid: [ { code: ` -const variable = 'Test', +const variable = 'Test'; enum Foo { A = 'TestStr', B = 2, From c9108848b4f1d2ad36c08f2a5b6b7a8a11fe8686 Mon Sep 17 00:00:00 2001 From: John Sanders Date: Tue, 16 Jun 2020 23:50:18 -0700 Subject: [PATCH 3/6] switch rule to prefer-literal and disallow most expression types --- packages/eslint-plugin/README.md | 2 +- ...ember.md => prefer-literal-enum-member.md} | 21 +- packages/eslint-plugin/src/configs/all.ts | 1 + packages/eslint-plugin/src/rules/index.ts | 10 +- ...ember.ts => prefer-literal-enum-member.ts} | 19 +- .../rules/no-identifier-enum-member.test.ts | 44 ---- .../rules/prefer-literal-enum-member.test.ts | 207 ++++++++++++++++++ 7 files changed, 229 insertions(+), 75 deletions(-) rename packages/eslint-plugin/docs/rules/{no-identifier-enum-member.md => prefer-literal-enum-member.md} (50%) rename packages/eslint-plugin/src/rules/{no-identifier-enum-member.ts => prefer-literal-enum-member.ts} (66%) delete mode 100644 packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts create mode 100644 packages/eslint-plugin/tests/rules/prefer-literal-enum-member.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index efd6f508091..958212c4b2d 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -114,7 +114,6 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces | | | | | [`@typescript-eslint/no-floating-promises`](./docs/rules/no-floating-promises.md) | Requires Promise-like values to be handled appropriately | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop | :heavy_check_mark: | | :thought_balloon: | -| [`@typescript-eslint/no-identifier-enum-member`](./docs/rules/no-identifier-enum-member.md) | Disallow identifier (aka variable) enum members | | | | | [`@typescript-eslint/no-implied-eval`](./docs/rules/no-implied-eval.md) | Disallow the use of `eval()`-like methods | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/no-invalid-void-type`](./docs/rules/no-invalid-void-type.md) | Disallows usage of `void` type outside of generic or return types | | | | @@ -143,6 +142,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/prefer-for-of`](./docs/rules/prefer-for-of.md) | Prefer a ‘for-of’ loop over a standard ‘for’ loop if the index is only used to access the array being iterated | | | | | [`@typescript-eslint/prefer-function-type`](./docs/rules/prefer-function-type.md) | Use function types instead of interfaces with call signatures | | :wrench: | | | [`@typescript-eslint/prefer-includes`](./docs/rules/prefer-includes.md) | Enforce `includes` method over `indexOf` method | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/prefer-literal-enum-member`](./docs/rules/prefer-literal-enum-member.md) | Require that all enum members be literal values to prevent unintended enum member name shadow issues | | | | | [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/prefer-nullish-coalescing`](./docs/rules/prefer-nullish-coalescing.md) | Enforce the usage of the nullish coalescing operator instead of logical chaining | | | :thought_balloon: | | [`@typescript-eslint/prefer-optional-chain`](./docs/rules/prefer-optional-chain.md) | Prefer using concise optional chain expressions instead of chained logical ands | | | | diff --git a/packages/eslint-plugin/docs/rules/no-identifier-enum-member.md b/packages/eslint-plugin/docs/rules/prefer-literal-enum-member.md similarity index 50% rename from packages/eslint-plugin/docs/rules/no-identifier-enum-member.md rename to packages/eslint-plugin/docs/rules/prefer-literal-enum-member.md index 5454e50d649..e2350f35711 100644 --- a/packages/eslint-plugin/docs/rules/no-identifier-enum-member.md +++ b/packages/eslint-plugin/docs/rules/prefer-literal-enum-member.md @@ -1,7 +1,6 @@ -# Disallow identifier (aka variable) enum members (`no-identifier-enum-member`) +# Require that all enum members be literal values to prevent unintended enum member name shadow issues (`prefer-literal-enum-member`) -TypeScript allows the value of an enum member to be any valid JavaScript expression. However, because enums create their own scope whereby -each enum member becomes a variable in that scope, unexpected values could be used at runtime. Example: +TypeScript allows the value of an enum member to be many different kinds of valid JavaScript expressions. However, because enums create their own scope whereby each enum member becomes a variable in that scope, unexpected values could be used at runtime. Example: ```ts const imOutside = 2; @@ -20,7 +19,7 @@ The answer is that `Foo.c` will be `1` at runtime. The [playground](https://www. ## Rule Details -This rule is meant to prevent unexpected results in code by prohibiting the use of identifiers (aka variables) as enum members to prevent unexpected runtime behavior. All other values should be allowed since they do not have this particular issue. +This rule is meant to prevent unexpected results in code by requiring the use of literal values as enum members to prevent unexpected runtime behavior. Template literals, arrays, objects, constructors, and all other expression types can end up using a variable from its scope or the parent scope, which can result in the same unexpected behavior at runtime. Examples of **incorrect** code for this rule: @@ -28,6 +27,10 @@ Examples of **incorrect** code for this rule: const str = 'Test'; enum Invalid { A = str, // Variable assignment + B = {}, // Object assignment + C = `A template literal string`, // Template literal + D = new Set(1, 2, 3), // Constructor in assignment + E = 2 + 2, // Expression assignment } ``` @@ -40,17 +43,9 @@ enum Valid { C = 4, // A number D = null, E = /some_regex/, - F = 2 + 2, // Expressions - G = {}, // Objects - H = [], // Arrays - I = new Set([1, 2, 3]), // Any constructable class } ``` -## Options - -There are no options. - ## When Not To Use It -If you want the value of an enum member to be stored as a variable, do not use this rule. +If you want use anything other than simple literals as an enum value. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 642d6f736d0..4c92661518c 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -96,6 +96,7 @@ export = { '@typescript-eslint/prefer-for-of': 'error', '@typescript-eslint/prefer-function-type': 'error', '@typescript-eslint/prefer-includes': 'error', + '@typescript-eslint/prefer-literal-enum-member': 'error', '@typescript-eslint/prefer-namespace-keyword': 'error', '@typescript-eslint/prefer-nullish-coalescing': 'error', '@typescript-eslint/prefer-optional-chain': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 731666e05bc..73083bedfe4 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -17,7 +17,9 @@ import explicitMemberAccessibility from './explicit-member-accessibility'; import explicitModuleBoundaryTypes from './explicit-module-boundary-types'; import funcCallSpacing from './func-call-spacing'; import indent from './indent'; +import initDeclarations from './init-declarations'; import keywordSpacing from './keyword-spacing'; +import linesBetweenClassMembers from './lines-between-class-members'; import memberDelimiterStyle from './member-delimiter-style'; import memberOrdering from './member-ordering'; import methodSignatureStyle from './method-signature-style'; @@ -35,11 +37,12 @@ import noExtraParens from './no-extra-parens'; import noExtraSemi from './no-extra-semi'; import noFloatingPromises from './no-floating-promises'; import noForInArray from './no-for-in-array'; -import noIdentifierEnumMember from './no-identifier-enum-member'; +import preferLiteralEnumMember from './prefer-literal-enum-member'; import noImpliedEval from './no-implied-eval'; import noInferrableTypes from './no-inferrable-types'; import noInvalidThis from './no-invalid-this'; import noInvalidVoidType from './no-invalid-void-type'; +import noLossOfPrecision from './no-loss-of-precision'; import noMagicNumbers from './no-magic-numbers'; import noMisusedNew from './no-misused-new'; import noMisusedPromises from './no-misused-promises'; @@ -86,7 +89,6 @@ import requireAwait from './require-await'; import restrictPlusOperands from './restrict-plus-operands'; import restrictTemplateExpressions from './restrict-template-expressions'; import returnAwait from './return-await'; -import initDeclarations from './init-declarations'; import semi from './semi'; import spaceBeforeFunctionParen from './space-before-function-paren'; import strictBooleanExpressions from './strict-boolean-expressions'; @@ -96,8 +98,6 @@ import typeAnnotationSpacing from './type-annotation-spacing'; import typedef from './typedef'; import unboundMethod from './unbound-method'; import unifiedSignatures from './unified-signatures'; -import linesBetweenClassMembers from './lines-between-class-members'; -import noLossOfPrecision from './no-loss-of-precision'; export default { 'adjacent-overload-signatures': adjacentOverloadSignatures, @@ -172,7 +172,7 @@ export default { 'prefer-for-of': preferForOf, 'prefer-function-type': preferFunctionType, 'prefer-includes': preferIncludes, - 'no-identifier-enum-member': noIdentifierEnumMember, + 'prefer-literal-enum-member': preferLiteralEnumMember, 'prefer-namespace-keyword': preferNamespaceKeyword, 'prefer-nullish-coalescing': preferNullishCoalescing, 'prefer-optional-chain': preferOptionalChain, diff --git a/packages/eslint-plugin/src/rules/no-identifier-enum-member.ts b/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts similarity index 66% rename from packages/eslint-plugin/src/rules/no-identifier-enum-member.ts rename to packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts index 56e2dcc4cf0..6b13269b959 100644 --- a/packages/eslint-plugin/src/rules/no-identifier-enum-member.ts +++ b/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts @@ -2,24 +2,19 @@ import { createRule } from '../util'; import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; import { TSEnumMember } from '../../../typescript-estree/dist/ts-estree/ts-estree'; -export enum MessageId { - NoVariable = 'noVariable', -} - -type Options = []; - -export default createRule({ - name: 'no-identifier-enum-member', +export default createRule<[], 'notLiteral'>({ + name: 'prefer-literal-enum-member', meta: { type: 'suggestion', docs: { - description: 'Disallow identifier (aka variable) enum members', + description: + 'Require that all enum members be literal values to prevent unintended enum member name shadow issues', category: 'Best Practices', recommended: false, requiresTypeChecking: false, }, messages: { - [MessageId.NoVariable]: `Enum member can not be an ${AST_NODE_TYPES.Identifier}.`, + notLiteral: `Enum member must be a ${AST_NODE_TYPES.Literal}.`, }, schema: [], }, @@ -30,11 +25,11 @@ export default createRule({ // If there is no initializer, then this node is just the name of the member, so ignore. if ( node.initializer != null && - node.initializer.type === AST_NODE_TYPES.Identifier + node.initializer.type !== AST_NODE_TYPES.Literal ) { context.report({ node: node.id, - messageId: MessageId.NoVariable, + messageId: 'notLiteral', data: { type: node.initializer.type, }, diff --git a/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts b/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts deleted file mode 100644 index a3bd85b6781..00000000000 --- a/packages/eslint-plugin/tests/rules/no-identifier-enum-member.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import rule, { MessageId } from '../../src/rules/no-identifier-enum-member'; -import { RuleTester } from '../RuleTester'; - -const ruleTester = new RuleTester({ - parser: '@typescript-eslint/parser', -}); - -ruleTester.run('prefer-literal-enum-member', rule, { - valid: [ - ` -enum Valid { - A = /test/, - B = 'test', - C = 42, - D = null, - E, - F = {}, - G = [], - H = new Set(), - I = 2 + 2, -} - `, - ], - invalid: [ - { - code: ` -const variable = 'Test'; -enum Foo { - A = 'TestStr', - B = 2, - C, - V = variable, -} - `, - errors: [ - { - messageId: MessageId.NoVariable, - line: 7, - column: 3, - }, - ], - }, - ], -}); diff --git a/packages/eslint-plugin/tests/rules/prefer-literal-enum-member.test.ts b/packages/eslint-plugin/tests/rules/prefer-literal-enum-member.test.ts new file mode 100644 index 00000000000..50b6030ec21 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-literal-enum-member.test.ts @@ -0,0 +1,207 @@ +import rule from '../../src/rules/prefer-literal-enum-member'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('prefer-literal-enum-member', rule, { + valid: [ + ` +enum ValidRegex { + A = /test/, +} + `, + ` +enum ValidString { + A = 'test', +} + `, + ` +enum ValidNumber { + A = 42, +} + `, + ` +enum ValidNull { + A = null, +} + `, + ` +enum ValidPlain { + A, +} + `, + ` +enum ValidQuotedKey { + 'a', +} + `, + ` +enum ValidQuotedKeyWithAssignment { + 'a' = 1, +} + `, + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + ` +enum ValidKeyWithComputedSyntaxButNoComputedKey { + ['a'], +} + `, + ], + invalid: [ + { + code: ` +enum InvalidObject { + A = {}, +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 3, + column: 3, + }, + ], + }, + { + code: ` +enum InvalidArray { + A = [], +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 3, + column: 3, + }, + ], + }, + { + code: ` +enum InvalidTemplateLiteral { + A = \`a\`, +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 3, + column: 3, + }, + ], + }, + { + code: ` +enum InvalidConstructor { + A = new Set(), +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 3, + column: 3, + }, + ], + }, + { + code: ` +enum InvalidExpression { + A = 2 + 2, +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 3, + column: 3, + }, + ], + }, + { + code: ` +const variable = 'Test'; +enum InvalidVariable { + A = 'TestStr', + B = 2, + C, + V = variable, +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 7, + column: 3, + }, + ], + }, + { + code: ` +enum InvalidEnumMember { + A = 'TestStr', + B = A, +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 4, + column: 3, + }, + ], + }, + { + code: ` +const Valid = { A: 2 }; +enum InvalidObjectMember { + A = 'TestStr', + B = Valid.A, +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 5, + column: 3, + }, + ], + }, + { + code: ` +enum Valid { + A, +} +enum InvalidEnumMember { + A = 'TestStr', + B = Valid.A, +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 7, + column: 3, + }, + ], + }, + { + code: ` +const obj = { a: 1 }; +enum InvalidSpread { + A = 'TestStr', + B = { ...a }, +} + `, + errors: [ + { + messageId: 'notLiteral', + line: 5, + column: 3, + }, + ], + }, + ], +}); From 2f9ff3ab8ef22b885264fffd88f785a7b628eab9 Mon Sep 17 00:00:00 2001 From: John Sanders Date: Thu, 25 Jun 2020 16:18:39 -0700 Subject: [PATCH 4/6] fix: update error messaging and remove unnecessary null check --- .../src/rules/prefer-literal-enum-member.ts | 13 +++---------- .../tests/rules/prefer-literal-enum-member.test.ts | 5 ++--- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts b/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts index 6b13269b959..3eaabecc862 100644 --- a/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts +++ b/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts @@ -1,6 +1,5 @@ import { createRule } from '../util'; import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; -import { TSEnumMember } from '../../../typescript-estree/dist/ts-estree/ts-estree'; export default createRule<[], 'notLiteral'>({ name: 'prefer-literal-enum-member', @@ -14,25 +13,19 @@ export default createRule<[], 'notLiteral'>({ requiresTypeChecking: false, }, messages: { - notLiteral: `Enum member must be a ${AST_NODE_TYPES.Literal}.`, + notLiteral: `Explicit enum value must only be a literal value (string, number, boolean, etc).`, }, schema: [], }, defaultOptions: [], create(context) { return { - TSEnumMember(node: TSEnumMember): void { + TSEnumMember(node): void { // If there is no initializer, then this node is just the name of the member, so ignore. - if ( - node.initializer != null && - node.initializer.type !== AST_NODE_TYPES.Literal - ) { + if (node.initializer?.type !== AST_NODE_TYPES.Literal) { context.report({ node: node.id, messageId: 'notLiteral', - data: { - type: node.initializer.type, - }, }); } }, diff --git a/packages/eslint-plugin/tests/rules/prefer-literal-enum-member.test.ts b/packages/eslint-plugin/tests/rules/prefer-literal-enum-member.test.ts index 50b6030ec21..4e309edc357 100644 --- a/packages/eslint-plugin/tests/rules/prefer-literal-enum-member.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-literal-enum-member.test.ts @@ -1,5 +1,5 @@ import rule from '../../src/rules/prefer-literal-enum-member'; -import { RuleTester } from '../RuleTester'; +import { RuleTester, noFormat } from '../RuleTester'; const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', @@ -42,8 +42,7 @@ enum ValidQuotedKeyWithAssignment { 'a' = 1, } `, - // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting - ` + noFormat` enum ValidKeyWithComputedSyntaxButNoComputedKey { ['a'], } From 6445b112d7f7bfb1734263de72f318eda2122604 Mon Sep 17 00:00:00 2001 From: John Sanders Date: Fri, 26 Jun 2020 09:20:35 -0700 Subject: [PATCH 5/6] fix: don't use optional chaining for Node 10 compatibility --- .../eslint-plugin/src/rules/prefer-literal-enum-member.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts b/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts index 3eaabecc862..db1be2eea7e 100644 --- a/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts +++ b/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts @@ -1,5 +1,6 @@ import { createRule } from '../util'; import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; export default createRule<[], 'notLiteral'>({ name: 'prefer-literal-enum-member', @@ -20,9 +21,9 @@ export default createRule<[], 'notLiteral'>({ defaultOptions: [], create(context) { return { - TSEnumMember(node): void { + 'TSEnumMember[initializer != null]'(node: TSESTree.TSEnumMember): void { // If there is no initializer, then this node is just the name of the member, so ignore. - if (node.initializer?.type !== AST_NODE_TYPES.Literal) { + if (node.initializer!.type !== AST_NODE_TYPES.Literal) { context.report({ node: node.id, messageId: 'notLiteral', From 2cb9f2ee8915c52797320a37dd056a435da1569b Mon Sep 17 00:00:00 2001 From: John Sanders Date: Fri, 26 Jun 2020 10:07:12 -0700 Subject: [PATCH 6/6] fix: replace selector with runtime nullish check --- .../src/rules/prefer-literal-enum-member.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts b/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts index db1be2eea7e..8f95fd5946b 100644 --- a/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts +++ b/packages/eslint-plugin/src/rules/prefer-literal-enum-member.ts @@ -1,6 +1,5 @@ -import { createRule } from '../util'; import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; -import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { createRule } from '../util'; export default createRule<[], 'notLiteral'>({ name: 'prefer-literal-enum-member', @@ -21,9 +20,12 @@ export default createRule<[], 'notLiteral'>({ defaultOptions: [], create(context) { return { - 'TSEnumMember[initializer != null]'(node: TSESTree.TSEnumMember): void { + TSEnumMember(node): void { // If there is no initializer, then this node is just the name of the member, so ignore. - if (node.initializer!.type !== AST_NODE_TYPES.Literal) { + if ( + node.initializer != null && + node.initializer.type !== AST_NODE_TYPES.Literal + ) { context.report({ node: node.id, messageId: 'notLiteral',