Skip to content

Commit

Permalink
feat(eslint-plugin): [no-duplicate-enum-values] add rule (#4833)
Browse files Browse the repository at this point in the history
* feat(eslint-plugin): create a new rule to disallow duplicate enum values

* fix(eslint-plugin): remove unused imported variable from no-duplicate-enum-values.test.ts

* fix(eslint-plugin): test falsy values and fix some metadata

* fix(eslint-plugin): make Enums in the falsy test valid

Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>
  • Loading branch information
aifreedom and JoshuaKGoldberg committed Apr 27, 2022
1 parent acb5310 commit 5899164
Show file tree
Hide file tree
Showing 6 changed files with 267 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -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: | |
Expand Down
51 changes: 51 additions & 0 deletions 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.

<!--tabs-->

### ❌ 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
9 changes: 5 additions & 4 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
72 changes: 72 additions & 0 deletions packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts
@@ -0,0 +1,72 @@
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,
},
hasSuggestions: true,
messages: {
duplicateValue: 'Duplicate enum member 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<number | string>();

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: 'duplicateValue',
data: {
value,
},
});
} else {
seenValues.add(value);
}
});
},
};
},
});
136 changes: 136 additions & 0 deletions packages/eslint-plugin/tests/rules/no-duplicate-enum-values.test.ts
@@ -0,0 +1,136 @@
import rule from '../../src/rules/no-duplicate-enum-values';
import { RuleTester } 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(),
}
`,
`
enum E {
A = '',
B = 0,
}
`,
`
enum E {
A = 0,
B = -0,
C = NaN,
}
`,
],
invalid: [
{
code: `
enum E {
A = 1,
B = 1,
}
`,
errors: [
{
line: 4,
column: 3,
messageId: 'duplicateValue',
data: { value: 1 },
},
],
},
{
code: `
enum E {
A = 'A',
B = 'A',
}
`,
errors: [
{
line: 4,
column: 3,
messageId: 'duplicateValue',
data: { value: 'A' },
},
],
},
{
code: `
enum E {
A = 'A',
B = 'A',
C = 1,
D = 1,
}
`,
errors: [
{
line: 4,
column: 3,
messageId: 'duplicateValue',
data: { value: 'A' },
},
{
line: 6,
column: 3,
messageId: 'duplicateValue',
data: { value: 1 },
},
],
},
],
});

0 comments on commit 5899164

Please sign in to comment.