From 85b7de58529610fd5d07b088403b954932dc4e66 Mon Sep 17 00:00:00 2001 From: Scott O'Hara Date: Tue, 21 Sep 2021 10:23:54 +1000 Subject: [PATCH] feat(eslint-plugin): [no-type-alias]: add allowGenerics option (#3865) --- .../eslint-plugin/docs/rules/no-type-alias.md | 23 ++++++++ .../eslint-plugin/src/rules/no-type-alias.ts | 17 ++++++ .../src/rules/prefer-regexp-exec.ts | 27 +++++++-- .../tests/rules/no-type-alias.test.ts | 17 ++++++ .../tests/rules/prefer-regexp-exec.test.ts | 58 ++++++++++++++++++- 5 files changed, 137 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-type-alias.md b/packages/eslint-plugin/docs/rules/no-type-alias.md index 5b720b2f331d..a3b61f1da446 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"`) +- `allowGenerics` set to `"always"` will allow you to use type aliases with generics (Defaults to `"never"`) ### `allowAliases` @@ -555,6 +556,28 @@ type Foo = [number] & [number, number]; type Foo = [string] | [number]; ``` +### `allowGenerics` + +This applies to generic types, including TypeScript provided global utility types (`type Foo = Record`). + +The setting accepts the following options: + +- `"always"` or `"never"` to active or deactivate the feature. + +Examples of **correct** code for the `{ "allowGenerics": "always" }` options: + +```ts +type Foo = Bar; + +type Foo = Record; + +type Foo = Readonly; + +type Foo = Partial; + +type Foo = Omit; +``` + ## 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 3e93a995e64d..6cd149dd77c7 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; + allowGenerics?: 'always' | 'never'; }, ]; type MessageIds = 'noTypeAlias' | 'noCompositionAlias'; @@ -79,6 +80,9 @@ export default util.createRule({ allowTupleTypes: { enum: enumValues, }, + allowGenerics: { + enum: ['always', 'never'], + }, }, additionalProperties: false, }, @@ -93,6 +97,7 @@ export default util.createRule({ allowLiterals: 'never', allowMappedTypes: 'never', allowTupleTypes: 'never', + allowGenerics: 'never', }, ], create( @@ -106,6 +111,7 @@ export default util.createRule({ allowLiterals, allowMappedTypes, allowTupleTypes, + allowGenerics, }, ], ) { @@ -203,6 +209,13 @@ export default util.createRule({ return false; }; + const isValidGeneric = (type: TypeWithLabel): boolean => { + return ( + type.node.type === AST_NODE_TYPES.TSTypeReference && + type.node.typeParameters !== undefined + ); + }; + const checkAndReport = ( optionValue: Values, isTopLevel: boolean, @@ -260,6 +273,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/src/rules/prefer-regexp-exec.ts b/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts index 0d6693715bac..ebb92364e464 100644 --- a/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts +++ b/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts @@ -75,13 +75,31 @@ export default createRule({ return result; } + function isLikelyToContainGlobalFlag( + node: TSESTree.CallExpressionArgument, + ): boolean { + if ( + node.type === AST_NODE_TYPES.CallExpression || + node.type === AST_NODE_TYPES.NewExpression + ) { + const [, flags] = node.arguments; + return ( + flags.type === AST_NODE_TYPES.Literal && + typeof flags.value === 'string' && + flags.value.includes('g') + ); + } + + return node.type === AST_NODE_TYPES.Identifier; + } + return { "CallExpression[arguments.length=1] > MemberExpression.callee[property.name='match'][computed=false]"( memberNode: TSESTree.MemberExpression, ): void { const objectNode = memberNode.object; const callNode = memberNode.parent as TSESTree.CallExpression; - const argumentNode = callNode.arguments[0]; + const [argumentNode] = callNode.arguments; const argumentValue = getStaticValue(argumentNode, globalScope); if ( @@ -96,9 +114,10 @@ export default createRule({ // Don't report regular expressions with global flag. if ( - argumentValue && - argumentValue.value instanceof RegExp && - argumentValue.value.flags.includes('g') + (!argumentValue && isLikelyToContainGlobalFlag(argumentNode)) || + (argumentValue && + argumentValue.value instanceof RegExp && + argumentValue.value.flags.includes('g')) ) { return; } 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 1f44675a8515..06d06f7672ec 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: [{ allowGenerics: 'always' }], + }, ], invalid: [ { @@ -3329,5 +3333,18 @@ type Foo = { }, ], }, + { + code: 'type Foo = Record;', + errors: [ + { + messageId: 'noTypeAlias', + data: { + alias: 'generics', + }, + line: 1, + column: 12, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts b/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts index b45935108071..8a5bdfd099db 100644 --- a/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts @@ -1,5 +1,5 @@ import rule from '../../src/rules/prefer-regexp-exec'; -import { RuleTester, getFixturesRootDir } from '../RuleTester'; +import { getFixturesRootDir, RuleTester } from '../RuleTester'; const rootPath = getFixturesRootDir(); @@ -56,6 +56,24 @@ const matchers = [{ match: (s: RegExp) => false }]; const file = ''; matchers.some(matcher => !!file.match(matcher)); `, + // https://github.com/typescript-eslint/typescript-eslint/issues/3477 + ` +function test(pattern: string) { + 'hello hello'.match(RegExp(pattern, 'g'))?.reduce(() => []); +} + `, + // https://github.com/typescript-eslint/typescript-eslint/issues/3477 + ` +function test(pattern: string) { + 'hello hello'.match(new RegExp(pattern, 'gi'))?.reduce(() => []); +} + `, + // https://github.com/typescript-eslint/typescript-eslint/issues/3477 + ` +const matchCount = (str: string, re: RegExp) => { + return (str.match(re) || []).length; +}; + `, ], invalid: [ { @@ -174,6 +192,44 @@ function f(s: T) { output: ` function f(s: T) { /thing/.exec(s); +} + `, + }, + { + code: ` +const text = 'something'; +const search = new RegExp('test', ''); +text.match(search); + `, + errors: [ + { + messageId: 'regExpExecOverStringMatch', + line: 4, + column: 6, + }, + ], + output: ` +const text = 'something'; +const search = new RegExp('test', ''); +search.exec(text); + `, + }, + { + code: ` +function test(pattern: string) { + 'check'.match(new RegExp(pattern, undefined)); +} + `, + errors: [ + { + messageId: 'regExpExecOverStringMatch', + line: 3, + column: 11, + }, + ], + output: ` +function test(pattern: string) { + new RegExp(pattern, undefined).exec('check'); } `, },