From 3cabb56aec226f00af2e312135f6acc439503180 Mon Sep 17 00:00:00 2001 From: Mohsen Azimi Date: Sat, 23 Feb 2019 22:34:37 -0800 Subject: [PATCH 1/2] feat(eslint-plugin): add "no-enum-literals" rule --- .../docs/rules/no-enum-literals.md | 67 +++++++++ .../src/rules/no-enum-literals.ts | 104 +++++++++++++ .../tests/rules/no-enum-literals.test.ts | 142 ++++++++++++++++++ packages/eslint-plugin/typings/ts-eslint.d.ts | 2 + 4 files changed, 315 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-enum-literals.md create mode 100644 packages/eslint-plugin/src/rules/no-enum-literals.ts create mode 100644 packages/eslint-plugin/tests/rules/no-enum-literals.test.ts diff --git a/packages/eslint-plugin/docs/rules/no-enum-literals.md b/packages/eslint-plugin/docs/rules/no-enum-literals.md new file mode 100644 index 000000000000..fd09e220291d --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-enum-literals.md @@ -0,0 +1,67 @@ +# Disallows usage of literals instead of enums + +## Rule Details + +It's possible to use number of string literals instead of enum values. This rule disallows using string or number literals instead of enum values. + +Examples of **incorrect** code for this rule: + +```ts +enum Foo { + ONE, + TWO, +} + +function foo(f: Foo) { + if (f === 1) { + } + + let ff: Foo; + + ff = 1; +} +foo(1); + +enum Bar { + ONE = 'ONE', + TWO = 'TWO', +} + +function bar(b: Bar) { + if (b === 'ONE') { + } +} +``` + +Examples of **correct** code for this rule: + +```ts +enum Foo { + ONE, + TWO, +} + +function foo(f: Foo) { + if (f === Foo.ONE) { + } + + let ff: Foo; + + ff = Foo.TWO; +} +foo(1); + +enum Bar { + ONE = 'ONE', + TWO = 'TWO', +} + +function bar(b: Bar) { + if (b === Bar.ONE) { + } +} +``` + +## When Not To Use It + +If you want to allow usage of literals instead of enums diff --git a/packages/eslint-plugin/src/rules/no-enum-literals.ts b/packages/eslint-plugin/src/rules/no-enum-literals.ts new file mode 100644 index 000000000000..0984a0b8a81e --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-enum-literals.ts @@ -0,0 +1,104 @@ +import ts from 'typescript'; +import * as util from '../util'; +import { TSESTree } from '@typescript-eslint/typescript-estree'; + +export default util.createRule({ + name: 'no-enum-literals', + meta: { + type: 'suggestion', + docs: { + description: 'Disallows usage of literals instead of enums', + category: 'Best Practices', + recommended: 'error', + }, + messages: { + noLiterals: 'Do not use literal values instead of enums', + }, + schema: [ + { + type: 'object', + additionalProperties: false, + }, + ], + }, + defaultOptions: [{}], + create(context) { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + + /** + * is node an identifier with type of an enum + * @param node identifier node + */ + function isNodeEnumIdentifier(node: TSESTree.Node): boolean { + const originalNode = parserServices.esTreeNodeToTSNodeMap.get< + ts.Identifier + >(node); + const type = checker.getTypeAtLocation(originalNode); + + if (!type.symbol) { + return false; + } + + const { name } = type.symbol; + + return !['Number', 'String'].includes(name); + } + + function isNumberOrStringLiteral( + node: TSESTree.Node, + ): node is TSESTree.Literal { + return ( + node.type === 'Literal' && + ['number', 'string'].includes(typeof node.value) + ); + } + + return { + AssignmentExpression(node) { + if ( + isNodeEnumIdentifier(node.left) && + isNumberOrStringLiteral(node.right) + ) { + context.report({ + node: node.right, + messageId: 'noLiterals', + }); + } + }, + BinaryExpression(node) { + if ( + isNodeEnumIdentifier(node.left) && + isNumberOrStringLiteral(node.right) + ) { + context.report({ + node: node.right, + messageId: 'noLiterals', + }); + } + + if ( + isNumberOrStringLiteral(node.left) && + isNodeEnumIdentifier(node.right) + ) { + context.report({ + node: node.left, + messageId: 'noLiterals', + }); + } + }, + VariableDeclarator(node) { + if ( + isNodeEnumIdentifier(node.id) && + node.init && + isNumberOrStringLiteral(node.init) + ) { + context.report({ + node: node.init, + messageId: 'noLiterals', + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-enum-literals.test.ts b/packages/eslint-plugin/tests/rules/no-enum-literals.test.ts new file mode 100644 index 000000000000..f767c20aed97 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-enum-literals.test.ts @@ -0,0 +1,142 @@ +import rule from '../../src/rules/no-enum-literals'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + project: './tsconfig.json', + }, +}); + +ruleTester.run('no-enum-literals', rule, { + valid: [ + '0 === 0', + 'var a = 1', + 'var b = "str"', + 'var a = 1; (a === 0)', + 'var a = 1; (0 === a)', + 'var a = 1; (a === a)', + 'var a = "str"; (a === "other")', + 'var a: number = 1', + 'var a: string = "str"', + 'enum Foo { ONE, TWO }; let f: Foo; (f === Foo.ONE);', + 'enum Foo { ONE, TWO }; let f: Foo; (Foo.ONE === f);', + 'enum Foo { ONE, TWO }; let f = Foo.ONE;', + 'enum Foo { ONE, TWO }; let f: Foo; f = Foo.ONE;', + 'enum Foo { ONE, TWO }; function foo(f: Foo) {}; foo(Foo.ONE);', + 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; (f === Foo.ONE);', + 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; (Foo.ONE === f);', + 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; f = Foo.ONE;', + 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; f = Foo.ONE;', + 'enum Foo { ONE = 1, TWO = 2 }; function foo(f: Foo) {}; foo(Foo.ONE);', + 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; (f === Foo.ONE);', + 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; (Foo.ONE === f);', + 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; f = Foo.ONE;', + 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo = Foo.ONE;', + 'enum Foo { ONE = "ONE", TWO = "TWO" }; function foo(f: Foo) {}; foo(Foo.ONE);', + ], + + invalid: [ + { + code: 'enum Foo { ONE, TWO }; let f: Foo; (f === 0);', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 43, + }, + ], + }, + { + code: 'enum Foo { ONE, TWO }; let f: Foo; (0 === f);', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 37, + }, + ], + }, + { + code: 'enum Foo { ONE, TWO }; let f: Foo; f = 0;', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 40, + }, + ], + }, + + { + code: 'enum Foo { ONE, TWO }; let f: Foo = 0;', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 37, + }, + ], + }, + { + code: 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; (f === 0);', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 51, + }, + ], + }, + { + code: 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; (0 === f);', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 45, + }, + ], + }, + { + code: 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; f = 0;', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 48, + }, + ], + }, + { + code: 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo = 0;', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 45, + }, + ], + }, + { + code: 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; (f === "ONE");', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 59, + }, + ], + }, + { + code: 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; ("ONE" === f);', + errors: [ + { + messageId: 'noLiterals', + line: 1, + column: 53, + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/ts-eslint.d.ts b/packages/eslint-plugin/typings/ts-eslint.d.ts index 998c69583428..421ac358a9c8 100644 --- a/packages/eslint-plugin/typings/ts-eslint.d.ts +++ b/packages/eslint-plugin/typings/ts-eslint.d.ts @@ -404,8 +404,10 @@ declare module 'ts-eslint' { ArrayExpression?: RuleFunction; ArrayPattern?: RuleFunction; ArrowFunctionExpression?: RuleFunction; + AssignmentExpression?: RuleFunction; AssignmentPattern?: RuleFunction; AwaitExpression?: RuleFunction; + BinaryExpression?: RuleFunction; BlockStatement?: RuleFunction; BreakStatement?: RuleFunction; CallExpression?: RuleFunction; From df08d28c35319183ac4894b1d9558fae73d9751c Mon Sep 17 00:00:00 2001 From: Mohsen Azimi Date: Fri, 8 Mar 2019 10:01:27 -0500 Subject: [PATCH 2/2] fix(eslint-plugin): fix tests and remove unused schema Make test code multiline Remove unused schema and defaultOption --- .../src/rules/no-enum-literals.ts | 9 +- .../tests/rules/no-enum-literals.test.ts | 182 +++++++++++++----- 2 files changed, 135 insertions(+), 56 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-enum-literals.ts b/packages/eslint-plugin/src/rules/no-enum-literals.ts index 0984a0b8a81e..912cd27ced8b 100644 --- a/packages/eslint-plugin/src/rules/no-enum-literals.ts +++ b/packages/eslint-plugin/src/rules/no-enum-literals.ts @@ -14,14 +14,9 @@ export default util.createRule({ messages: { noLiterals: 'Do not use literal values instead of enums', }, - schema: [ - { - type: 'object', - additionalProperties: false, - }, - ], + schema: [], }, - defaultOptions: [{}], + defaultOptions: [], create(context) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); diff --git a/packages/eslint-plugin/tests/rules/no-enum-literals.test.ts b/packages/eslint-plugin/tests/rules/no-enum-literals.test.ts index f767c20aed97..80bec392ce9c 100644 --- a/packages/eslint-plugin/tests/rules/no-enum-literals.test.ts +++ b/packages/eslint-plugin/tests/rules/no-enum-literals.test.ts @@ -10,131 +10,215 @@ const ruleTester = new RuleTester({ ruleTester.run('no-enum-literals', rule, { valid: [ + // Make sure non-enum BinaryExpressions are still valid '0 === 0', 'var a = 1', 'var b = "str"', - 'var a = 1; (a === 0)', - 'var a = 1; (0 === a)', - 'var a = 1; (a === a)', - 'var a = "str"; (a === "other")', + ` +var a = 1; +(a === 0)`, + ` +var a = 1; +(0 === a)`, + ` +var a = 1; +(a === a)`, + ` +var a = "str"; +(a === "other")`, + + // Make sure non-enum VariableDeclaration are still valid 'var a: number = 1', 'var a: string = "str"', - 'enum Foo { ONE, TWO }; let f: Foo; (f === Foo.ONE);', - 'enum Foo { ONE, TWO }; let f: Foo; (Foo.ONE === f);', - 'enum Foo { ONE, TWO }; let f = Foo.ONE;', - 'enum Foo { ONE, TWO }; let f: Foo; f = Foo.ONE;', - 'enum Foo { ONE, TWO }; function foo(f: Foo) {}; foo(Foo.ONE);', - 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; (f === Foo.ONE);', - 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; (Foo.ONE === f);', - 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; f = Foo.ONE;', - 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; f = Foo.ONE;', - 'enum Foo { ONE = 1, TWO = 2 }; function foo(f: Foo) {}; foo(Foo.ONE);', - 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; (f === Foo.ONE);', - 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; (Foo.ONE === f);', - 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; f = Foo.ONE;', - 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo = Foo.ONE;', - 'enum Foo { ONE = "ONE", TWO = "TWO" }; function foo(f: Foo) {}; foo(Foo.ONE);', + + // Enum cases + ` +enum Foo { ONE, TWO }; +let f: Foo; +(f === Foo.ONE);`, + ` +enum Foo { ONE, TWO }; +let f: Foo; +(Foo.ONE === f);`, + ` +enum Foo { ONE, TWO }; +let f = Foo.ONE;`, + ` +enum Foo { ONE, TWO }; +let f: Foo; +f = Foo.ONE;`, + ` +enum Foo { ONE, TWO }; +function foo(f: Foo) {}; +foo(Foo.ONE);`, + ` +enum Foo { ONE = 1, TWO = 2 }; +let f: Foo; +(f === Foo.ONE);`, + ` +enum Foo { ONE = 1, TWO = 2 }; +let f: Foo; +(Foo.ONE === f);`, + ` +enum Foo { ONE = 1, TWO = 2 }; +let f: Foo; +f = Foo.ONE;`, + ` +enum Foo { ONE = 1, TWO = 2 }; +let f: Foo; +f = Foo.ONE;`, + ` +enum Foo { ONE = 1, TWO = 2 }; +function foo(f: Foo) {}; +foo(Foo.ONE);`, + ` +enum Foo { ONE = "ONE", TWO = "TWO" }; +let f: Foo; +(f === Foo.ONE);`, + ` +enum Foo { ONE = "ONE", TWO = "TWO" }; +let f: Foo; +(Foo.ONE === f);`, + ` +enum Foo { ONE = "ONE", TWO = "TWO" }; +let f: Foo; +f = Foo.ONE;`, + ` +enum Foo { ONE = "ONE", TWO = "TWO" }; +let f: Foo = Foo.ONE;`, + ` +enum Foo { ONE = "ONE", TWO = "TWO" }; +function foo(f: Foo) {}; +foo(Foo.ONE);`, ], invalid: [ { - code: 'enum Foo { ONE, TWO }; let f: Foo; (f === 0);', + code: ` +enum Foo { ONE, TWO }; +let f: Foo; +(f === 0);`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 43, + line: 4, + column: 8, }, ], }, { - code: 'enum Foo { ONE, TWO }; let f: Foo; (0 === f);', + code: ` +enum Foo { ONE, TWO }; +let f: Foo; +(0 === f);`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 37, + line: 4, + column: 2, }, ], }, { - code: 'enum Foo { ONE, TWO }; let f: Foo; f = 0;', + code: ` +enum Foo { ONE, TWO }; +let f: Foo; +f = 0;`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 40, + line: 4, + column: 5, }, ], }, { - code: 'enum Foo { ONE, TWO }; let f: Foo = 0;', + code: ` +enum Foo { ONE, TWO }; +let f: Foo = 0;`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 37, + line: 3, + column: 14, }, ], }, { - code: 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; (f === 0);', + code: ` +enum Foo { ONE = 1, TWO = 2 }; +let f: Foo; +(f === 0);`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 51, + line: 4, + column: 8, }, ], }, { - code: 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; (0 === f);', + code: ` +enum Foo { ONE = 1, TWO = 2 }; +let f: Foo; +(0 === f);`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 45, + line: 4, + column: 2, }, ], }, { - code: 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo; f = 0;', + code: ` +enum Foo { ONE = 1, TWO = 2 }; +let f: Foo; +f = 0;`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 48, + line: 4, + column: 5, }, ], }, { - code: 'enum Foo { ONE = 1, TWO = 2 }; let f: Foo = 0;', + code: ` +enum Foo { ONE = 1, TWO = 2 }; +let f: Foo = 0;`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 45, + line: 3, + column: 14, }, ], }, { - code: 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; (f === "ONE");', + code: ` +enum Foo { ONE = "ONE", TWO = "TWO" }; +let f: Foo; +(f === "ONE");`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 59, + line: 4, + column: 8, }, ], }, { - code: 'enum Foo { ONE = "ONE", TWO = "TWO" }; let f: Foo; ("ONE" === f);', + code: ` +enum Foo { ONE = "ONE", TWO = "TWO" }; +let f: Foo; +("ONE" === f);`, errors: [ { messageId: 'noLiterals', - line: 1, - column: 53, + line: 4, + column: 2, }, ], },