Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): [no-unsafe-enum-comparison] add switch suggestion #7691

Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 52 additions & 4 deletions packages/eslint-plugin/src/rules/enum-utils/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,23 @@ function getBaseEnumType(typeChecker: ts.TypeChecker, type: ts.Type): ts.Type {
return typeChecker.getTypeAtLocation(symbol.valueDeclaration!.parent);
}

/**
* Retrieve only the Enum literals from a type. for example:
* - 123 --> []
* - {} --> []
* - Fruit.Apple --> [Fruit.Apple]
* - Fruit.Apple | Vegetable.Lettuce --> [Fruit.Apple, Vegetable.Lettuce]
* - Fruit.Apple | Vegetable.Lettuce | 123 --> [Fruit.Apple, Vegetable.Lettuce]
* - T extends Fruit --> [Fruit]
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Praise] Great comments, thanks for continuing the existing standard 🙂

export function getEnumLiterals(type: ts.Type): ts.LiteralType[] {
return tsutils
.unionTypeParts(type)
.filter((subType): subType is ts.LiteralType =>
isTypeFlagSet(subType, ts.TypeFlags.EnumLiteral),
);
}

/**
* A type can have 0 or more enum types. For example:
* - 123 --> []
Expand All @@ -33,8 +50,39 @@ export function getEnumTypes(
typeChecker: ts.TypeChecker,
type: ts.Type,
): ts.Type[] {
return tsutils
.unionTypeParts(type)
.filter(subType => isTypeFlagSet(subType, ts.TypeFlags.EnumLiteral))
.map(type => getBaseEnumType(typeChecker, type));
return getEnumLiterals(type).map(type => getBaseEnumType(typeChecker, type));
}

/**
* Returns the enum key that matches the given literal node, or null if none
* match. For example:
* ```ts
* enum Fruit {
* Apple = 'apple',
* Banana = 'banana',
* }
*
* getEnumKeyForLiteral([Fruit.Apple, Fruit.Banana], 'apple') --> 'Fruit.Apple'
* getEnumKeyForLiteral([Fruit.Apple, Fruit.Banana], 'banana') --> 'Fruit.Banana'
* getEnumKeyForLiteral([Fruit.Apple, Fruit.Banana], 'cherry') --> null
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
* ```
*/
export function getEnumKeyForLiteral(
enumLiterals: ts.LiteralType[],
literal: unknown,
): string | null {
for (const enumLiteral of enumLiterals) {
if (enumLiteral.value === literal) {
const memberSymbol = enumLiteral.symbol;
const enumSymbol = (memberSymbol.valueDeclaration as ts.EnumMember)
.parent;

const enumName = enumSymbol.name.getText();
const memberName = memberSymbol.name;

return `${enumName}.${memberName}`;
}
}

return null;
}
49 changes: 46 additions & 3 deletions packages/eslint-plugin/src/rules/no-unsafe-enum-comparison.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import type { TSESTree } from '@typescript-eslint/utils';
import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
import * as tsutils from 'ts-api-utils';
import * as ts from 'typescript';

import { createRule, getParserServices } from '../util';
import { getEnumTypes } from './enum-utils/shared';
import { createRule, getParserServices, getStaticValue } from '../util';
import {
getEnumKeyForLiteral,
getEnumLiterals,
getEnumTypes,
} from './enum-utils/shared';

/**
* @returns Whether the right type is an unsafe comparison against any left type.
Expand Down Expand Up @@ -39,6 +43,7 @@ function getEnumValueType(type: ts.Type): ts.TypeFlags | undefined {
export default createRule({
name: 'no-unsafe-enum-comparison',
meta: {
hasSuggestions: true,
type: 'suggestion',
docs: {
description: 'Disallow comparing an enum value with a non-enum value',
Expand All @@ -48,6 +53,7 @@ export default createRule({
messages: {
mismatched:
'The two values in this comparison do not have a shared enum type.',
replaceValueWithEnum: 'Replace with an enum value comparison.',
},
schema: [],
},
Expand Down Expand Up @@ -107,6 +113,43 @@ export default createRule({
context.report({
messageId: 'mismatched',
node,
suggest: [
{
messageId: 'replaceValueWithEnum',
fix(fixer): TSESLint.RuleFix | null {
// Replace the right side with an enum key if possible:
//
// ```ts
// Fruit.Apple === 'apple'; // Fruit.Apple === Fruit.Apple
// ```
const leftEnumKey = getEnumKeyForLiteral(
getEnumLiterals(left),
getStaticValue(node.right)?.value,
);

if (leftEnumKey) {
return fixer.replaceText(node.right, leftEnumKey);
}

// Replace the left side with an enum key if possible:
//
// ```ts
// declare const fruit: Fruit;
// 'apple' === Fruit.Apple; // Fruit.Apple === Fruit.Apple
// ```
const rightEnumKey = getEnumKeyForLiteral(
getEnumLiterals(right),
getStaticValue(node.left)?.value,
);

if (rightEnumKey) {
return fixer.replaceText(node.left, rightEnumKey);
}

return null;
},
},
],
});
}
},
Expand Down
112 changes: 112 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unsafe-enum-comparison.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -565,5 +565,117 @@ ruleTester.run('strict-enums-comparison', rule, {
`,
errors: [{ messageId: 'mismatched' }],
},
{
code: `
enum Str {
A = 'a',
B = 'b',
}
declare const str: Str;
str === 'b';
`,
errors: [
{
messageId: 'mismatched',
suggestions: [
{
messageId: 'replaceValueWithEnum',
output: `
enum Str {
A = 'a',
B = 'b',
}
declare const str: Str;
str === Str.B;
`,
},
],
},
],
},
{
code: `
enum Num {
A = 1,
B = 2,
}
declare const num: Num;
1 === num;
`,
errors: [
{
messageId: 'mismatched',
suggestions: [
{
messageId: 'replaceValueWithEnum',
output: `
enum Num {
A = 1,
B = 2,
}
declare const num: Num;
Num.A === num;
`,
},
],
},
],
},
{
code: `
enum Mixed {
A = 1,
B = 'b',
}
declare const mixed: Mixed;
mixed === 1;
`,
errors: [
{
messageId: 'mismatched',
suggestions: [
{
messageId: 'replaceValueWithEnum',
output: `
enum Mixed {
A = 1,
B = 'b',
}
declare const mixed: Mixed;
mixed === Mixed.A;
`,
},
],
},
],
},
{
code: `
enum Mixed {
A = 1,
B = 'b',
}
declare const mixed: Mixed;
mixed === 'b';
`,
errors: [
{
messageId: 'mismatched',
suggestions: [
{
messageId: 'replaceValueWithEnum',
output: `
enum Mixed {
A = 1,
B = 'b',
}
declare const mixed: Mixed;
mixed === Mixed.B;
`,
},
],
},
],
},
],
});