diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 5b32c4ef29e7..2ca157759a0c 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -152,6 +152,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements | :white_check_mark: | | | | [`@typescript-eslint/non-nullable-type-assertion-style`](./docs/rules/non-nullable-type-assertion-style.md) | Prefers a non-null assertion over explicit type cast when possible | | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-as-const`](./docs/rules/prefer-as-const.md) | Prefer usage of `as const` over literal type | :white_check_mark: | :wrench: | | +| [`@typescript-eslint/prefer-consistent-enums`](./docs/rules/prefer-consistent-enums.md) | Prefer consistent enum members | | | | | [`@typescript-eslint/prefer-enum-initializers`](./docs/rules/prefer-enum-initializers.md) | Prefer initializing each enums member value | | | | | [`@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: | | diff --git a/packages/eslint-plugin/docs/rules/prefer-consistent-enums.md b/packages/eslint-plugin/docs/rules/prefer-consistent-enums.md new file mode 100644 index 000000000000..e652738d32ea --- /dev/null +++ b/packages/eslint-plugin/docs/rules/prefer-consistent-enums.md @@ -0,0 +1,75 @@ +# Prefer consistent enum members (`prefer-consistent-enums`) + +This rule recommends having each `enum`s member type to be the same. + +## Rule Details + +You can iterate over enums using `Object.keys` / `Object.values`. + +If all enum members are strings — result is consistent and number of items will match number of enum members: + +```ts +enum Status { + Open = 'open', + Closed = 'closed', +} + +Object.values(Status); // ['open','closed'] +``` + +But if enum will have some members that are initialized with numbers, or not initialized at all — iteration over that enum will have additional auto generated items + +```ts +enum Status { + Pending = 0, + Open = 'open', + Closed = 'closed', +} + +Object.values(Status); // ["Pending", 0, "open", "closed"] +``` + +Examples of **incorrect** code for this rule: + +```ts +enum Status { + Pending = 0, + Open = 'open', + Closed = 'closed', +} + +enum Direction { + Up = 0, + Down, +} + +enum Color { + Red = 5, + Green = 'Green' + Blue = 'Blue', +} +``` + +Examples of **correct** code for this rule: + +```ts +enum Status { + Open = 'Open', + Close = 'Close', +} + +enum Direction { + Up = 1, + Down = 2, +} + +enum Color { + Red = 'Red', + Green = 'Green', + Blue = 'Blue', +} +``` + +## When Not To Use It + +If you don't iterate over `enum`s you can safely disable this rule. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 9d7fde81314f..d1cceed5fad4 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -120,6 +120,7 @@ export = { 'padding-line-between-statements': 'off', '@typescript-eslint/padding-line-between-statements': 'error', '@typescript-eslint/prefer-as-const': 'error', + '@typescript-eslint/prefer-consistent-enums': 'error', '@typescript-eslint/prefer-enum-initializers': 'error', '@typescript-eslint/prefer-for-of': 'error', '@typescript-eslint/prefer-function-type': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 1f6aa56c7cba..9b29c22e304c 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -84,6 +84,7 @@ import nonNullableTypeAssertionStyle from './non-nullable-type-assertion-style'; import objectCurlySpacing from './object-curly-spacing'; import paddingLineBetweenStatements from './padding-line-between-statements'; import preferAsConst from './prefer-as-const'; +import preferConsistentEnums from './prefer-consistent-enums'; import preferEnumInitializers from './prefer-enum-initializers'; import preferForOf from './prefer-for-of'; import preferFunctionType from './prefer-function-type'; @@ -205,6 +206,7 @@ export default { 'object-curly-spacing': objectCurlySpacing, 'padding-line-between-statements': paddingLineBetweenStatements, 'prefer-as-const': preferAsConst, + 'prefer-consistent-enums': preferConsistentEnums, 'prefer-enum-initializers': preferEnumInitializers, 'prefer-for-of': preferForOf, 'prefer-function-type': preferFunctionType, diff --git a/packages/eslint-plugin/src/rules/prefer-consistent-enums.ts b/packages/eslint-plugin/src/rules/prefer-consistent-enums.ts new file mode 100644 index 000000000000..26fbc3453a38 --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-consistent-enums.ts @@ -0,0 +1,161 @@ +import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +type MessageIds = + | 'nonConsistentEnum' + | 'nonConsistentEnumSuggestion' + | 'nonConsistentEnumSuggestionNoInitializer'; +const NO_INITIALIZER = 'noInitializer'; + +export default util.createRule<[], MessageIds>({ + name: 'prefer-consistent-enums', + meta: { + type: 'suggestion', + docs: { + description: 'Prefer consistent enum members', + category: 'Possible Errors', + recommended: false, + requiresTypeChecking: false, + }, + messages: { + nonConsistentEnum: `All enum members of {{ name }} must be same type (string, number, boolean, etc).`, + nonConsistentEnumSuggestion: `Can be fixed to {{ name }} = {{ suggested }}`, + nonConsistentEnumSuggestionNoInitializer: `Can be fixed to {{ name }}`, + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const sourceCode = context.getSourceCode(); + + function TSEnumDeclaration(node: TSESTree.TSEnumDeclaration): void { + const enumName = sourceCode.getText(node.id); + const { members } = node; + + let enumType: string; + let lastNumericValue: number; + + members.forEach((member, index) => { + let memberType: string | undefined; + let memberValue: TSESTree.Literal['value'] | undefined; + + /** + * Getting enum member initializer type + * If it's number — get its value to suggest new one + * If it's binary expression — treat it like number but without getting its value + */ + if (member.initializer) { + switch (member.initializer.type) { + case TSESTree.AST_NODE_TYPES.Literal: + memberValue = member.initializer.value; + memberType = typeof member.initializer.value; + if (memberType === 'number') { + lastNumericValue = member.initializer.value as number; + } + break; + case TSESTree.AST_NODE_TYPES.BinaryExpression: + memberType = 'number'; + break; + } + } else { + memberType = NO_INITIALIZER; + } + + if (!memberType) { + return; + } + + /** + * If it's first enum member — remember its type and continue to next one + */ + if (!enumType) { + enumType = memberType; + return; + } + + /** + * If initializers types dont match — suggest change + */ + if (enumType !== memberType) { + const name = sourceCode.getText(member.id); + + /** + * If base enum type is string — transforming initializer to string + * or create new one base on enum member name + */ + if (enumType === 'string') { + context.report({ + node: member, + messageId: 'nonConsistentEnum', + data: { name: enumName }, + suggest: [ + { + messageId: 'nonConsistentEnumSuggestion', + data: { name, suggested: `'${memberValue ?? name}'` }, + fix: (fixer): TSESLint.RuleFix => + fixer.replaceText( + member, + `${name} = '${memberValue ?? name}'`, + ), + }, + ], + }); + return; + } + + /** + * If base enum type is number — suggest replacing initializer + * with last numeric identifier + 1 or just enum member index + */ + if (enumType === 'number') { + const newIndex = + typeof lastNumericValue !== 'undefined' + ? lastNumericValue + 1 + : index + 1; + context.report({ + node: member, + messageId: 'nonConsistentEnum', + data: { name: enumName }, + suggest: [ + { + messageId: 'nonConsistentEnumSuggestion', + data: { name, suggested: newIndex }, + fix: (fixer): TSESLint.RuleFix => + fixer.replaceText(member, `${name} = ${newIndex}`), + }, + ], + }); + return; + } + + /** + * If enum have no initializers — suggest removing one + */ + if (enumType === NO_INITIALIZER) { + context.report({ + node: member, + messageId: 'nonConsistentEnum', + data: { name: enumName }, + suggest: [ + { + messageId: 'nonConsistentEnumSuggestionNoInitializer', + data: { name }, + fix: (fixer): TSESLint.RuleFix => + fixer.replaceText(member, `${name}`), + }, + ], + }); + } + + /** + * No suggestions for other enum types + */ + } + }); + } + + return { + TSEnumDeclaration, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/prefer-consistent-enums.test.ts b/packages/eslint-plugin/tests/rules/prefer-consistent-enums.test.ts new file mode 100644 index 000000000000..138f722c1bc2 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-consistent-enums.test.ts @@ -0,0 +1,210 @@ +import rule from '../../src/rules/prefer-consistent-enums'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('prefer-consistent-enums', rule, { + valid: [ + ` +enum Direction {} + `, + ` +enum Direction { + Up = 1, +} + `, + ` +enum Direction { + Up = 1, + Down = 2, +} + `, + ` +enum Direction { + Up = 'Up', + Down = 'Down', +} + `, + ], + // We need to keep indentation for avoiding @typescript-eslint/internal/plugin-test-formatting. + // Use trimRight() to make tests pass for now. https://github.com/typescript-eslint/typescript-eslint/pull/2326#discussion_r461760044 + invalid: [ + { + code: ` +enum Direction { + Up = 'Up', + Down, +} + `.trimRight(), + errors: [ + { + messageId: 'nonConsistentEnum', + data: { name: 'Direction' }, + line: 4, + suggestions: [ + { + messageId: 'nonConsistentEnumSuggestion', + output: ` +enum Direction { + Up = 'Up', + Down = 'Down', +} + `.trimRight(), + }, + ], + }, + ], + }, + { + code: ` +enum Direction { + Up, + Down = 5, +} + `.trimRight(), + errors: [ + { + messageId: 'nonConsistentEnum', + data: { name: 'Direction' }, + line: 4, + suggestions: [ + { + messageId: 'nonConsistentEnumSuggestionNoInitializer', + output: ` +enum Direction { + Up, + Down, +} + `.trimRight(), + }, + ], + }, + ], + }, + { + code: ` +enum Direction { + Up = 0, + Down = 'Down', +} + `.trimRight(), + errors: [ + { + messageId: 'nonConsistentEnum', + data: { name: 'Direction' }, + line: 4, + suggestions: [ + { + messageId: 'nonConsistentEnumSuggestion', + output: ` +enum Direction { + Up = 0, + Down = 1, +} + `.trimRight(), + }, + ], + }, + ], + }, + { + code: ` +enum Direction { + Up = 123, + Down = 'Down', +} + `.trimRight(), + errors: [ + { + messageId: 'nonConsistentEnum', + data: { name: 'Direction' }, + line: 4, + suggestions: [ + { + messageId: 'nonConsistentEnumSuggestion', + output: ` +enum Direction { + Up = 123, + Down = 124, +} + `.trimRight(), + }, + ], + }, + ], + }, + { + code: ` +enum Direction { + Up = 'Up', + Down = 2, + Left = 3, +} + `.trimRight(), + errors: [ + { + messageId: 'nonConsistentEnum', + data: { name: 'Direction' }, + line: 4, + suggestions: [ + { + messageId: 'nonConsistentEnumSuggestion', + output: ` +enum Direction { + Up = 'Up', + Down = '2', + Left = 3, +} + `.trimRight(), + }, + ], + }, + { + messageId: 'nonConsistentEnum', + data: { name: 'Direction' }, + line: 5, + suggestions: [ + { + messageId: 'nonConsistentEnumSuggestion', + output: ` +enum Direction { + Up = 'Up', + Down = 2, + Left = '3', +} + `.trimRight(), + }, + ], + }, + ], + }, + { + code: ` +enum Test { + A, + B = 1 + 2, +} + `.trimRight(), + errors: [ + { + messageId: 'nonConsistentEnum', + data: { name: 'Test' }, + line: 4, + suggestions: [ + { + messageId: 'nonConsistentEnumSuggestionNoInitializer', + output: ` +enum Test { + A, + B, +} + `.trimRight(), + }, + ], + }, + ], + }, + ], +});