Skip to content

Commit 915f9c2

Browse files
JoshuaKGoldbergZamiellbradzacherJosh-Cena
authored
feat(eslint-plugin): [no-unsafe-enum-comparison] add rule (#6107)
* feat: adding enums to member-ordering rule * feat(eslint-plugin): [strict-enums] adding check for null/undefined * feat(eslint-plugin): [strict-enums] refactoring tests to separate files * feat(eslint-plugin): [strict-enums] allowing more operators * fix(eslint-plugin): [strict-enums] adding union test * fix(eslint-plugin): [strict-enums] refactoring + adding failing class test * fix(eslint-plugin): [strict-enums] adding constraint code * fix(eslint-plugin): [strict-enums] better eslint messages * fix(eslint-plugin): [strict-enums] removing vscode setting changes * fix: changing function definition to reflect reality * fix: pass string enum literal into function that take string * fix: allow passing enums to functions with any/unknown * fix: using flags instead of names also fixes for codebase * fix: adding test that breaks the rule * fix: adding test for variadic functions * fix: adding isSymbolFlagSet internally * fix: adding ignoring for remaining lint failures * fix: better comments * fix: broken test * fix: adding failing test for generic functions * fix: refactoring tests + adding tests * fix: refactoring enum helper function locations * fix: cleanup * fix: refactoring + fixing tests * fix: more tests * fix: refactoring and making tests pass * fix: adding array code, all tests pass now * fix: adding failing test * fix: allow empty arrays * fix: adding logic for arrays with no enums * fix: adding more tests * fix: fixing test * fix: fixing linter * fix: reverting comment fixes * fix: removing refactor * fix: removing fixes to dot-notation * fix: removing semi refactor * fix: removing jest logic * fix: removing comparison operators check * fix: adding failing test * fix: making test pass * fix: adding comment * fix: adding back in bitwise operator checks since apparently they are needed * fix: remove bad comment * fix: removing unnecessary comments * fix: remove types from error messages * fix: removing isArray + refactoring * Update packages/eslint-plugin/src/rules/strict-enums.ts Co-authored-by: Brad Zacher <[email protected]> * fix: removing strict-enums from recommended * fix: simplify * fix: undoing refactoring * fix: undoing refactoring * fix: moving tests into subdirectory * Update packages/eslint-plugin/src/rules/strict-enums.ts Co-authored-by: Brad Zacher <[email protected]> * Update packages/eslint-plugin/src/rules/strict-enums.ts Co-authored-by: Brad Zacher <[email protected]> * fix: adding failing test * fix: making boolean tests pass * fix: refactor tests + fix linter * fix: adding brads tests * fix: brads tests now pass * Update packages/eslint-plugin/docs/rules/strict-enums.md Co-authored-by: Joshua Chen <[email protected]> * Update packages/eslint-plugin/src/rules/strict-enums.ts Co-authored-by: Brad Zacher <[email protected]> * Update packages/eslint-plugin/src/rules/strict-enums.ts Co-authored-by: Brad Zacher <[email protected]> * Update packages/eslint-plugin/src/rules/strict-enums.ts Co-authored-by: Brad Zacher <[email protected]> * fix: make brads updates actually compile * Update strict-enums.ts * Continued fixing merge conflicts * Continued fixing the build * Passing build * Update packages/eslint-plugin/src/rules/strict-enums.ts * Update packages/eslint-plugin/src/rules/strict-enums.ts * A few more reverts * Just a bit more changing typeFlagUtils * Fixed strict-enums.md build * Convert tests to not pushing * Simplified the rule a whole bunch * Add back getEnumNames * Even more trimming down * ...and just a bit more * Undo some JSDoc changes * Progress: no-unsafe-enum-assignment is tested * A bit more testing for assignments, as requested * Finished testing and sharing * Added comparison operators * Added back no-unsafe-enum-comparison * Remove unrelated changes * Reduce coverage * Touched up docs --------- Co-authored-by: James <[email protected]> Co-authored-by: Brad Zacher <[email protected]> Co-authored-by: Joshua Chen <[email protected]>
1 parent 75806c9 commit 915f9c2

File tree

9 files changed

+806
-4
lines changed

9 files changed

+806
-4
lines changed

‎packages/eslint-plugin/docs/rules/no-unsafe-enum-comparison.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
---
2+
description: 'Disallow comparing an enum value with a non-enum value.'
3+
---
4+
5+
> 🛑 This file is source code, not the primary documentation location! 🛑
6+
>
7+
> See **https://typescript-eslint.io/rules/no-unsafe-enum-comparison** for documentation.
8+
9+
The TypeScript compiler can be surprisingly lenient when working with enums.
10+
For example, it will allow you to compare enum values against numbers even though they might not have any type overlap:
11+
12+
```ts
13+
enum Fruit {
14+
Apple,
15+
Banana,
16+
}
17+
18+
declare let fruit: Fruit;
19+
20+
fruit === 999; // No error
21+
```
22+
23+
This rule flags when an enum typed value is compared to a non-enum `number`.
24+
25+
<!--tabs-->
26+
27+
### ❌ Incorrect
28+
29+
```ts
30+
enum Fruit {
31+
Apple,
32+
}
33+
34+
declare let fruit: Fruit;
35+
36+
fruit === 999;
37+
```
38+
39+
```ts
40+
enum Vegetable {
41+
Asparagus = 'asparagus',
42+
}
43+
44+
declare let vegetable: Vegetable;
45+
46+
vegetable === 'asparagus';
47+
```
48+
49+
### ✅ Correct
50+
51+
```ts
52+
enum Fruit {
53+
Apple,
54+
}
55+
56+
declare let fruit: Fruit;
57+
58+
fruit === Fruit.Banana;
59+
```
60+
61+
```ts
62+
enum Vegetable {
63+
Asparagus = 'asparagus',
64+
}
65+
66+
declare let vegetable: Vegetable;
67+
68+
vegetable === Vegetable.Asparagus;
69+
```
70+
71+
<!--/tabs-->
72+
73+
## When Not to Use It
74+
75+
If you don't mind number and/or literal string constants being compared against enums, you likely don't need this rule.

‎packages/eslint-plugin/src/configs/all.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ export = {
116116
'@typescript-eslint/no-unsafe-assignment': 'error',
117117
'@typescript-eslint/no-unsafe-call': 'error',
118118
'@typescript-eslint/no-unsafe-declaration-merging': 'error',
119+
'@typescript-eslint/no-unsafe-enum-comparison': 'error',
119120
'@typescript-eslint/no-unsafe-member-access': 'error',
120121
'@typescript-eslint/no-unsafe-return': 'error',
121122
'no-unused-expressions': 'off',

‎packages/eslint-plugin/src/configs/strict.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export = {
2929
'@typescript-eslint/no-unnecessary-condition': 'warn',
3030
'@typescript-eslint/no-unnecessary-type-arguments': 'warn',
3131
'@typescript-eslint/no-unsafe-declaration-merging': 'warn',
32+
'@typescript-eslint/no-unsafe-enum-comparison': 'warn',
3233
'no-useless-constructor': 'off',
3334
'@typescript-eslint/no-useless-constructor': 'warn',
3435
'@typescript-eslint/non-nullable-type-assertion-style': 'warn',

‎packages/eslint-plugin/src/rules/enum-utils/shared.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import * as tsutils from 'tsutils';
2+
import * as ts from 'typescript';
3+
4+
import * as util from '../../util';
5+
6+
/*
7+
* If passed an enum member, returns the type of the parent. Otherwise,
8+
* returns itself.
9+
*
10+
* For example:
11+
* - `Fruit` --> `Fruit`
12+
* - `Fruit.Apple` --> `Fruit`
13+
*/
14+
function getBaseEnumType(typeChecker: ts.TypeChecker, type: ts.Type): ts.Type {
15+
const symbol = type.getSymbol()!;
16+
if (!tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.EnumMember)) {
17+
return type;
18+
}
19+
20+
return typeChecker.getTypeAtLocation(symbol.valueDeclaration!.parent);
21+
}
22+
23+
/**
24+
* A type can have 0 or more enum types. For example:
25+
* - 123 --> []
26+
* - {} --> []
27+
* - Fruit.Apple --> [Fruit]
28+
* - Fruit.Apple | Vegetable.Lettuce --> [Fruit, Vegetable]
29+
* - Fruit.Apple | Vegetable.Lettuce | 123 --> [Fruit, Vegetable]
30+
* - T extends Fruit --> [Fruit]
31+
*/
32+
export function getEnumTypes(
33+
typeChecker: ts.TypeChecker,
34+
type: ts.Type,
35+
): ts.Type[] {
36+
return tsutils
37+
.unionTypeParts(type)
38+
.filter(subType => util.isTypeFlagSet(subType, ts.TypeFlags.EnumLiteral))
39+
.map(type => getBaseEnumType(typeChecker, type));
40+
}

‎packages/eslint-plugin/src/rules/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ import noUnsafeArgument from './no-unsafe-argument';
8585
import noUnsafeAssignment from './no-unsafe-assignment';
8686
import noUnsafeCall from './no-unsafe-call';
8787
import noUnsafeDeclarationMerging from './no-unsafe-declaration-merging';
88+
import noUnsafeEnumComparison from './no-unsafe-enum-comparison';
8889
import noUnsafeMemberAccess from './no-unsafe-member-access';
8990
import noUnsafeReturn from './no-unsafe-return';
9091
import noUnusedExpressions from './no-unused-expressions';
@@ -222,6 +223,7 @@ export default {
222223
'no-unsafe-assignment': noUnsafeAssignment,
223224
'no-unsafe-call': noUnsafeCall,
224225
'no-unsafe-declaration-merging': noUnsafeDeclarationMerging,
226+
'no-unsafe-enum-comparison': noUnsafeEnumComparison,
225227
'no-unsafe-member-access': noUnsafeMemberAccess,
226228
'no-unsafe-return': noUnsafeReturn,
227229
'no-unused-expressions': noUnusedExpressions,

‎packages/eslint-plugin/src/rules/no-unsafe-enum-comparison.ts

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import type { TSESTree } from '@typescript-eslint/utils';
2+
import * as tsutils from 'tsutils';
3+
import * as ts from 'typescript';
4+
5+
import * as util from '../util';
6+
import { getEnumTypes } from './enum-utils/shared';
7+
8+
/**
9+
* @returns Whether the right type is an unsafe comparison against any left type.
10+
*/
11+
function typeViolates(leftTypeParts: ts.Type[], right: ts.Type): boolean {
12+
const leftValueKinds = new Set(leftTypeParts.map(getEnumValueType));
13+
14+
return (
15+
(leftValueKinds.has(ts.TypeFlags.Number) &&
16+
tsutils.isTypeFlagSet(
17+
right,
18+
ts.TypeFlags.Number | ts.TypeFlags.NumberLike,
19+
)) ||
20+
(leftValueKinds.has(ts.TypeFlags.String) &&
21+
tsutils.isTypeFlagSet(
22+
right,
23+
ts.TypeFlags.String | ts.TypeFlags.StringLike,
24+
))
25+
);
26+
}
27+
28+
/**
29+
* @returns What type a type's enum value is (number or string), if either.
30+
*/
31+
function getEnumValueType(type: ts.Type): ts.TypeFlags | undefined {
32+
return util.isTypeFlagSet(type, ts.TypeFlags.EnumLike)
33+
? util.isTypeFlagSet(type, ts.TypeFlags.NumberLiteral)
34+
? ts.TypeFlags.Number
35+
: ts.TypeFlags.String
36+
: undefined;
37+
}
38+
39+
export default util.createRule({
40+
name: 'no-unsafe-enum-comparison',
41+
meta: {
42+
type: 'suggestion',
43+
docs: {
44+
description: 'Disallow comparing an enum value with a non-enum value',
45+
recommended: 'strict',
46+
requiresTypeChecking: true,
47+
},
48+
messages: {
49+
mismatched:
50+
'The two values in this comparison do not have a shared enum type.',
51+
},
52+
schema: [],
53+
},
54+
defaultOptions: [],
55+
create(context) {
56+
const parserServices = util.getParserServices(context);
57+
const typeChecker = parserServices.program.getTypeChecker();
58+
59+
function getTypeFromNode(node: TSESTree.Node): ts.Type {
60+
return typeChecker.getTypeAtLocation(
61+
parserServices.esTreeNodeToTSNodeMap.get(node),
62+
);
63+
}
64+
65+
return {
66+
'BinaryExpression[operator=/=|<|>/]'(
67+
node: TSESTree.BinaryExpression,
68+
): void {
69+
const left = getTypeFromNode(node.left);
70+
const right = getTypeFromNode(node.right);
71+
72+
// Allow comparisons that don't have anything to do with enums:
73+
//
74+
// ```ts
75+
// 1 === 2;
76+
// ```
77+
const leftEnumTypes = getEnumTypes(typeChecker, left);
78+
const rightEnumTypes = new Set(getEnumTypes(typeChecker, right));
79+
if (leftEnumTypes.length === 0 && rightEnumTypes.size === 0) {
80+
return;
81+
}
82+
83+
// Allow comparisons that share an enum type:
84+
//
85+
// ```ts
86+
// Fruit.Apple === Fruit.Banana;
87+
// ```
88+
for (const leftEnumType of leftEnumTypes) {
89+
if (rightEnumTypes.has(leftEnumType)) {
90+
return;
91+
}
92+
}
93+
94+
const leftTypeParts = tsutils.unionTypeParts(left);
95+
const rightTypeParts = tsutils.unionTypeParts(right);
96+
97+
// If a type exists in both sides, we consider this comparison safe:
98+
//
99+
// ```ts
100+
// declare const fruit: Fruit.Apple | 0;
101+
// fruit === 0;
102+
// ```
103+
for (const leftTypePart of leftTypeParts) {
104+
if (rightTypeParts.includes(leftTypePart)) {
105+
return;
106+
}
107+
}
108+
109+
if (
110+
typeViolates(leftTypeParts, right) ||
111+
typeViolates(rightTypeParts, left)
112+
) {
113+
context.report({
114+
messageId: 'mismatched',
115+
node,
116+
});
117+
}
118+
},
119+
};
120+
},
121+
});

0 commit comments

Comments
 (0)