feat(eslint-plugin): [no-useless-template-literals] add new rule (#7957)

* feat(eslint-plugin): [no-useless-template-literals] add new rule Closes #2846 * fix tests * thank you josh * hopefully fix tests? * support template literals with new lines * support also quotes * fix all files (damn, we need an auto fixer...) * wip * report on specific node * fix docs * fix lint * wip * fix lint * revert unrelated changes * more reverts * wip * wip --------- Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
typescript-eslint · Dec 12, 2023 · ff75785 · ff75785
1 parent c9661c8
commit ff75785
Show file tree

Hide file tree

Showing 12 changed files with 505 additions and 6 deletions.
diff --git a/packages/eslint-plugin/docs/rules/no-useless-template-literals.md b/packages/eslint-plugin/docs/rules/no-useless-template-literals.md
@@ -0,0 +1,57 @@
+---
+description: 'Disallow unnecessary template literals.'
+---
+
+> 🛑 This file is source code, not the primary documentation location! 🛑
+>
+> See **https://typescript-eslint.io/rules/no-useless-template-literals** for documentation.
+
+This rule reports template literals that can be simplified to a normal string literal.
+
+## Examples
+
+<!--tabs-->
+
+### ❌ Incorrect
+
+```ts
+const ab1 = `${'a'}${'b'}`;
+const ab2 = `a${'b'}`;
+
+const stringWithNumber = `${'1 + 1 = '}${2}`;
+
+const stringWithBoolean = `${'true is '}${true}`;
+
+const text = 'a';
+const wrappedText = `${text}`;
+
+declare const intersectionWithString: string & { _brand: 'test-brand' };
+const wrappedIntersection = `${intersectionWithString}`;
+```
+
+### ✅ Correct
+
+```ts
+const ab1 = 'ab';
+const ab2 = 'ab';
+
+const stringWithNumber = `1 + 1 = 2`;
+
+const stringWithBoolean = `true is true`;
+
+const text = 'a';
+const wrappedText = text;
+
+declare const intersectionWithString: string & { _brand: 'test-brand' };
+const wrappedIntersection = intersectionWithString;
+```
+
+<!--/tabs-->
+
+## When Not To Use It
+
+When you want to allow string expressions inside template literals.
+
+## Related To
+
+- [`restrict-template-expressions`](./restrict-template-expressions.md)
diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts
@@ -135,6 +135,7 @@ export = {
     'no-useless-constructor': 'off',
     '@typescript-eslint/no-useless-constructor': 'error',
     '@typescript-eslint/no-useless-empty-export': 'error',
+    '@typescript-eslint/no-useless-template-literals': 'error',
     '@typescript-eslint/no-var-requires': 'error',
     '@typescript-eslint/non-nullable-type-assertion-style': 'error',
     'object-curly-spacing': 'off',

diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts
@@ -35,6 +35,7 @@ export = {
     '@typescript-eslint/no-unsafe-member-access': 'off',
     '@typescript-eslint/no-unsafe-return': 'off',
     '@typescript-eslint/no-unsafe-unary-minus': 'off',
+    '@typescript-eslint/no-useless-template-literals': 'off',
     '@typescript-eslint/non-nullable-type-assertion-style': 'off',
     '@typescript-eslint/prefer-destructuring': 'off',
     '@typescript-eslint/prefer-includes': 'off',

diff --git a/packages/eslint-plugin/src/configs/strict-type-checked.ts b/packages/eslint-plugin/src/configs/strict-type-checked.ts
@@ -56,6 +56,7 @@ export = {
     '@typescript-eslint/no-unused-vars': 'error',
     'no-useless-constructor': 'off',
     '@typescript-eslint/no-useless-constructor': 'error',
+    '@typescript-eslint/no-useless-template-literals': 'error',
     '@typescript-eslint/no-var-requires': 'error',
     '@typescript-eslint/prefer-as-const': 'error',
     '@typescript-eslint/prefer-includes': 'error',

diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts
@@ -189,7 +189,7 @@ export default createRule<Options, MessageIds>({
 
             // We have both type and value violations.
             const allExportNames = report.typeBasedSpecifiers.map(
-              specifier => `${specifier.local.name}`,
+              specifier => specifier.local.name,
             );
 
             if (allExportNames.length === 1) {

diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts
@@ -93,6 +93,7 @@ import noUnusedVars from './no-unused-vars';
 import noUseBeforeDefine from './no-use-before-define';
 import noUselessConstructor from './no-useless-constructor';
 import noUselessEmptyExport from './no-useless-empty-export';
+import noUselessTemplateLiterals from './no-useless-template-literals';
 import noVarRequires from './no-var-requires';
 import nonNullableTypeAssertionStyle from './non-nullable-type-assertion-style';
 import objectCurlySpacing from './object-curly-spacing';
@@ -231,6 +232,7 @@ export default {
   'no-use-before-define': noUseBeforeDefine,
   'no-useless-constructor': noUselessConstructor,
   'no-useless-empty-export': noUselessEmptyExport,
+  'no-useless-template-literals': noUselessTemplateLiterals,
   'no-var-requires': noVarRequires,
   'non-nullable-type-assertion-style': nonNullableTypeAssertionStyle,
   'object-curly-spacing': objectCurlySpacing,

diff --git a/packages/eslint-plugin/src/rules/no-useless-template-literals.ts b/packages/eslint-plugin/src/rules/no-useless-template-literals.ts
@@ -0,0 +1,91 @@
+import type { TSESTree } from '@typescript-eslint/utils';
+import { AST_NODE_TYPES } from '@typescript-eslint/utils';
+import * as ts from 'typescript';
+
+import {
+  createRule,
+  getConstrainedTypeAtLocation,
+  getParserServices,
+  isTypeFlagSet,
+  isUndefinedIdentifier,
+} from '../util';
+
+type MessageId = 'noUselessTemplateLiteral';
+
+export default createRule<[], MessageId>({
+  name: 'no-useless-template-literals',
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'Disallow unnecessary template literals',
+      recommended: 'strict',
+      requiresTypeChecking: true,
+    },
+    messages: {
+      noUselessTemplateLiteral:
+        'Template literal expression is unnecessary and can be simplified.',
+    },
+    schema: [],
+  },
+  defaultOptions: [],
+  create(context) {
+    const services = getParserServices(context);
+
+    function isUnderlyingTypeString(
+      expression: TSESTree.Expression,
+    ): expression is TSESTree.StringLiteral | TSESTree.Identifier {
+      const type = getConstrainedTypeAtLocation(services, expression);
+
+      const isString = (t: ts.Type): boolean => {
+        return isTypeFlagSet(t, ts.TypeFlags.StringLike);
+      };
+
+      if (type.isUnion()) {
+        return type.types.every(isString);
+      }
+
+      if (type.isIntersection()) {
+        return type.types.some(isString);
+      }
+
+      return isString(type);
+    }
+
+    return {
+      TemplateLiteral(node: TSESTree.TemplateLiteral): void {
+        if (node.parent.type === AST_NODE_TYPES.TaggedTemplateExpression) {
+          return;
+        }
+
+        const hasSingleStringVariable =
+          node.quasis.length === 2 &&
+          node.quasis[0].value.raw === '' &&
+          node.quasis[1].value.raw === '' &&
+          node.expressions.length === 1 &&
+          isUnderlyingTypeString(node.expressions[0]);
+
+        if (hasSingleStringVariable) {
+          context.report({
+            node: node.expressions[0],
+            messageId: 'noUselessTemplateLiteral',
+          });
+
+          return;
+        }
+
+        const literalsOrUndefinedExpressions = node.expressions.filter(
+          (expression): expression is TSESTree.Literal | TSESTree.Identifier =>
+            expression.type === AST_NODE_TYPES.Literal ||
+            isUndefinedIdentifier(expression),
+        );
+
+        literalsOrUndefinedExpressions.forEach(expression => {
+          context.report({
+            node: expression,
+            messageId: 'noUselessTemplateLiteral',
+          });
+        });
+      },
+    };
+  },
+});
diff --git a/packages/eslint-plugin/tests/docs.test.ts b/packages/eslint-plugin/tests/docs.test.ts
@@ -159,7 +159,7 @@ describe('Validating rule metadata', () => {
   }
 
   for (const [ruleName, rule] of rulesData) {
-    describe(`${ruleName}`, () => {
+    describe(ruleName, () => {
       it('`name` field in rule must match the filename', () => {
         // validate if rule name is same as url
         // there is no way to access this field but its used only in generation of docs url

diff --git a/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts b/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts
@@ -3,7 +3,7 @@
 /* eslint "@typescript-eslint/internal/plugin-test-formatting": ["error", { formatWithPrettier: false }] */
 /* eslint-enable eslint-comments/no-use */
 
-import { RuleTester } from '@typescript-eslint/rule-tester';
+import { noFormat, RuleTester } from '@typescript-eslint/rule-tester';
 
 import rule from '../../src/rules/no-extra-parens';
 
@@ -743,7 +743,7 @@ const Component = (
     />
 )
       `,
-      output: `
+      output: noFormat`
 const Component =${' '}
     <div>
         <p />

diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
@@ -1235,7 +1235,7 @@ foo ?.
 foo
   ?. ();
       `,
-      output: `
+      output: noFormat`
 let foo = () => {};
 foo();
 foo  ();
@@ -1285,7 +1285,7 @@ foo ?.
 foo
   ?. (bar);
       `,
-      output: `
+      output: noFormat`
 let foo = () => {};
 foo(bar);
 foo  (bar);