diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-enum-comparison.md b/packages/eslint-plugin/docs/rules/no-unsafe-enum-comparison.md new file mode 100644 index 000000000000..6931d46d1b36 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unsafe-enum-comparison.md @@ -0,0 +1,75 @@ +--- +description: 'Disallow comparing an enum value with a non-enum value.' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/no-unsafe-enum-comparison** for documentation. + +The TypeScript compiler can be surprisingly lenient when working with enums. +For example, it will allow you to compare enum values against numbers even though they might not have any type overlap: + +```ts +enum Fruit { + Apple, + Banana, +} + +declare let fruit: Fruit; + +fruit === 999; // No error +``` + +This rule flags when an enum typed value is compared to a non-enum `number`. + + + +### ❌ Incorrect + +```ts +enum Fruit { + Apple, +} + +declare let fruit: Fruit; + +fruit === 999; +``` + +```ts +enum Vegetable { + Asparagus = 'asparagus', +} + +declare let vegetable: Vegetable; + +vegetable === 'asparagus'; +``` + +### ✅ Correct + +```ts +enum Fruit { + Apple, +} + +declare let fruit: Fruit; + +fruit === Fruit.Banana; +``` + +```ts +enum Vegetable { + Asparagus = 'asparagus', +} + +declare let vegetable: Vegetable; + +vegetable === Vegetable.Asparagus; +``` + + + +## When Not to Use It + +If you don't mind number and/or literal string constants being compared against enums, you likely don't need this rule. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index e712d0b6f290..777cf069827e 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -116,6 +116,7 @@ export = { '@typescript-eslint/no-unsafe-assignment': 'error', '@typescript-eslint/no-unsafe-call': 'error', '@typescript-eslint/no-unsafe-declaration-merging': 'error', + '@typescript-eslint/no-unsafe-enum-comparison': 'error', '@typescript-eslint/no-unsafe-member-access': 'error', '@typescript-eslint/no-unsafe-return': 'error', 'no-unused-expressions': 'off', diff --git a/packages/eslint-plugin/src/configs/strict.ts b/packages/eslint-plugin/src/configs/strict.ts index c63b4173452b..26b71c931b00 100644 --- a/packages/eslint-plugin/src/configs/strict.ts +++ b/packages/eslint-plugin/src/configs/strict.ts @@ -29,6 +29,7 @@ export = { '@typescript-eslint/no-unnecessary-condition': 'warn', '@typescript-eslint/no-unnecessary-type-arguments': 'warn', '@typescript-eslint/no-unsafe-declaration-merging': 'warn', + '@typescript-eslint/no-unsafe-enum-comparison': 'warn', 'no-useless-constructor': 'off', '@typescript-eslint/no-useless-constructor': 'warn', '@typescript-eslint/non-nullable-type-assertion-style': 'warn', diff --git a/packages/eslint-plugin/src/rules/enum-utils/shared.ts b/packages/eslint-plugin/src/rules/enum-utils/shared.ts new file mode 100644 index 000000000000..6a3349143aa6 --- /dev/null +++ b/packages/eslint-plugin/src/rules/enum-utils/shared.ts @@ -0,0 +1,40 @@ +import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; + +import * as util from '../../util'; + +/* + * If passed an enum member, returns the type of the parent. Otherwise, + * returns itself. + * + * For example: + * - `Fruit` --> `Fruit` + * - `Fruit.Apple` --> `Fruit` + */ +function getBaseEnumType(typeChecker: ts.TypeChecker, type: ts.Type): ts.Type { + const symbol = type.getSymbol()!; + if (!tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.EnumMember)) { + return type; + } + + return typeChecker.getTypeAtLocation(symbol.valueDeclaration!.parent); +} + +/** + * A type can have 0 or more enum types. For example: + * - 123 --> [] + * - {} --> [] + * - Fruit.Apple --> [Fruit] + * - Fruit.Apple | Vegetable.Lettuce --> [Fruit, Vegetable] + * - Fruit.Apple | Vegetable.Lettuce | 123 --> [Fruit, Vegetable] + * - T extends Fruit --> [Fruit] + */ +export function getEnumTypes( + typeChecker: ts.TypeChecker, + type: ts.Type, +): ts.Type[] { + return tsutils + .unionTypeParts(type) + .filter(subType => util.isTypeFlagSet(subType, ts.TypeFlags.EnumLiteral)) + .map(type => getBaseEnumType(typeChecker, type)); +} diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 88f6d8c2cb82..405514edd1f0 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -85,6 +85,7 @@ import noUnsafeArgument from './no-unsafe-argument'; import noUnsafeAssignment from './no-unsafe-assignment'; import noUnsafeCall from './no-unsafe-call'; import noUnsafeDeclarationMerging from './no-unsafe-declaration-merging'; +import noUnsafeEnumComparison from './no-unsafe-enum-comparison'; import noUnsafeMemberAccess from './no-unsafe-member-access'; import noUnsafeReturn from './no-unsafe-return'; import noUnusedExpressions from './no-unused-expressions'; @@ -222,6 +223,7 @@ export default { 'no-unsafe-assignment': noUnsafeAssignment, 'no-unsafe-call': noUnsafeCall, 'no-unsafe-declaration-merging': noUnsafeDeclarationMerging, + 'no-unsafe-enum-comparison': noUnsafeEnumComparison, 'no-unsafe-member-access': noUnsafeMemberAccess, 'no-unsafe-return': noUnsafeReturn, 'no-unused-expressions': noUnusedExpressions, diff --git a/packages/eslint-plugin/src/rules/no-unsafe-enum-comparison.ts b/packages/eslint-plugin/src/rules/no-unsafe-enum-comparison.ts new file mode 100644 index 000000000000..ac7a64727bcb --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unsafe-enum-comparison.ts @@ -0,0 +1,121 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; + +import * as util from '../util'; +import { getEnumTypes } from './enum-utils/shared'; + +/** + * @returns Whether the right type is an unsafe comparison against any left type. + */ +function typeViolates(leftTypeParts: ts.Type[], right: ts.Type): boolean { + const leftValueKinds = new Set(leftTypeParts.map(getEnumValueType)); + + return ( + (leftValueKinds.has(ts.TypeFlags.Number) && + tsutils.isTypeFlagSet( + right, + ts.TypeFlags.Number | ts.TypeFlags.NumberLike, + )) || + (leftValueKinds.has(ts.TypeFlags.String) && + tsutils.isTypeFlagSet( + right, + ts.TypeFlags.String | ts.TypeFlags.StringLike, + )) + ); +} + +/** + * @returns What type a type's enum value is (number or string), if either. + */ +function getEnumValueType(type: ts.Type): ts.TypeFlags | undefined { + return util.isTypeFlagSet(type, ts.TypeFlags.EnumLike) + ? util.isTypeFlagSet(type, ts.TypeFlags.NumberLiteral) + ? ts.TypeFlags.Number + : ts.TypeFlags.String + : undefined; +} + +export default util.createRule({ + name: 'no-unsafe-enum-comparison', + meta: { + type: 'suggestion', + docs: { + description: 'Disallow comparing an enum value with a non-enum value', + recommended: 'strict', + requiresTypeChecking: true, + }, + messages: { + mismatched: + 'The two values in this comparison do not have a shared enum type.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const parserServices = util.getParserServices(context); + const typeChecker = parserServices.program.getTypeChecker(); + + function getTypeFromNode(node: TSESTree.Node): ts.Type { + return typeChecker.getTypeAtLocation( + parserServices.esTreeNodeToTSNodeMap.get(node), + ); + } + + return { + 'BinaryExpression[operator=/=|<|>/]'( + node: TSESTree.BinaryExpression, + ): void { + const left = getTypeFromNode(node.left); + const right = getTypeFromNode(node.right); + + // Allow comparisons that don't have anything to do with enums: + // + // ```ts + // 1 === 2; + // ``` + const leftEnumTypes = getEnumTypes(typeChecker, left); + const rightEnumTypes = new Set(getEnumTypes(typeChecker, right)); + if (leftEnumTypes.length === 0 && rightEnumTypes.size === 0) { + return; + } + + // Allow comparisons that share an enum type: + // + // ```ts + // Fruit.Apple === Fruit.Banana; + // ``` + for (const leftEnumType of leftEnumTypes) { + if (rightEnumTypes.has(leftEnumType)) { + return; + } + } + + const leftTypeParts = tsutils.unionTypeParts(left); + const rightTypeParts = tsutils.unionTypeParts(right); + + // If a type exists in both sides, we consider this comparison safe: + // + // ```ts + // declare const fruit: Fruit.Apple | 0; + // fruit === 0; + // ``` + for (const leftTypePart of leftTypeParts) { + if (rightTypeParts.includes(leftTypePart)) { + return; + } + } + + if ( + typeViolates(leftTypeParts, right) || + typeViolates(rightTypeParts, left) + ) { + context.report({ + messageId: 'mismatched', + node, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-enum-comparison.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-enum-comparison.test.ts new file mode 100644 index 000000000000..5e84c983076c --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unsafe-enum-comparison.test.ts @@ -0,0 +1,553 @@ +import rule from '../../src/rules/no-unsafe-enum-comparison'; +import { getFixturesRootDir, RuleTester } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2015, + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('strict-enums-comparison', rule, { + valid: [ + "'a' > 'b';", + "'a' < 'b';", + "'a' == 'b';", + "'a' === 'b';", + '1 > 2;', + '1 < 2;', + '1 == 2;', + '1 === 2;', + ` + enum Fruit { + Apple, + } + Fruit.Apple === ({} as any); + `, + ` + enum Fruit { + Apple, + } + Fruit.Apple === undefined; + `, + ` + enum Fruit { + Apple, + } + Fruit.Apple === null; + `, + ` + enum Fruit { + Apple, + } + declare const fruit: Fruit | -1; + fruit === -1; + `, + ` + enum Fruit { + Apple, + } + declare const fruit: Fruit | number; + fruit === -1; + `, + ` + enum Fruit { + Apple, + } + declare const fruit: Fruit | 'apple'; + fruit === 'apple'; + `, + ` + enum Fruit { + Apple, + } + declare const fruit: Fruit | string; + fruit === 'apple'; + `, + ` + enum Fruit { + Apple = 'apple', + } + declare const fruit: Fruit | 'apple'; + fruit === 'apple'; + `, + ` + enum Fruit { + Apple = 'apple', + } + declare const fruit: Fruit | string; + fruit === 'apple'; + `, + ` + enum Fruit { + Apple = 'apple', + } + declare const fruit: Fruit | 0; + fruit === 0; + `, + ` + enum Fruit { + Apple = 'apple', + } + declare const fruit: Fruit | number; + fruit === 0; + `, + ` + enum Fruit { + Apple, + } + declare const fruit: Fruit | 'apple'; + fruit === Math.random() > 0.5 ? 'apple' : Fruit.Apple; + `, + ` + enum Fruit { + Apple = 'apple', + } + declare const fruit: Fruit | 'apple'; + fruit === Math.random() > 0.5 ? 'apple' : Fruit.Apple; + `, + ` + enum Fruit { + Apple = 'apple', + } + declare const fruit: Fruit | string; + fruit === Math.random() > 0.5 ? 'apple' : Fruit.Apple; + `, + ` + enum Fruit { + Apple = 'apple', + } + declare const fruit: Fruit | 0; + fruit === Math.random() > 0.5 ? 0 : Fruit.Apple; + `, + ` + enum Fruit { + Apple = 'apple', + } + declare const fruit: Fruit | number; + fruit === Math.random() > 0.5 ? 0 : Fruit.Apple; + `, + ` + enum Fruit { + Apple, + Banana, + } + Fruit.Apple === Fruit.Banana; + `, + ` + enum Fruit { + Apple = 0, + Banana = 1, + } + Fruit.Apple === Fruit.Banana; + `, + ` + enum Fruit { + Apple = 'apple', + Banana = 'banana', + } + Fruit.Apple === Fruit.Banana; + `, + ` + enum Fruit { + Apple, + Banana, + } + const fruit = Fruit.Apple; + fruit === Fruit.Banana; + `, + ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + const vegetable = Vegetable.Asparagus; + vegetable === Vegetable.Beet; + `, + ` + enum Fruit { + Apple, + Banana, + Cherry, + } + const fruit1 = Fruit.Apple; + const fruit2 = Fruit.Banana; + fruit1 === fruit2; + `, + ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + const vegetable1 = Vegetable.Asparagus; + const vegetable2 = Vegetable.Beet; + vegetable1 === vegetable2; + `, + ` + enum Fruit { + Apple, + Banana, + Cherry, + } + enum Fruit2 { + Apple2, + Banana2, + Cherry2, + } + declare const left: number | Fruit; + declare const right: number | Fruit2; + left === right; + `, + ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + enum Vegetable2 { + Asparagus2 = 'asparagus2', + Beet2 = 'beet2', + Celery2 = 'celery2', + } + declare const left: string | Vegetable; + declare const right: string | Vegetable2; + left === right; + `, + ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + type WeirdString = string & { __someBrand: void }; + declare const weirdString: WeirdString; + Vegetable.Asparagus === weirdString; + `, + ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + const foo = {}; + const vegetable = Vegetable.Asparagus; + vegetable in foo; + `, + ` + enum Fruit { + Apple, + Banana, + Cherry, + } + declare const fruitOrBoolean: Fruit | boolean; + fruitOrBoolean === true; + `, + ` + enum Str { + A = 'a', + } + enum Num { + B = 1, + } + enum Mixed { + A = 'a', + B = 1, + } + + declare const str: Str; + declare const strOrString: Str | string; + + declare const num: Num; + declare const numOrNumber: Num | number; + + declare const mixed: Mixed; + declare const mixedOrStringOrNumber: Mixed | string | number; + + function someFunction() {} + + // following are all ignored due to the presence of "| string" or "| number" + strOrString === 'a'; + numOrNumber === 1; + mixedOrStringOrNumber === 'a'; + mixedOrStringOrNumber === 1; + + // following are all ignored because the value can never be an enum value + str === 1; + num === 'a'; + str === {}; + num === {}; + mixed === {}; + str === true; + num === true; + mixed === true; + str === someFunction; + num === someFunction; + mixed === someFunction; + `, + ], + invalid: [ + { + code: ` + enum Fruit { + Apple, + } + Fruit.Apple < 1; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple, + } + Fruit.Apple > 1; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple, + } + Fruit.Apple == 1; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple, + } + Fruit.Apple === 1; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple, + } + Fruit.Apple != 1; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple, + } + Fruit.Apple !== 1; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple = 0, + Banana = 'banana', + } + Fruit.Apple === 0; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple = 0, + Banana = 'banana', + } + Fruit.Banana === ''; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + Vegetable.Asparagus === 'beet'; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple, + Banana, + Cherry, + } + 1 === Fruit.Apple; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + 'beet' === Vegetable.Asparagus; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple, + Banana, + Cherry, + } + const fruit = Fruit.Apple; + fruit === 1; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + const vegetable = Vegetable.Asparagus; + vegetable === 'beet'; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Fruit { + Apple, + Banana, + Cherry, + } + const fruit = Fruit.Apple; + 1 === fruit; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + const vegetable = Vegetable.Asparagus; + 'beet' === vegetable; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: + 'enum Fruit { Apple, Banana, Cherry }' + + `enum Fruit2 { + Apple2, + Banana2, + Cherry2, +} + Fruit.Apple === Fruit2.Apple2; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + enum Vegetable2 { + Asparagus2 = 'asparagus2', + Beet2 = 'beet2', + Celery2 = 'celery2', + } + Vegetable.Asparagus === Vegetable2.Asparagus2; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: + 'enum Fruit { Apple, Banana, Cherry }' + + `enum Fruit2 { + Apple2, + Banana2, + Cherry2, +} + const fruit = Fruit.Apple; + fruit === Fruit2.Apple2; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Vegetable { + Asparagus = 'asparagus', + Beet = 'beet', + Celery = 'celery', + } + enum Vegetable2 { + Asparagus2 = 'asparagus2', + Beet2 = 'beet2', + Celery2 = 'celery2', + } + const vegetable = Vegetable.Asparagus; + vegetable === Vegetable2.Asparagus2; + `, + errors: [{ messageId: 'mismatched' }], + }, + { + code: ` + enum Str { + A = 'a', + } + enum Num { + B = 1, + } + enum Mixed { + A = 'a', + B = 1, + } + + declare const str: Str; + declare const num: Num; + declare const mixed: Mixed; + + // following are all errors because the value might be an enum value + str === 'a'; + num === 1; + mixed === 'a'; + mixed === 1; + `, + errors: [ + { messageId: 'mismatched' }, + { messageId: 'mismatched' }, + { messageId: 'mismatched' }, + { messageId: 'mismatched' }, + ], + }, + { + code: ` + enum Fruit { + Apple = 'apple', + } + type __String = + | (string & { __escapedIdentifier: void }) + | (void & { __escapedIdentifier: void }) + | Fruit; + declare const weirdString: __String; + weirdString === 'someArbitraryValue'; + `, + errors: [{ messageId: 'mismatched' }], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index 0f0b38a5ba6f..e08a8fb42267 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -4,6 +4,8 @@ declare module 'typescript' { interface TypeChecker { // internal TS APIs + getContextualTypeForArgumentAtIndex(node: Node, argIndex: number): Type; + /** * @returns `true` if the given type is an array type: * - `Array` diff --git a/packages/type-utils/src/typeFlagUtils.ts b/packages/type-utils/src/typeFlagUtils.ts index aa71d702b19a..24d6c5be28b4 100644 --- a/packages/type-utils/src/typeFlagUtils.ts +++ b/packages/type-utils/src/typeFlagUtils.ts @@ -1,8 +1,10 @@ import { unionTypeParts } from 'tsutils'; import * as ts from 'typescript'; +const ANY_OR_UNKNOWN = ts.TypeFlags.Any | ts.TypeFlags.Unknown; + /** - * Gets all of the type flags in a type, iterating through unions automatically + * Gets all of the type flags in a type, iterating through unions automatically. */ export function getTypeFlags(type: ts.Type): ts.TypeFlags { // @ts-expect-error Since typescript 5.0, this is invalid, but uses 0 as the default value of TypeFlags. @@ -14,8 +16,13 @@ export function getTypeFlags(type: ts.Type): ts.TypeFlags { } /** - * Checks if the given type is (or accepts) the given flags - * @param isReceiver true if the type is a receiving type (i.e. the type of a called function's parameter) + * @param flagsToCheck The composition of one or more `ts.TypeFlags`. + * @param isReceiver Whether the type is a receiving type (e.g. the type of a + * called function's parameter). + * @remarks + * Note that if the type is a union, this function will decompose it into the + * parts and get the flags of every union constituent. If this is not desired, + * use the `isTypeFlag` function from tsutils. */ export function isTypeFlagSet( type: ts.Type, @@ -24,7 +31,7 @@ export function isTypeFlagSet( ): boolean { const flags = getTypeFlags(type); - if (isReceiver && flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) { + if (isReceiver && flags & ANY_OR_UNKNOWN) { return true; }