feat(eslint-plugin): [restrict-template-expressions] add allowArray…

… option (#8389)
typescript-eslint · Mar 16, 2024 · e4b1672 · e4b1672
1 parent b5e5bda
commit e4b1672
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 110 deletions.
diff --git a/packages/eslint-plugin/docs/rules/restrict-template-expressions.md b/packages/eslint-plugin/docs/rules/restrict-template-expressions.md
@@ -12,10 +12,10 @@ This rule reports on values used in a template literal string that aren't string
 
 :::note
 
-This rule intentionally does not allow objects with a custom `toString()` method to be used in template literals, because the stringification result may not be user-friendly.
+The default settings of this rule intentionally do not allow objects with a custom `toString()` method to be used in template literals, because the stringification result may not be user-friendly.
 
 For example, arrays have a custom [`toString()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toString) method, which only calls `join()` internally, which joins the array elements with commas. This means that (1) array elements are not necessarily stringified to useful results (2) the commas don't have spaces after them, making the result not user-friendly. The best way to format arrays is to use [`Intl.ListFormat`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/ListFormat), which even supports adding the "and" conjunction where necessary.
-You must explicitly call `object.toString()` if you want to use this object in a template literal.
+You must explicitly call `object.toString()` if you want to use this object in a template literal, or turn on the `allowArray` option to specifically allow arrays.
 The [`no-base-to-string`](./no-base-to-string.md) rule can be used to guard this case against producing `"[object Object]"` by accident.
 
 :::
@@ -111,6 +111,15 @@ const arg = 'something';
 const msg1 = typeof arg === 'string' ? arg : `arg = ${arg}`;
 ```
 
+### `allowArray`
+
+Examples of additional **correct** code for this rule with `{ allowArray: true }`:
+
+```ts option='{ "allowArray": true }' showPlaygroundButton
+const arg = ['foo', 'bar'];
+const msg1 = `arg = ${arg}`;
+```
+
 ## When Not To Use It
 
 If you're not worried about incorrectly stringifying non-string values in template literals, then you likely don't need this rule.

diff --git a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts
@@ -1,6 +1,7 @@
 import type { TSESTree } from '@typescript-eslint/utils';
 import { AST_NODE_TYPES } from '@typescript-eslint/utils';
-import * as ts from 'typescript';
+import type { Type, TypeChecker } from 'typescript';
+import { TypeFlags } from 'typescript';
 
 import {
   createRule,
@@ -12,15 +13,44 @@ import {
   isTypeNeverType,
 } from '../util';
 
+type OptionTester = (
+  type: Type,
+  checker: TypeChecker,
+  recursivelyCheckType: (type: Type) => boolean,
+) => boolean;
+
+const testTypeFlag =
+  (flagsToCheck: TypeFlags): OptionTester =>
+  type =>
+    isTypeFlagSet(type, flagsToCheck);
+
+const optionTesters = (
+  [
+    ['Any', isTypeAnyType],
+    [
+      'Array',
+      (type, checker, recursivelyCheckType): boolean =>
+        checker.isArrayType(type) &&
+        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+        recursivelyCheckType(type.getNumberIndexType()!),
+    ],
+    // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum
+    ['Boolean', testTypeFlag(TypeFlags.BooleanLike)],
+    ['Nullish', testTypeFlag(TypeFlags.Null | TypeFlags.Undefined)],
+    ['Number', testTypeFlag(TypeFlags.NumberLike | TypeFlags.BigIntLike)],
+    [
+      'RegExp',
+      (type, checker): boolean => getTypeName(checker, type) === 'RegExp',
+    ],
+    ['Never', isTypeNeverType],
+  ] satisfies [string, OptionTester][]
+).map(([type, tester]) => ({
+  type,
+  option: `allow${type}` as const,
+  tester,
+}));
 type Options = [
-  {
-    allowAny?: boolean;
-    allowBoolean?: boolean;
-    allowNullish?: boolean;
-    allowNumber?: boolean;
-    allowRegExp?: boolean;
-    allowNever?: boolean;
-  },
+  { [Type in (typeof optionTesters)[number]['option']]?: boolean },
 ];
 
 type MessageId = 'invalidType';
@@ -42,38 +72,15 @@ export default createRule<Options, MessageId>({
       {
         type: 'object',
         additionalProperties: false,
-        properties: {
-          allowAny: {
-            description:
-              'Whether to allow `any` typed values in template expressions.',
-            type: 'boolean',
-          },
-          allowBoolean: {
-            description:
-              'Whether to allow `boolean` typed values in template expressions.',
-            type: 'boolean',
-          },
-          allowNullish: {
-            description:
-              'Whether to allow `nullish` typed values in template expressions.',
-            type: 'boolean',
-          },
-          allowNumber: {
-            description:
-              'Whether to allow `number` typed values in template expressions.',
-            type: 'boolean',
-          },
-          allowRegExp: {
-            description:
-              'Whether to allow `regexp` typed values in template expressions.',
-            type: 'boolean',
-          },
-          allowNever: {
-            description:
-              'Whether to allow `never` typed values in template expressions.',
-            type: 'boolean',
-          },
-        },
+        properties: Object.fromEntries(
+          optionTesters.map(({ option, type }) => [
+            option,
+            {
+              description: `Whether to allow \`${type.toLowerCase()}\` typed values in template expressions.`,
+              type: 'boolean',
+            },
+          ]),
+        ),
       },
     ],
   },
@@ -89,47 +96,9 @@ export default createRule<Options, MessageId>({
   create(context, [options]) {
     const services = getParserServices(context);
     const checker = services.program.getTypeChecker();
-
-    function isUnderlyingTypePrimitive(type: ts.Type): boolean {
-      if (isTypeFlagSet(type, ts.TypeFlags.StringLike)) {
-        return true;
-      }
-
-      if (
-        options.allowNumber &&
-        isTypeFlagSet(type, ts.TypeFlags.NumberLike | ts.TypeFlags.BigIntLike)
-      ) {
-        return true;
-      }
-
-      if (
-        options.allowBoolean &&
-        isTypeFlagSet(type, ts.TypeFlags.BooleanLike)
-      ) {
-        return true;
-      }
-
-      if (options.allowAny && isTypeAnyType(type)) {
-        return true;
-      }
-
-      if (options.allowRegExp && getTypeName(checker, type) === 'RegExp') {
-        return true;
-      }
-
-      if (
-        options.allowNullish &&
-        isTypeFlagSet(type, ts.TypeFlags.Null | ts.TypeFlags.Undefined)
-      ) {
-        return true;
-      }
-
-      if (options.allowNever && isTypeNeverType(type)) {
-        return true;
-      }
-
-      return false;
-    }
+    const enabledOptionTesters = optionTesters.filter(
+      ({ option }) => options[option],
+    );
 
     return {
       TemplateLiteral(node: TSESTree.TemplateLiteral): void {
@@ -144,12 +113,7 @@ export default createRule<Options, MessageId>({
             expression,
           );
 
-          if (
-            !isInnerUnionOrIntersectionConformingTo(
-              expressionType,
-              isUnderlyingTypePrimitive,
-            )
-          ) {
+          if (!recursivelyCheckType(expressionType)) {
             context.report({
               node: expression,
               messageId: 'invalidType',
@@ -160,23 +124,21 @@ export default createRule<Options, MessageId>({
       },
     };
 
-    function isInnerUnionOrIntersectionConformingTo(
-      type: ts.Type,
-      predicate: (underlyingType: ts.Type) => boolean,
-    ): boolean {
-      return rec(type);
-
-      function rec(innerType: ts.Type): boolean {
-        if (innerType.isUnion()) {
-          return innerType.types.every(rec);
-        }
-
-        if (innerType.isIntersection()) {
-          return innerType.types.some(rec);
-        }
+    function recursivelyCheckType(innerType: Type): boolean {
+      if (innerType.isUnion()) {
+        return innerType.types.every(recursivelyCheckType);
+      }
 
-        return predicate(innerType);
+      if (innerType.isIntersection()) {
+        return innerType.types.some(recursivelyCheckType);
       }
+
+      return (
+        isTypeFlagSet(innerType, TypeFlags.StringLike) ||
+        enabledOptionTesters.some(({ tester }) =>
+          tester(innerType, checker, recursivelyCheckType),
+        )
+      );
     }
   },
 });
diff --git a/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts b/packages/eslint-plugin/tests/rules/restrict-template-expressions.test.ts
@@ -131,6 +131,30 @@ ruleTester.run('restrict-template-expressions', rule, {
         }
       `,
     },
+    // allowArray
+    {
+      options: [{ allowArray: true }],
+      code: `
+        const arg = [];
+        const msg = \`arg = \${arg}\`;
+      `,
+    },
+    {
+      options: [{ allowArray: true }],
+      code: `
+        const arg = [];
+        const msg = \`arg = \${arg || 'default'}\`;
+      `,
+    },
+    {
+      options: [{ allowArray: true }],
+      code: `
+        const arg = [];
+        function test<T extends string[]>(arg: T) {
+          return \`arg = \${arg}\`;
+        }
+      `,
+    },
     // allowAny
     {
       options: [{ allowAny: true }],
@@ -341,6 +365,20 @@ ruleTester.run('restrict-template-expressions', rule, {
       ],
       options: [{ allowNullish: false }],
     },
+    {
+      code: `
+        const msg = \`arg = \${[, 2]}\`;
+      `,
+      errors: [
+        {
+          messageId: 'invalidType',
+          data: { type: '(number | undefined)[]' },
+          line: 2,
+          column: 30,
+        },
+      ],
+      options: [{ allowNullish: false, allowArray: true }],
+    },
     {
       code: `
         declare const arg: number;
@@ -369,11 +407,7 @@ ruleTester.run('restrict-template-expressions', rule, {
           column: 30,
         },
       ],
-      options: [
-        {
-          allowBoolean: false,
-        },
-      ],
+      options: [{ allowBoolean: false }],
     },
     {
       options: [{ allowNumber: true, allowBoolean: true, allowNullish: true }],

diff --git a/packages/eslint-plugin/tests/schema-snapshots/restrict-template-expressions.shot b/packages/eslint-plugin/tests/schema-snapshots/restrict-template-expressions.shot