From b3b4343cb0b398ef218f96bfb6a9ab2f074c00f7 Mon Sep 17 00:00:00 2001 From: Song Xie Date: Mon, 4 Apr 2022 00:17:05 -0700 Subject: [PATCH 1/4] feat(eslint-plugin): create a new rule to disallow duplicate enum values --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-duplicate-enum-values.md | 51 ++++++++ packages/eslint-plugin/src/configs/all.ts | 9 +- packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-duplicate-enum-values.ts | 73 +++++++++++ .../rules/no-duplicate-enum-values.test.ts | 123 ++++++++++++++++++ 6 files changed, 255 insertions(+), 4 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-duplicate-enum-values.md create mode 100644 packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts create mode 100644 packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index f9126c5f133..7b1d5d03cc2 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -119,6 +119,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-base-to-string`](./docs/rules/no-base-to-string.md) | Requires that `.toString()` is only called on objects which provide useful information when stringified | | | :thought_balloon: | | [`@typescript-eslint/no-confusing-non-null-assertion`](./docs/rules/no-confusing-non-null-assertion.md) | Disallow non-null assertion in locations that may be confusing | | :wrench: | | | [`@typescript-eslint/no-confusing-void-expression`](./docs/rules/no-confusing-void-expression.md) | Requires expressions of type void to appear in statement position | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-duplicate-enum-values`](./docs/rules/no-duplicate-enum-values.md) | Disallow duplicate enum member values | | | | | [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Disallow the delete operator with computed key expressions | | :wrench: | | | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :white_check_mark: | :wrench: | | | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :white_check_mark: | :wrench: | | diff --git a/packages/eslint-plugin/docs/rules/no-duplicate-enum-values.md b/packages/eslint-plugin/docs/rules/no-duplicate-enum-values.md new file mode 100644 index 00000000000..2146d603980 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-duplicate-enum-values.md @@ -0,0 +1,51 @@ +# `no-duplicate-enum-values` + +Disallow duplicate enum member values. + +Although TypeScript supports duplicate enum member values, people usually expect members to have unique values within the same enum. Duplicate values can lead to bugs that are hard to track down. + +## Rule Details + +This rule disallows defining an enum with multiple members initialized to the same value. Now it only enforces on enum members initialized with String or Number literals. Members without initializer or initialized with an expression are not checked by this rule. + + + +### ❌ Incorrect + +```ts +enum E { + A = 0, + B = 0, +} +``` + +```ts +enum E { + A = 'A' + B = 'A' +} +``` + +### ✅ Correct + +```ts +enum E { + A = 0, + B = 1, +} +``` + +```ts +enum E { + A = 'A' + B = 'B' +} +``` + +This rule is not configurable. + +## Attributes + +- [ ] ✅ Recommended +- [ ] 🔧 Fixable +- [ ] 💭 Requires type information diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index c55e0e6ac37..b3adbd1a3ac 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -21,8 +21,8 @@ export = { '@typescript-eslint/consistent-indexed-object-style': 'error', '@typescript-eslint/consistent-type-assertions': 'error', '@typescript-eslint/consistent-type-definitions': 'error', - '@typescript-eslint/consistent-type-imports': 'error', '@typescript-eslint/consistent-type-exports': 'error', + '@typescript-eslint/consistent-type-imports': 'error', 'default-param-last': 'off', '@typescript-eslint/default-param-last': 'error', 'dot-notation': 'off', @@ -51,6 +51,7 @@ export = { '@typescript-eslint/no-confusing-void-expression': 'error', 'no-dupe-class-members': 'off', '@typescript-eslint/no-dupe-class-members': 'error', + '@typescript-eslint/no-duplicate-enum-values': 'error', 'no-duplicate-imports': 'off', '@typescript-eslint/no-duplicate-imports': 'error', '@typescript-eslint/no-dynamic-delete': 'error', @@ -115,9 +116,9 @@ export = { '@typescript-eslint/no-unused-vars': 'error', 'no-use-before-define': 'off', '@typescript-eslint/no-use-before-define': 'error', - '@typescript-eslint/no-useless-empty-export': 'error', 'no-useless-constructor': 'off', '@typescript-eslint/no-useless-constructor': 'error', + '@typescript-eslint/no-useless-empty-export': 'error', '@typescript-eslint/no-var-requires': 'error', '@typescript-eslint/non-nullable-type-assertion-style': 'error', 'object-curly-spacing': 'off', @@ -153,12 +154,12 @@ export = { semi: 'off', '@typescript-eslint/semi': 'error', '@typescript-eslint/sort-type-union-intersection-members': 'error', + 'space-before-blocks': 'off', + '@typescript-eslint/space-before-blocks': 'error', 'space-before-function-paren': 'off', '@typescript-eslint/space-before-function-paren': 'error', 'space-infix-ops': 'off', '@typescript-eslint/space-infix-ops': 'error', - 'space-before-blocks': 'off', - '@typescript-eslint/space-before-blocks': 'error', '@typescript-eslint/strict-boolean-expressions': 'error', '@typescript-eslint/switch-exhaustiveness-check': 'error', '@typescript-eslint/triple-slash-reference': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index fb48ee5cadc..41afc88199f 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -32,6 +32,7 @@ import noBaseToString from './no-base-to-string'; import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion'; import noConfusingVoidExpression from './no-confusing-void-expression'; import noDupeClassMembers from './no-dupe-class-members'; +import noDuplicateEnumValues from './no-duplicate-enum-values'; import noDuplicateImports from './no-duplicate-imports'; import noDynamicDelete from './no-dynamic-delete'; import noEmptyFunction from './no-empty-function'; @@ -159,6 +160,7 @@ export default { 'no-confusing-non-null-assertion': confusingNonNullAssertionLikeNotEqual, 'no-confusing-void-expression': noConfusingVoidExpression, 'no-dupe-class-members': noDupeClassMembers, + 'no-duplicate-enum-values': noDuplicateEnumValues, 'no-duplicate-imports': noDuplicateImports, 'no-dynamic-delete': noDynamicDelete, 'no-empty-function': noEmptyFunction, diff --git a/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts b/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts new file mode 100644 index 00000000000..f5f045f7de4 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts @@ -0,0 +1,73 @@ +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; +import * as util from '../util'; + +export default util.createRule({ + name: 'no-duplicate-enum-values', + meta: { + type: 'problem', + docs: { + description: 'Disallow duplicate enum member values', + recommended: false, + suggestion: true, + }, + hasSuggestions: true, + messages: { + duplicateMember: 'Duplicate enum member with value {{value}}.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + function isStringLiteral( + node: TSESTree.Expression, + ): node is TSESTree.StringLiteral { + return ( + node.type === AST_NODE_TYPES.Literal && typeof node.value === 'string' + ); + } + + function isNumberLiteral( + node: TSESTree.Expression, + ): node is TSESTree.NumberLiteral { + return ( + node.type === AST_NODE_TYPES.Literal && typeof node.value === 'number' + ); + } + + return { + TSEnumDeclaration(node: TSESTree.TSEnumDeclaration): void { + const enumMembers = node.members; + const seenValues = new Set(); + + enumMembers.forEach(member => { + if (member.initializer === undefined) { + return; + } + + let value: string | number | undefined; + if (isStringLiteral(member.initializer)) { + value = String(member.initializer.value); + } else if (isNumberLiteral(member.initializer)) { + value = Number(member.initializer.value); + } + + if (value === undefined) { + return; + } + + if (seenValues.has(value)) { + context.report({ + node: member, + messageId: 'duplicateMember', + data: { + value, + }, + }); + } else { + seenValues.add(value); + } + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts b/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts new file mode 100644 index 00000000000..4dd578b6244 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts @@ -0,0 +1,123 @@ +import rule from '../../src/rules/no-duplicate-enum-values'; +import { RuleTester, noFormat } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-duplicate-enum-values', rule, { + valid: [ + ` +enum E { + A, + B, +} + `, + ` +enum E { + A = 1, + B, +} + `, + ` +enum E { + A = 1, + B = 2, +} + `, + ` +enum E { + A = 'A', + B = 'B', +} + `, + ` +enum E { + A = 'A', + B = 'B', + C, +} + `, + ` +enum E { + A = 'A', + B = 'B', + C = 2, + D = 1 + 1, +} + `, + ` +enum E { + A = 3, + B = 2, + C, +} + `, + ` +enum E { + A = 'A', + B = 'B', + C = 2, + D = foo(), +} + `, + ], + invalid: [ + { + code: ` +enum E { + A = 1, + B = 1, +} + `, + errors: [ + { + line: 4, + column: 3, + messageId: 'duplicateMember', + data: { value: 1 }, + }, + ], + }, + { + code: ` +enum E { + A = 'A', + B = 'A', +} + `, + errors: [ + { + line: 4, + column: 3, + messageId: 'duplicateMember', + data: { value: 'A' }, + }, + ], + }, + { + code: ` +enum E { + A = 'A', + B = 'A', + C = 1, + D = 1, +} + `, + errors: [ + { + line: 4, + column: 3, + messageId: 'duplicateMember', + data: { value: 'A' }, + }, + { + line: 6, + column: 3, + messageId: 'duplicateMember', + data: { value: 1 }, + }, + ], + }, + ], +}); From 2b80aa84a3de98f64b3f5614e259e69ebecefee8 Mon Sep 17 00:00:00 2001 From: Song Xie Date: Sun, 17 Apr 2022 16:45:32 -0700 Subject: [PATCH 2/4] fix(eslint-plugin): remove unused imported variable from no-duplicate-enum-values.test.ts --- .../eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts b/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts index 4dd578b6244..dd59f867e2f 100644 --- a/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts +++ b/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts @@ -1,5 +1,5 @@ import rule from '../../src/rules/no-duplicate-enum-values'; -import { RuleTester, noFormat } from '../RuleTester'; +import { RuleTester } from '../RuleTester'; const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', From 74b9c2c009c066b4eb1779992e5843301f359f5e Mon Sep 17 00:00:00 2001 From: Song Xie Date: Fri, 22 Apr 2022 23:16:05 -0700 Subject: [PATCH 3/4] fix(eslint-plugin): test falsy values and fix some metadata --- .../src/rules/no-duplicate-enum-values.ts | 5 ++--- .../rules/no-duplicate-enum-values.test.ts | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts b/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts index f5f045f7de4..3ede60b9fc3 100644 --- a/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts +++ b/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts @@ -8,11 +8,10 @@ export default util.createRule({ docs: { description: 'Disallow duplicate enum member values', recommended: false, - suggestion: true, }, hasSuggestions: true, messages: { - duplicateMember: 'Duplicate enum member with value {{value}}.', + duplicateValue: 'Duplicate enum member value {{value}}.', }, schema: [], }, @@ -58,7 +57,7 @@ export default util.createRule({ if (seenValues.has(value)) { context.report({ node: member, - messageId: 'duplicateMember', + messageId: 'duplicateValue', data: { value, }, diff --git a/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts b/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts index dd59f867e2f..a05a604dc80 100644 --- a/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts +++ b/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts @@ -59,6 +59,17 @@ enum E { B = 'B', C = 2, D = foo(), +} + `, + ` +enum E { + A = false, + B = 0, + C = -0, + D = 0n, + E = null, + F = undefined, + G = NaN, } `, ], @@ -74,7 +85,7 @@ enum E { { line: 4, column: 3, - messageId: 'duplicateMember', + messageId: 'duplicateValue', data: { value: 1 }, }, ], @@ -90,7 +101,7 @@ enum E { { line: 4, column: 3, - messageId: 'duplicateMember', + messageId: 'duplicateValue', data: { value: 'A' }, }, ], @@ -108,13 +119,13 @@ enum E { { line: 4, column: 3, - messageId: 'duplicateMember', + messageId: 'duplicateValue', data: { value: 'A' }, }, { line: 6, column: 3, - messageId: 'duplicateMember', + messageId: 'duplicateValue', data: { value: 1 }, }, ], From 54841614045ca58e72d3183daf0a6ccc41f4aa60 Mon Sep 17 00:00:00 2001 From: Song Xie Date: Fri, 22 Apr 2022 23:24:46 -0700 Subject: [PATCH 4/4] fix(eslint-plugin): make Enums in the falsy test valid --- .../tests/rules/no-duplicate-enum-values.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts b/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts index a05a604dc80..037579d12bb 100644 --- a/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts +++ b/packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts @@ -63,13 +63,15 @@ enum E { `, ` enum E { - A = false, + A = '', B = 0, - C = -0, - D = 0n, - E = null, - F = undefined, - G = NaN, +} + `, + ` +enum E { + A = 0, + B = -0, + C = NaN, } `, ],