diff --git a/packages/eslint-plugin/docs/rules/no-mixed-enums.md b/packages/eslint-plugin/docs/rules/no-mixed-enums.md new file mode 100644 index 00000000000..96ccbddf4e0 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-mixed-enums.md @@ -0,0 +1,88 @@ +--- +description: 'Disallow enums from having both number and string members.' +--- + +> πŸ›‘ This file is source code, not the primary documentation location! πŸ›‘ +> +> See **https://typescript-eslint.io/rules/no-mixed-enums** for documentation. + +TypeScript enums are allowed to assign numeric or string values to their members. +Most enums contain either all numbers or all strings, but in theory you can mix-and-match within the same enum. +Mixing enum member types is generally considered confusing and a bad practice. + +## Examples + + + +### ❌ Incorrect + +```ts +enum Status { + Unknown, + Closed = 1, + Open = 'open', +} +``` + +### βœ… Correct (Explicit Numbers) + +```ts +enum Status { + Unknown = 0, + Closed = 1, + Open = 2, +} +``` + +### βœ… Correct (Implicit Numbers) + +```ts +enum Status { + Unknown, + Closed, + Open, +} +``` + +### βœ… Correct (Strings) + +```ts +enum Status { + Unknown = 'unknown', + Closed = 'closed', + Open = 'open', +} +``` + +## Iteration Pitfalls of Mixed Enum Member Values + +Enum values may be iterated over using `Object.entries`/`Object.keys`/`Object.values`. + +If all enum members are strings, the number of items will match the number of enum members: + +```ts +enum Status { + Closed = 'closed', + Open = 'open', +} + +// ['closed', 'open'] +Object.values(Status); +``` + +But if the enum contains members that are initialized with numbers -including implicitly initialized numbersβ€” then iteration over that enum will include those numbers as well: + +```ts +enum Status { + Unknown, + Closed = 1, + Open = 'open', +} + +// ["Unknown", "Closed", 0, 1, "open"] +Object.values(Status); +``` + +## When Not To Use It + +If you don't mind the confusion of mixed enum member values and don't iterate over enums, 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 eb3856f10c3..cea7994eb13 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -84,6 +84,7 @@ export = { '@typescript-eslint/no-meaningless-void-operator': 'error', '@typescript-eslint/no-misused-new': 'error', '@typescript-eslint/no-misused-promises': 'error', + '@typescript-eslint/no-mixed-enums': 'error', '@typescript-eslint/no-namespace': 'error', '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error', '@typescript-eslint/no-non-null-asserted-optional-chain': 'error', diff --git a/packages/eslint-plugin/src/configs/strict.ts b/packages/eslint-plugin/src/configs/strict.ts index 99b4e83b508..c63b4173452 100644 --- a/packages/eslint-plugin/src/configs/strict.ts +++ b/packages/eslint-plugin/src/configs/strict.ts @@ -21,6 +21,7 @@ export = { '@typescript-eslint/no-extraneous-class': 'warn', '@typescript-eslint/no-invalid-void-type': 'warn', '@typescript-eslint/no-meaningless-void-operator': 'warn', + '@typescript-eslint/no-mixed-enums': 'warn', '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'warn', 'no-throw-literal': 'off', '@typescript-eslint/no-throw-literal': 'warn', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index bbddfc8d470..ea28c1c1f53 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -58,6 +58,7 @@ import noMagicNumbers from './no-magic-numbers'; import noMeaninglessVoidOperator from './no-meaningless-void-operator'; import noMisusedNew from './no-misused-new'; import noMisusedPromises from './no-misused-promises'; +import noMixedEnums from './no-mixed-enums'; import noNamespace from './no-namespace'; import noNonNullAssertedNullishCoalescing from './no-non-null-asserted-nullish-coalescing'; import noNonNullAssertedOptionalChain from './no-non-null-asserted-optional-chain'; @@ -191,6 +192,7 @@ export default { 'no-meaningless-void-operator': noMeaninglessVoidOperator, 'no-misused-new': noMisusedNew, 'no-misused-promises': noMisusedPromises, + 'no-mixed-enums': noMixedEnums, 'no-namespace': noNamespace, 'no-non-null-asserted-nullish-coalescing': noNonNullAssertedNullishCoalescing, 'no-non-null-asserted-optional-chain': noNonNullAssertedOptionalChain, diff --git a/packages/eslint-plugin/src/rules/no-mixed-enums.ts b/packages/eslint-plugin/src/rules/no-mixed-enums.ts new file mode 100644 index 00000000000..047471d4b26 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-mixed-enums.ts @@ -0,0 +1,224 @@ +import type { Scope } from '@typescript-eslint/scope-manager'; +import { DefinitionType } from '@typescript-eslint/scope-manager'; +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; + +import * as util from '../util'; + +enum AllowedType { + Number, + String, + Unknown, +} + +export default util.createRule({ + name: 'no-mixed-enums', + meta: { + docs: { + description: 'Disallow enums from having both number and string members', + recommended: 'strict', + requiresTypeChecking: true, + }, + messages: { + mixed: `Mixing number and string enums can be confusing.`, + }, + schema: [], + type: 'problem', + }, + defaultOptions: [], + create(context) { + const parserServices = util.getParserServices(context); + const typeChecker = parserServices.program.getTypeChecker(); + + interface CollectedDefinitions { + imports: TSESTree.Node[]; + previousSibling: TSESTree.TSEnumDeclaration | undefined; + } + + function collectNodeDefinitions( + node: TSESTree.TSEnumDeclaration, + ): CollectedDefinitions { + const { name } = node.id; + const found: CollectedDefinitions = { + imports: [], + previousSibling: undefined, + }; + let scope: Scope | null = context.getScope(); + + for (const definition of scope.upper?.set.get(name)?.defs ?? []) { + if ( + definition.node.type === AST_NODE_TYPES.TSEnumDeclaration && + definition.node.range[0] < node.range[0] && + definition.node.members.length > 0 + ) { + found.previousSibling = definition.node; + break; + } + } + + while (scope) { + scope.set.get(name)?.defs?.forEach(definition => { + if (definition.type === DefinitionType.ImportBinding) { + found.imports.push(definition.node); + } + }); + + scope = scope.upper; + } + + return found; + } + + function getAllowedTypeForNode(node: ts.Node): AllowedType { + return tsutils.isTypeFlagSet( + typeChecker.getTypeAtLocation(node), + ts.TypeFlags.StringLike, + ) + ? AllowedType.String + : AllowedType.Number; + } + + function getTypeFromImported( + imported: TSESTree.Node, + ): AllowedType | undefined { + const type = typeChecker.getTypeAtLocation( + parserServices.esTreeNodeToTSNodeMap.get(imported), + ); + + const valueDeclaration = type.getSymbol()?.valueDeclaration; + if ( + !valueDeclaration || + !ts.isEnumDeclaration(valueDeclaration) || + valueDeclaration.members.length === 0 + ) { + return undefined; + } + + return getAllowedTypeForNode(valueDeclaration.members[0]); + } + + function getMemberType(member: TSESTree.TSEnumMember): AllowedType { + if (!member.initializer) { + return AllowedType.Number; + } + + switch (member.initializer.type) { + case AST_NODE_TYPES.Literal: + switch (typeof member.initializer.value) { + case 'number': + return AllowedType.Number; + case 'string': + return AllowedType.String; + default: + return AllowedType.Unknown; + } + + case AST_NODE_TYPES.TemplateLiteral: + return AllowedType.String; + + default: + return getAllowedTypeForNode( + parserServices.esTreeNodeToTSNodeMap.get(member.initializer), + ); + } + } + + function getDesiredTypeForDefinition( + node: TSESTree.TSEnumDeclaration, + ): AllowedType | ts.TypeFlags.Unknown | undefined { + const { imports, previousSibling } = collectNodeDefinitions(node); + + // Case: Merged ambiently via module augmentation + // import { MyEnum } from 'other-module'; + // declare module 'other-module' { + // enum MyEnum { A } + // } + for (const imported of imports) { + const typeFromImported = getTypeFromImported(imported); + if (typeFromImported !== undefined) { + return typeFromImported; + } + } + + // Case: Multiple enum declarations in the same file + // enum MyEnum { A } + // enum MyEnum { B } + if (previousSibling) { + return getMemberType(previousSibling.members[0]); + } + + // Case: Namespace declaration merging + // namespace MyNamespace { + // export enum MyEnum { A } + // } + // namespace MyNamespace { + // export enum MyEnum { B } + // } + if ( + node.parent!.type === AST_NODE_TYPES.ExportNamedDeclaration && + node.parent!.parent!.type === AST_NODE_TYPES.TSModuleBlock + ) { + // TODO: We don't need to dip into the TypeScript type checker here! + // Merged namespaces must all exist in the same file. + // We could instead compare this file's nodes to find the merges. + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node.id); + const declarations = typeChecker + .getSymbolAtLocation(tsNode)! + .getDeclarations()!; + + for (const declaration of declarations) { + for (const member of (declaration as ts.EnumDeclaration).members) { + return member.initializer + ? tsutils.isTypeFlagSet( + typeChecker.getTypeAtLocation(member.initializer), + ts.TypeFlags.StringLike, + ) + ? AllowedType.String + : AllowedType.Number + : AllowedType.Number; + } + } + } + + // Finally, we default to the type of the first enum member + return getMemberType(node.members[0]); + } + + return { + TSEnumDeclaration(node): void { + if (!node.members.length) { + return; + } + + let desiredType = getDesiredTypeForDefinition(node); + if (desiredType === ts.TypeFlags.Unknown) { + return; + } + + for (const member of node.members) { + const currentType = getMemberType(member); + if (currentType === AllowedType.Unknown) { + return; + } + + if (currentType === AllowedType.Number) { + desiredType ??= currentType; + } + + if ( + currentType !== desiredType && + (currentType !== undefined || desiredType === AllowedType.String) + ) { + context.report({ + messageId: 'mixed', + node: member.initializer ?? member, + }); + return; + } + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-mixed-enums.test.ts b/packages/eslint-plugin/tests/rules/no-mixed-enums.test.ts new file mode 100644 index 00000000000..3847fda0131 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-mixed-enums.test.ts @@ -0,0 +1,661 @@ +import rule from '../../src/rules/no-mixed-enums'; +import { getFixturesRootDir, RuleTester } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); +const ruleTester = new RuleTester({ + parserOptions: { + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-mixed-enums', rule, { + valid: [ + ` + enum Fruit {} + `, + ` + enum Fruit { + Apple, + } + `, + ` + enum Fruit { + Apple = false, + } + `, + ` + enum Fruit { + Apple, + Banana, + } + `, + ` + enum Fruit { + Apple = 0, + Banana, + } + `, + ` + enum Fruit { + Apple, + Banana = 1, + } + `, + ` + enum Fruit { + Apple = 0, + Banana = 1, + } + `, + ` + enum Fruit { + Apple, + Banana = false, + } + `, + ` + const getValue = () => 0; + enum Fruit { + Apple, + Banana = getValue(), + } + `, + ` + const getValue = () => 0; + enum Fruit { + Apple = getValue(), + Banana = getValue(), + } + `, + ` + const getValue = () => ''; + enum Fruit { + Apple = '', + Banana = getValue(), + } + `, + ` + const getValue = () => ''; + enum Fruit { + Apple = getValue(), + Banana = '', + } + `, + ` + const getValue = () => ''; + enum Fruit { + Apple = getValue(), + Banana = getValue(), + } + `, + ` + enum First { + A = 1, + } + + enum Second { + A = First.A, + B = 2, + } + `, + ` + enum First { + A = '', + } + + enum Second { + A = First.A, + B = 'b', + } + `, + ` + enum Foo { + A, + } + enum Foo { + B, + } + `, + ` + enum Foo { + A = 0, + } + enum Foo { + B, + } + `, + ` + enum Foo { + A, + } + enum Foo { + B = 1, + } + `, + ` + enum Foo { + A = 0, + } + enum Foo { + B = 1, + } + `, + ` + enum Foo { + A = 'a', + } + enum Foo { + B = 'b', + } + `, + ` + declare const Foo: any; + enum Foo { + A, + } + `, + ` +enum Foo { + A = 1, +} +enum Foo { + B = 2, +} + `, + ` +enum Foo { + A = \`A\`, +} +enum Foo { + B = \`B\`, +} + `, + ` +enum Foo { + A = false, // (TS error) +} +enum Foo { + B = \`B\`, +} + `, + ` +enum Foo { + A = 'A', +} +enum Foo { + B = false, // (TS error) +} + `, + ` +import { AST_NODE_TYPES } from '@typescript-eslint/types'; + +declare module '@typescript-eslint/types' { + enum AST_NODE_TYPES { + StringLike = 'StringLike', + } +} + `, + ` +import { TSESTree } from '@typescript-eslint/types'; + +declare module '@typescript-eslint/types' { + enum TSESTree { + StringLike = 'StringLike', + } +} + `, + ` +namespace Test { + export enum Bar { + A = 1, + } +} +namespace Test { + export enum Bar { + B = 2, + } +} + `, + ` +namespace Outer { + namespace Test { + export enum Bar { + A = 1, + } + } +} +namespace Outer { + namespace Test { + export enum Bar { + B = 'B', + } + } +} + `, + ` +namespace Outer { + namespace Test { + export enum Bar { + A = 1, + } + } +} +namespace Different { + namespace Test { + export enum Bar { + B = 'B', + } + } +} + `, + ], + invalid: [ + { + code: ` + enum Fruit { + Apple, + Banana = 'banana', + } + `, + errors: [ + { + endColumn: 28, + column: 20, + line: 4, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Fruit { + Apple, + Banana = 'banana', + Cherry = 'cherry', + } + `, + errors: [ + { + endColumn: 28, + column: 20, + line: 4, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Fruit { + Apple, + Banana, + Cherry = 'cherry', + } + `, + errors: [ + { + endColumn: 28, + column: 20, + line: 5, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Fruit { + Apple = 0, + Banana = 'banana', + } + `, + errors: [ + { + endColumn: 28, + column: 20, + line: 4, + messageId: 'mixed', + }, + ], + }, + { + code: ` + const getValue = () => 0; + enum Fruit { + Apple = getValue(), + Banana = 'banana', + } + `, + errors: [ + { + endColumn: 28, + column: 20, + line: 5, + messageId: 'mixed', + }, + ], + }, + { + code: ` + const getValue = () => ''; + enum Fruit { + Apple, + Banana = getValue(), + } + `, + errors: [ + { + endColumn: 30, + column: 20, + line: 5, + messageId: 'mixed', + }, + ], + }, + { + code: ` + const getValue = () => ''; + enum Fruit { + Apple = getValue(), + Banana = 0, + } + `, + errors: [ + { + endColumn: 21, + column: 20, + line: 5, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum First { + A = 1, + } + + enum Second { + A = First.A, + B = 'b', + } + `, + errors: [ + { + endColumn: 18, + column: 15, + line: 8, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum First { + A = 'a', + } + + enum Second { + A = First.A, + B = 1, + } + `, + errors: [ + { + endColumn: 16, + column: 15, + line: 8, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Foo { + A, + } + enum Foo { + B = 'b', + } + `, + errors: [ + { + endColumn: 18, + column: 15, + line: 6, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Foo { + A = 1, + } + enum Foo { + B = 'b', + } + `, + errors: [ + { + endColumn: 18, + column: 15, + line: 6, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Foo { + A = 'a', + } + enum Foo { + B, + } + `, + errors: [ + { + endColumn: 12, + column: 11, + line: 6, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Foo { + A = 'a', + } + enum Foo { + B = 0, + } + `, + errors: [ + { + endColumn: 16, + column: 15, + line: 6, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Foo { + A, + } + enum Foo { + B = 'b', + } + enum Foo { + C = 'c', + } + `, + errors: [ + { + endColumn: 18, + column: 15, + line: 6, + messageId: 'mixed', + }, + { + endColumn: 18, + column: 15, + line: 9, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Foo { + A, + } + enum Foo { + B = 'b', + } + enum Foo { + C, + } + `, + errors: [ + { + endColumn: 18, + column: 15, + line: 6, + messageId: 'mixed', + }, + ], + }, + { + code: ` + enum Foo { + A, + } + enum Foo { + B, + } + enum Foo { + C = 'c', + } + `, + errors: [ + { + endColumn: 18, + column: 15, + line: 9, + messageId: 'mixed', + }, + ], + }, + { + code: ` +import { AST_NODE_TYPES } from '@typescript-eslint/types'; + +declare module '@typescript-eslint/types' { + enum AST_NODE_TYPES { + Numeric = 0, + } +} + `, + errors: [ + { + endColumn: 16, + column: 15, + line: 6, + messageId: 'mixed', + }, + ], + }, + { + code: ` +enum Foo { + A = 1, +} +enum Foo { + B = 'B', +} + `, + errors: [ + { + endColumn: 10, + column: 7, + line: 6, + messageId: 'mixed', + }, + ], + }, + { + code: ` +namespace Test { + export enum Bar { + A = 1, + } +} +namespace Test { + export enum Bar { + B = 'B', + } +} + `, + errors: [ + { + endColumn: 12, + column: 9, + line: 9, + messageId: 'mixed', + }, + ], + }, + { + code: ` +namespace Test { + export enum Bar { + A, + } +} +namespace Test { + export enum Bar { + B = 'B', + } +} + `, + errors: [ + { + endColumn: 12, + column: 9, + line: 9, + messageId: 'mixed', + }, + ], + }, + { + code: ` +namespace Outer { + export namespace Test { + export enum Bar { + A = 1, + } + } +} +namespace Outer { + export namespace Test { + export enum Bar { + B = 'B', + } + } +} + `, + errors: [ + { + endColumn: 14, + column: 11, + line: 12, + messageId: 'mixed', + }, + ], + }, + ], +});