From 37867db6255f1d0cac9f4800c3b1c4470ab2fe3a Mon Sep 17 00:00:00 2001 From: Scott O'Hara Date: Fri, 10 Sep 2021 09:10:10 +1000 Subject: [PATCH 1/4] feat(eslint-plugin): [no-type-alias]: add allowedAliasNames option (Fixes #3784) --- .../eslint-plugin/docs/rules/no-type-alias.md | 39 +++++++++++++++++++ .../eslint-plugin/src/rules/no-type-alias.ts | 31 ++++++++++++++- .../tests/rules/no-type-alias.test.ts | 18 +++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-type-alias.md b/packages/eslint-plugin/docs/rules/no-type-alias.md index 5b720b2f331..3d8ad846ccb 100644 --- a/packages/eslint-plugin/docs/rules/no-type-alias.md +++ b/packages/eslint-plugin/docs/rules/no-type-alias.md @@ -89,6 +89,7 @@ or more of the following you may pass an object with the options set as follows: - `allowLiterals` set to `"always"` will allow you to use type aliases with literal objects (Defaults to `"never"`) - `allowMappedTypes` set to `"always"` will allow you to use type aliases as mapping tools (Defaults to `"never"`) - `allowTupleTypes` set to `"always"` will allow you to use type aliases with tuples (Defaults to `"never"`) +- `allowedAliasNames` allows you to specify a list of alias names that the rule should ignore (Defaults to `[]`) ### `allowAliases` @@ -555,6 +556,44 @@ type Foo = [number] & [number, number]; type Foo = [string] | [number]; ``` +### `allowedAliasNames` + +A string array of alias names that the rule should ignore. + +Use this option to avoid conflicts with other rules, for example: + +```ts +/* eslint @typescript-eslint/consistent-indexed-object-style: "error" */ + +// error +interface { [key: string]: number }; + +// OK +type Foo = Record; +``` + +```ts +/* eslint @typescript-eslint/no-type-alias: "error" */ + +// error +type Foo = Record; +``` + +In the example above, the type alias is necessary to pass one rule, but triggers an error in another. +To avoid a conflict such as this, you can tell the rule to ignore specific aliases such as `Record`. + +Examples of **correct** code for the `{ "allowAliases": "never", "allowedAliasNames": ["Record"] }` options: + +```ts +type Foo = Record; +``` + +Examples of **incorrect** code for the `{ "allowAliases": "never", "allowedAliasNames": [] }` options: + +```ts +type Foo = Record; +``` + ## When Not To Use It When you can't express some shape with an interface or you need to use a union, tuple type, diff --git a/packages/eslint-plugin/src/rules/no-type-alias.ts b/packages/eslint-plugin/src/rules/no-type-alias.ts index 3e93a995e64..cbea9002fcb 100644 --- a/packages/eslint-plugin/src/rules/no-type-alias.ts +++ b/packages/eslint-plugin/src/rules/no-type-alias.ts @@ -27,6 +27,7 @@ type Options = [ allowLiterals?: Values; allowMappedTypes?: Values; allowTupleTypes?: Values; + allowedAliasNames?: string[]; }, ]; type MessageIds = 'noTypeAlias' | 'noCompositionAlias'; @@ -79,6 +80,12 @@ export default util.createRule({ allowTupleTypes: { enum: enumValues, }, + allowedAliasNames: { + type: 'array', + items: { + type: 'string', + }, + }, }, additionalProperties: false, }, @@ -93,6 +100,7 @@ export default util.createRule({ allowLiterals: 'never', allowMappedTypes: 'never', allowTupleTypes: 'never', + allowedAliasNames: [], }, ], create( @@ -106,6 +114,7 @@ export default util.createRule({ allowLiterals, allowMappedTypes, allowTupleTypes, + allowedAliasNames, }, ], ) { @@ -150,6 +159,19 @@ export default util.createRule({ ); } + /** + * Determines if the type is a by the allowed flags. + * @param node the kind of type alias being validated + */ + function isAllowedAliasName(node: TSESTree.Node): boolean { + return ( + node.type === AST_NODE_TYPES.TSTypeReference && + node.typeName.type === AST_NODE_TYPES.Identifier && + allowedAliasNames !== undefined && + allowedAliasNames.includes(node.typeName.name) + ); + } + /** * Gets the message to be displayed based on the node type and whether the node is a top level declaration. * @param node the location @@ -210,8 +232,13 @@ export default util.createRule({ label: string, ): void => { if ( - optionValue === 'never' || - !isSupportedComposition(isTopLevel, type.compositionType, optionValue) + (optionValue === 'never' || + !isSupportedComposition( + isTopLevel, + type.compositionType, + optionValue, + )) && + !isAllowedAliasName(type.node) ) { reportError(type.node, type.compositionType, isTopLevel, label); } diff --git a/packages/eslint-plugin/tests/rules/no-type-alias.test.ts b/packages/eslint-plugin/tests/rules/no-type-alias.test.ts index 1f44675a851..1a51a1da2e9 100644 --- a/packages/eslint-plugin/tests/rules/no-type-alias.test.ts +++ b/packages/eslint-plugin/tests/rules/no-type-alias.test.ts @@ -481,6 +481,10 @@ type KeyNames = keyof typeof SCALARS; code: 'type Foo = new (bar: number) => string | null;', options: [{ allowConstructors: 'always' }], }, + { + code: 'type Foo = Record;', + options: [{ allowedAliasNames: ['Record'] }], + }, ], invalid: [ { @@ -3329,5 +3333,19 @@ type Foo = { }, ], }, + { + code: 'type Foo = Record;', + options: [{ allowedAliasNames: ['Other'] }], + errors: [ + { + messageId: 'noTypeAlias', + data: { + alias: 'aliases', + }, + line: 1, + column: 12, + }, + ], + }, ], }); From e3244068fa5fda1184d8ffad7654f0231363bbc1 Mon Sep 17 00:00:00 2001 From: Scott O'Hara Date: Fri, 10 Sep 2021 09:32:15 +1000 Subject: [PATCH 2/4] Fix JSDoc comment --- packages/eslint-plugin/src/rules/no-type-alias.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-type-alias.ts b/packages/eslint-plugin/src/rules/no-type-alias.ts index cbea9002fcb..cfe78b0fea8 100644 --- a/packages/eslint-plugin/src/rules/no-type-alias.ts +++ b/packages/eslint-plugin/src/rules/no-type-alias.ts @@ -160,7 +160,7 @@ export default util.createRule({ } /** - * Determines if the type is a by the allowed flags. + * Determines if the alias name is in the list of allowed names. * @param node the kind of type alias being validated */ function isAllowedAliasName(node: TSESTree.Node): boolean { From 3d46d0b6369531290d9195cc1c1ee53b7dcf3ae9 Mon Sep 17 00:00:00 2001 From: Scott O'Hara Date: Mon, 13 Sep 2021 12:57:41 +1000 Subject: [PATCH 3/4] Replaced allowedAliasNames option with allowGenerics --- .../eslint-plugin/docs/rules/no-type-alias.md | 36 ++++--------- .../eslint-plugin/src/rules/no-type-alias.ts | 50 ++++++++----------- .../tests/rules/no-type-alias.test.ts | 5 +- 3 files changed, 34 insertions(+), 57 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-type-alias.md b/packages/eslint-plugin/docs/rules/no-type-alias.md index 3d8ad846ccb..a3b61f1da44 100644 --- a/packages/eslint-plugin/docs/rules/no-type-alias.md +++ b/packages/eslint-plugin/docs/rules/no-type-alias.md @@ -89,7 +89,7 @@ or more of the following you may pass an object with the options set as follows: - `allowLiterals` set to `"always"` will allow you to use type aliases with literal objects (Defaults to `"never"`) - `allowMappedTypes` set to `"always"` will allow you to use type aliases as mapping tools (Defaults to `"never"`) - `allowTupleTypes` set to `"always"` will allow you to use type aliases with tuples (Defaults to `"never"`) -- `allowedAliasNames` allows you to specify a list of alias names that the rule should ignore (Defaults to `[]`) +- `allowGenerics` set to `"always"` will allow you to use type aliases with generics (Defaults to `"never"`) ### `allowAliases` @@ -556,42 +556,26 @@ type Foo = [number] & [number, number]; type Foo = [string] | [number]; ``` -### `allowedAliasNames` +### `allowGenerics` -A string array of alias names that the rule should ignore. +This applies to generic types, including TypeScript provided global utility types (`type Foo = Record`). -Use this option to avoid conflicts with other rules, for example: +The setting accepts the following options: -```ts -/* eslint @typescript-eslint/consistent-indexed-object-style: "error" */ +- `"always"` or `"never"` to active or deactivate the feature. -// error -interface { [key: string]: number }; - -// OK -type Foo = Record; -``` +Examples of **correct** code for the `{ "allowGenerics": "always" }` options: ```ts -/* eslint @typescript-eslint/no-type-alias: "error" */ +type Foo = Bar; -// error type Foo = Record; -``` -In the example above, the type alias is necessary to pass one rule, but triggers an error in another. -To avoid a conflict such as this, you can tell the rule to ignore specific aliases such as `Record`. +type Foo = Readonly; -Examples of **correct** code for the `{ "allowAliases": "never", "allowedAliasNames": ["Record"] }` options: +type Foo = Partial; -```ts -type Foo = Record; -``` - -Examples of **incorrect** code for the `{ "allowAliases": "never", "allowedAliasNames": [] }` options: - -```ts -type Foo = Record; +type Foo = Omit; ``` ## When Not To Use It diff --git a/packages/eslint-plugin/src/rules/no-type-alias.ts b/packages/eslint-plugin/src/rules/no-type-alias.ts index cfe78b0fea8..9502297a8c2 100644 --- a/packages/eslint-plugin/src/rules/no-type-alias.ts +++ b/packages/eslint-plugin/src/rules/no-type-alias.ts @@ -27,7 +27,7 @@ type Options = [ allowLiterals?: Values; allowMappedTypes?: Values; allowTupleTypes?: Values; - allowedAliasNames?: string[]; + allowGenerics?: 'always' | 'never'; }, ]; type MessageIds = 'noTypeAlias' | 'noCompositionAlias'; @@ -80,11 +80,8 @@ export default util.createRule({ allowTupleTypes: { enum: enumValues, }, - allowedAliasNames: { - type: 'array', - items: { - type: 'string', - }, + allowGenerics: { + enum: ['always', 'never'], }, }, additionalProperties: false, @@ -100,7 +97,7 @@ export default util.createRule({ allowLiterals: 'never', allowMappedTypes: 'never', allowTupleTypes: 'never', - allowedAliasNames: [], + allowGenerics: 'never', }, ], create( @@ -114,7 +111,7 @@ export default util.createRule({ allowLiterals, allowMappedTypes, allowTupleTypes, - allowedAliasNames, + allowGenerics, }, ], ) { @@ -159,19 +156,6 @@ export default util.createRule({ ); } - /** - * Determines if the alias name is in the list of allowed names. - * @param node the kind of type alias being validated - */ - function isAllowedAliasName(node: TSESTree.Node): boolean { - return ( - node.type === AST_NODE_TYPES.TSTypeReference && - node.typeName.type === AST_NODE_TYPES.Identifier && - allowedAliasNames !== undefined && - allowedAliasNames.includes(node.typeName.name) - ); - } - /** * Gets the message to be displayed based on the node type and whether the node is a top level declaration. * @param node the location @@ -225,6 +209,17 @@ export default util.createRule({ return false; }; + /** + * Determines if the alias name is in the list of allowed names. + * @param node the kind of type alias being validated + */ + const isValidGeneric = (type: TypeWithLabel): boolean => { + return ( + type.node.type === AST_NODE_TYPES.TSTypeReference && + type.node.typeParameters !== undefined + ); + }; + const checkAndReport = ( optionValue: Values, isTopLevel: boolean, @@ -232,13 +227,8 @@ export default util.createRule({ label: string, ): void => { if ( - (optionValue === 'never' || - !isSupportedComposition( - isTopLevel, - type.compositionType, - optionValue, - )) && - !isAllowedAliasName(type.node) + optionValue === 'never' || + !isSupportedComposition(isTopLevel, type.compositionType, optionValue) ) { reportError(type.node, type.compositionType, isTopLevel, label); } @@ -287,6 +277,10 @@ export default util.createRule({ } else if (isValidTupleType(type)) { // tuple types checkAndReport(allowTupleTypes!, isTopLevel, type, 'Tuple Types'); + } else if (isValidGeneric(type)) { + if (allowGenerics === 'never') { + reportError(type.node, type.compositionType, isTopLevel, 'Generics'); + } } else if ( // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum type.node.type.endsWith('Keyword') || diff --git a/packages/eslint-plugin/tests/rules/no-type-alias.test.ts b/packages/eslint-plugin/tests/rules/no-type-alias.test.ts index 1a51a1da2e9..06d06f7672e 100644 --- a/packages/eslint-plugin/tests/rules/no-type-alias.test.ts +++ b/packages/eslint-plugin/tests/rules/no-type-alias.test.ts @@ -483,7 +483,7 @@ type KeyNames = keyof typeof SCALARS; }, { code: 'type Foo = Record;', - options: [{ allowedAliasNames: ['Record'] }], + options: [{ allowGenerics: 'always' }], }, ], invalid: [ @@ -3335,12 +3335,11 @@ type Foo = { }, { code: 'type Foo = Record;', - options: [{ allowedAliasNames: ['Other'] }], errors: [ { messageId: 'noTypeAlias', data: { - alias: 'aliases', + alias: 'generics', }, line: 1, column: 12, From e8622291aed0512988dc9d9268dbb035097a752f Mon Sep 17 00:00:00 2001 From: Scott O'Hara Date: Mon, 13 Sep 2021 13:07:35 +1000 Subject: [PATCH 4/4] Remove redundant JSDoc comment --- packages/eslint-plugin/src/rules/no-type-alias.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-type-alias.ts b/packages/eslint-plugin/src/rules/no-type-alias.ts index 9502297a8c2..6cd149dd77c 100644 --- a/packages/eslint-plugin/src/rules/no-type-alias.ts +++ b/packages/eslint-plugin/src/rules/no-type-alias.ts @@ -209,10 +209,6 @@ export default util.createRule({ return false; }; - /** - * Determines if the alias name is in the list of allowed names. - * @param node the kind of type alias being validated - */ const isValidGeneric = (type: TypeWithLabel): boolean => { return ( type.node.type === AST_NODE_TYPES.TSTypeReference &&