From 054e62ecb277690471709567c831d949c0bffb3e Mon Sep 17 00:00:00 2001 From: Ulrich Buchgraber Date: Thu, 26 Mar 2020 14:40:47 +0100 Subject: [PATCH 1/8] feat(eslint-plugin): [restrict-template-expressions] add support for intersection types (fixes #1797) --- .../rules/restrict-template-expressions.ts | 138 ++++++++---------- .../restrict-template-expressions.test.ts | 21 +++ 2 files changed, 82 insertions(+), 77 deletions(-) diff --git a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts index 6ff1627daf5..31d873d038b 100644 --- a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts +++ b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts @@ -44,29 +44,36 @@ export default util.createRule({ const service = util.getParserServices(context); const typeChecker = service.program.getTypeChecker(); - type BaseType = - | 'string' - | 'number' - | 'bigint' - | 'boolean' - | 'null' - | 'undefined' - | 'other'; - - const allowedTypes: BaseType[] = [ - 'string', - ...(options.allowNumber ? (['number', 'bigint'] as const) : []), - ...(options.allowBoolean ? (['boolean'] as const) : []), - ...(options.allowNullable ? (['null', 'undefined'] as const) : []), - ]; - - function isAllowedType(types: BaseType[]): boolean { - for (const type of types) { - if (!allowedTypes.includes(type)) { - return false; - } + function isUnderlyingTypePrimitive(type: ts.Type): boolean { + if (util.isTypeFlagSet(type, ts.TypeFlags.StringLike)) { + return true; + } + + if ( + util.isTypeFlagSet( + type, + ts.TypeFlags.NumberLike | ts.TypeFlags.BigIntLike, + ) && + options.allowNumber + ) { + return true; } - return true; + + if ( + util.isTypeFlagSet(type, ts.TypeFlags.BooleanLike) && + options.allowBoolean + ) { + return true; + } + + if ( + util.isTypeFlagSet(type, ts.TypeFlags.Null | ts.TypeFlags.Undefined) && + options.allowNullable + ) { + return true; + } + + return false; } return { @@ -76,11 +83,15 @@ export default util.createRule({ return; } - for (const expr of node.expressions) { - const type = getNodeType(expr); - if (!isAllowedType(type)) { + for (const expression of node.expressions) { + if ( + !isUnderlyingExpressionTypeConfirmingTo( + expression, + isUnderlyingTypePrimitive, + ) + ) { context.report({ - node: expr, + node: expression, messageId: 'invalidType', }); } @@ -88,63 +99,36 @@ export default util.createRule({ }, }; - /** - * Helper function to get base type of node - * @param node the node to be evaluated. - */ - function getNodeType(node: TSESTree.Expression): BaseType[] { - const tsNode = service.esTreeNodeToTSNodeMap.get(node); - const type = typeChecker.getTypeAtLocation(tsNode); + function isUnderlyingExpressionTypeConfirmingTo( + expression: TSESTree.Expression, + predicate: (underlyingType: ts.Type) => boolean, + ): boolean { + const expressionType = getExpressionNodeType(expression); - return getBaseType(type); - } + return rec( + // "Extracts" generic constraint, indexed access and conditional types: + typeChecker.getBaseConstraintOfType(expressionType) ?? expressionType, + ); - function getBaseType(type: ts.Type): BaseType[] { - const constraint = type.getConstraint(); - if ( - constraint && - // for generic types with union constraints, it will return itself - constraint !== type - ) { - return getBaseType(constraint); - } - - if (type.isStringLiteral()) { - return ['string']; - } - if (type.isNumberLiteral()) { - return ['number']; - } - if (type.flags & ts.TypeFlags.BigIntLiteral) { - return ['bigint']; - } - if (type.flags & ts.TypeFlags.BooleanLiteral) { - return ['boolean']; - } - if (type.flags & ts.TypeFlags.Null) { - return ['null']; - } - if (type.flags & ts.TypeFlags.Undefined) { - return ['undefined']; - } + function rec(type: ts.Type): boolean { + if (type.isUnion()) { + return type.types.every(rec); + } - if (type.isUnion()) { - return type.types - .map(getBaseType) - .reduce((all, array) => [...all, ...array], []); - } + if (type.isIntersection()) { + return type.types.some(rec); + } - const stringType = typeChecker.typeToString(type); - if ( - stringType === 'string' || - stringType === 'number' || - stringType === 'bigint' || - stringType === 'boolean' - ) { - return [stringType]; + return predicate(type); } + } - return ['other']; + /** + * Helper function to extract the TS type of an TSESTree expression. + */ + function getExpressionNodeType(node: TSESTree.Expression): ts.Type { + const tsNode = service.esTreeNodeToTSNodeMap.get(node); + return typeChecker.getTypeAtLocation(tsNode); } }, }); diff --git a/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts b/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts index cdf59b0c819..e56d02adb47 100644 --- a/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts @@ -30,6 +30,12 @@ ruleTester.run('restrict-template-expressions', rule, { return \`arg = \${arg}\`; } `, + // Base case - intersection type + ` + function test(arg: T) { + return \`arg = \${arg}\`; + } + `, // Base case - don't check tagged templates ` tag\`arg = \${null}\`; @@ -68,6 +74,14 @@ ruleTester.run('restrict-template-expressions', rule, { } `, }, + { + options: [{ allowNumber: true }], + code: ` + function test(arg: T) { + return \`arg = \${arg}\`; + } + `, + }, { options: [{ allowNumber: true }], code: ` @@ -199,6 +213,13 @@ ruleTester.run('restrict-template-expressions', rule, { `, errors: [{ messageId: 'invalidType', line: 3, column: 30 }], }, + { + code: ` + declare const arg: { a: string } & { b: string }; + const msg = \`arg = \${arg}\`; + `, + errors: [{ messageId: 'invalidType', line: 3, column: 30 }], + }, { options: [{ allowNumber: true, allowBoolean: true, allowNullable: true }], code: ` From df0ad14670f76e180aa33b1a6149aa52f5a5fe8f Mon Sep 17 00:00:00 2001 From: Ulrich Buchgraber Date: Tue, 14 Apr 2020 16:27:40 +0200 Subject: [PATCH 2/8] Manually merge "allowAny" option --- .../eslint-plugin/src/rules/restrict-template-expressions.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts index 0aabe299610..8d92a003c5e 100644 --- a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts +++ b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts @@ -75,6 +75,10 @@ export default util.createRule({ return true; } + if (util.isTypeFlagSet(type, ts.TypeFlags.Any) && options.allowAny) { + return true; + } + return false; } From f2fab58d89c1fc722f3caf8a3398fdbebaa18b8f Mon Sep 17 00:00:00 2001 From: Ulrich Buchgraber Date: Tue, 14 Apr 2020 16:43:54 +0200 Subject: [PATCH 3/8] Refactor to `util.getConstrainedTypeAtLocation()` --- .../src/rules/restrict-template-expressions.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts index 8d92a003c5e..88c17ec8560 100644 --- a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts +++ b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts @@ -109,12 +109,7 @@ export default util.createRule({ expression: TSESTree.Expression, predicate: (underlyingType: ts.Type) => boolean, ): boolean { - const expressionType = getExpressionNodeType(expression); - - return rec( - // "Extracts" generic constraint, indexed access and conditional types: - typeChecker.getBaseConstraintOfType(expressionType) ?? expressionType, - ); + return rec(getExpressionNodeType(expression)); function rec(type: ts.Type): boolean { if (type.isUnion()) { @@ -134,7 +129,7 @@ export default util.createRule({ */ function getExpressionNodeType(node: TSESTree.Expression): ts.Type { const tsNode = service.esTreeNodeToTSNodeMap.get(node); - return typeChecker.getTypeAtLocation(tsNode); + return util.getConstrainedTypeAtLocation(typeChecker, tsNode); } }, }); From 9ebb323f0800b13edbe334b80eba0b88f3c64d50 Mon Sep 17 00:00:00 2001 From: Ulrich Buchgraber Date: Tue, 14 Apr 2020 16:46:25 +0200 Subject: [PATCH 4/8] Move the options check before `isTypeFlagSet()` check --- .../rules/restrict-template-expressions.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts index 88c17ec8560..b1fbda054ee 100644 --- a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts +++ b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts @@ -52,30 +52,30 @@ export default util.createRule({ } if ( - util.isTypeFlagSet( - type, - ts.TypeFlags.NumberLike | ts.TypeFlags.BigIntLike, - ) && - options.allowNumber + options.allowNullable && + util.isTypeFlagSet(type, ts.TypeFlags.Null | ts.TypeFlags.Undefined) ) { return true; } if ( - util.isTypeFlagSet(type, ts.TypeFlags.BooleanLike) && - options.allowBoolean + options.allowNumber && + util.isTypeFlagSet( + type, + ts.TypeFlags.NumberLike | ts.TypeFlags.BigIntLike, + ) ) { return true; } if ( - util.isTypeFlagSet(type, ts.TypeFlags.Null | ts.TypeFlags.Undefined) && - options.allowNullable + options.allowBoolean && + util.isTypeFlagSet(type, ts.TypeFlags.BooleanLike) ) { return true; } - if (util.isTypeFlagSet(type, ts.TypeFlags.Any) && options.allowAny) { + if (options.allowAny && util.isTypeFlagSet(type, ts.TypeFlags.Any)) { return true; } From 8d74671a0fc84de0b10f3be6c8e82785f50cd105 Mon Sep 17 00:00:00 2001 From: Ulrich Buchgraber Date: Tue, 14 Apr 2020 16:50:47 +0200 Subject: [PATCH 5/8] Bring the options order in sync and add missing `allowAny` in docs --- .../rules/restrict-template-expressions.md | 2 ++ .../rules/restrict-template-expressions.ts | 20 +++++++++---------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/restrict-template-expressions.md b/packages/eslint-plugin/docs/rules/restrict-template-expressions.md index 4c52e74445f..95064b2a147 100644 --- a/packages/eslint-plugin/docs/rules/restrict-template-expressions.md +++ b/packages/eslint-plugin/docs/rules/restrict-template-expressions.md @@ -28,6 +28,8 @@ type Options = { allowNumber?: boolean; // if true, also allow boolean type in template expressions allowBoolean?: boolean; + // if true, also allow any in template expressions + allowAny?: boolean; // if true, also allow null and undefined in template expressions allowNullable?: boolean; }; diff --git a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts index b1fbda054ee..90dd363f820 100644 --- a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts +++ b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts @@ -7,10 +7,10 @@ import * as util from '../util'; type Options = [ { - allowNullable?: boolean; allowNumber?: boolean; allowBoolean?: boolean; allowAny?: boolean; + allowNullable?: boolean; }, ]; @@ -33,10 +33,10 @@ export default util.createRule({ { type: 'object', properties: { - allowAny: { type: 'boolean' }, + allowNumber: { type: 'boolean' }, allowBoolean: { type: 'boolean' }, + allowAny: { type: 'boolean' }, allowNullable: { type: 'boolean' }, - allowNumber: { type: 'boolean' }, }, }, ], @@ -51,13 +51,6 @@ export default util.createRule({ return true; } - if ( - options.allowNullable && - util.isTypeFlagSet(type, ts.TypeFlags.Null | ts.TypeFlags.Undefined) - ) { - return true; - } - if ( options.allowNumber && util.isTypeFlagSet( @@ -79,6 +72,13 @@ export default util.createRule({ return true; } + if ( + options.allowNullable && + util.isTypeFlagSet(type, ts.TypeFlags.Null | ts.TypeFlags.Undefined) + ) { + return true; + } + return false; } From 8df74f44bc99a7cdd8060957ccd96b53b321ce64 Mon Sep 17 00:00:00 2001 From: Ulrich Buchgraber Date: Tue, 14 Apr 2020 16:53:57 +0200 Subject: [PATCH 6/8] Add intersection type example --- .../eslint-plugin/docs/rules/restrict-template-expressions.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/restrict-template-expressions.md b/packages/eslint-plugin/docs/rules/restrict-template-expressions.md index 95064b2a147..c42bf971b6c 100644 --- a/packages/eslint-plugin/docs/rules/restrict-template-expressions.md +++ b/packages/eslint-plugin/docs/rules/restrict-template-expressions.md @@ -6,6 +6,9 @@ Examples of **correct** code: const arg = 'foo'; const msg1 = `arg = ${arg}`; const msg2 = `arg = ${arg || 'default'}`; + +const stringWithKind: string & { _kind?: 'MyString' } = 'foo'; +const msg1 = `arg = ${stringWithKind}`; ``` Examples of **incorrect** code: From dfe2c64cc462cd077d74e3fe4e1697ea7bd7fe7c Mon Sep 17 00:00:00 2001 From: Ulrich Buchgraber Date: Tue, 14 Apr 2020 17:08:14 +0200 Subject: [PATCH 7/8] Fix linter errors --- .../tests/rules/restrict-template-expressions.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts b/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts index 3aa36abb249..2f5900f3d2a 100644 --- a/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts @@ -32,7 +32,7 @@ ruleTester.run('restrict-template-expressions', rule, { `, // Base case - intersection type ` - function test(arg: T) { + function test(arg: T) { return \`arg = \${arg}\`; } `, @@ -77,10 +77,10 @@ ruleTester.run('restrict-template-expressions', rule, { { options: [{ allowNumber: true }], code: ` - function test(arg: T) { - return \`arg = \${arg}\`; - } - `, + function test(arg: T) { + return \`arg = \${arg}\`; + } + `, }, { options: [{ allowNumber: true }], From e46717cc1a9314b0a1461b549de2e12ac6a0e62c Mon Sep 17 00:00:00 2001 From: Ulrich Buchgraber Date: Tue, 14 Apr 2020 17:26:57 +0200 Subject: [PATCH 8/8] Improve / fix example --- .../eslint-plugin/docs/rules/restrict-template-expressions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/restrict-template-expressions.md b/packages/eslint-plugin/docs/rules/restrict-template-expressions.md index c42bf971b6c..816a0b0f9d6 100644 --- a/packages/eslint-plugin/docs/rules/restrict-template-expressions.md +++ b/packages/eslint-plugin/docs/rules/restrict-template-expressions.md @@ -7,8 +7,8 @@ const arg = 'foo'; const msg1 = `arg = ${arg}`; const msg2 = `arg = ${arg || 'default'}`; -const stringWithKind: string & { _kind?: 'MyString' } = 'foo'; -const msg1 = `arg = ${stringWithKind}`; +const stringWithKindProp: string & { _kind?: 'MyString' } = 'foo'; +const msg3 = `stringWithKindProp = ${stringWithKindProp}`; ``` Examples of **incorrect** code: