fix(eslint-plugin): [no-unnecessary-template-expressions] allow template expressions used to make trailing whitespace visible (#10363)

* whitespace is useful

* no-longer-needed-disable

* windows stuff and template cov

* remove duplicate test case

* tweaks

Author: kirkwaiblinger

packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts

@@ -126,27 +126,58 @@ export default createRule<[], MessageId>({
           return;
         }
 
-        const fixableExpressions = node.expressions
-          .filter(
-            expression =>
-              isLiteral(expression) ||
-              isTemplateLiteral(expression) ||
+        const fixableExpressionsReversed = node.expressions
+          .map((expression, index) => ({
+            expression,
+            nextQuasi: node.quasis[index + 1],
+            prevQuasi: node.quasis[index],
+          }))
+          .filter(({ expression, nextQuasi }) => {
+            if (
               isUndefinedIdentifier(expression) ||
               isInfinityIdentifier(expression) ||
-              isNaNIdentifier(expression),
-          )
+              isNaNIdentifier(expression)
+            ) {
+              return true;
+            }
+
+            if (isLiteral(expression)) {
+              // allow trailing whitespace literal
+              if (startsWithNewLine(nextQuasi.value.raw)) {
+                return !(
+                  typeof expression.value === 'string' &&
+                  isWhitespace(expression.value)
+                );
+              }
+              return true;
+            }
+
+            if (isTemplateLiteral(expression)) {
+              // allow trailing whitespace literal
+              if (startsWithNewLine(nextQuasi.value.raw)) {
+                return !(
+                  expression.quasis.length === 1 &&
+                  isWhitespace(expression.quasis[0].value.raw)
+                );
+              }
+              return true;
+            }
+
+            return false;
+          })
           .reverse();
 
         let nextCharacterIsOpeningCurlyBrace = false;
 
-        for (const expression of fixableExpressions) {
+        for (const {
+          expression,
+          nextQuasi,
+          prevQuasi,
+        } of fixableExpressionsReversed) {
           const fixers: ((fixer: TSESLint.RuleFixer) => TSESLint.RuleFix[])[] =
             [];
-          const index = node.expressions.indexOf(expression);
-          const prevQuasi = node.quasis[index];
-          const nextQuasi = node.quasis[index + 1];
 
-          if (nextQuasi.value.raw.length !== 0) {
+          if (nextQuasi.value.raw !== '') {
             nextCharacterIsOpeningCurlyBrace =
               nextQuasi.value.raw.startsWith('{');
           }
@@ -271,3 +302,19 @@ export default createRule<[], MessageId>({
     };
   },
 });
+
+function isWhitespace(x: string): boolean {
+  // allow empty string too since we went to allow
+  // `      ${''}
+  // `;
+  //
+  // in addition to
+  // `${'        '}
+  // `;
+  //
+  return /^\s*$/.test(x);
+}
+
+function startsWithNewLine(x: string): boolean {
+  return x.startsWith('\n') || x.startsWith('\r\n');
+}

packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts

@@ -885,6 +885,75 @@ const invalidCases: readonly InvalidTestCase<
     errors: [{ messageId: 'noUnnecessaryTemplateExpression' }],
     output: '` \\ud83d\\udc68 `;',
   },
+  {
+    code: `
+\`
+this code does not have trailing whitespace: \${' '}\\n even though it might look it.\`;
+    `,
+    errors: [
+      {
+        messageId: 'noUnnecessaryTemplateExpression',
+      },
+    ],
+    output: `
+\`
+this code does not have trailing whitespace:  \\n even though it might look it.\`;
+    `,
+  },
+  {
+    code: `
+\`
+this code has trailing position template expression \${'but it isn\\'t whitespace'}
+    \`;
+    `,
+    errors: [
+      {
+        messageId: 'noUnnecessaryTemplateExpression',
+      },
+    ],
+    output: `
+\`
+this code has trailing position template expression but it isn\\'t whitespace
+    \`;
+    `,
+  },
+  {
+    code: noFormat`
+\`trailing whitespace followed by escaped windows newline: \${' '}\\r\\n\`;
+    `,
+    errors: [
+      {
+        messageId: 'noUnnecessaryTemplateExpression',
+      },
+    ],
+    output: `
+\`trailing whitespace followed by escaped windows newline:  \\r\\n\`;
+    `,
+  },
+  {
+    code: `
+\`template literal with interpolations followed by newline: \${\` \${'interpolation'} \`}
+\`;
+    `,
+    errors: [
+      {
+        messageId: 'noUnnecessaryTemplateExpression',
+      },
+      {
+        messageId: 'noUnnecessaryTemplateExpression',
+      },
+    ],
+    output: [
+      `
+\`template literal with interpolations followed by newline:  \${'interpolation'}${' '}
+\`;
+    `,
+      `
+\`template literal with interpolations followed by newline:  interpolation${' '}
+\`;
+    `,
+    ],
+  },
 ];
 
 describe('fixer should not change runtime value', () => {
@@ -1023,6 +1092,18 @@ ruleTester.run('no-unnecessary-template-expression', rule, {
     `
 \`not a useless \${String.raw\`nested interpolation \${a}\`}\`;
     `,
+    `
+\`
+this code has trailing whitespace: \${'    '}
+    \`;
+    `,
+    noFormat`
+\`this code has trailing whitespace with a windows \\\r new line: \${' '}\r\n\`;
+    `,
+    `
+\`trailing position interpolated empty string also makes whitespace clear    \${''}
+\`;
+    `,
   ],
 
   invalid: [

packages/eslint-plugin/tests/rules/no-useless-constructor.test.ts

@@ -209,8 +209,6 @@ class A extends Object {
     `,
   ],
   invalid: [
-    /* eslint-disable @typescript-eslint/no-unnecessary-template-expression -- trailing whitespace */
-
     {
       code: `
 class A {
@@ -432,6 +430,5 @@ ${'  '}
         },
       ],
     },
-    /* eslint-enable @typescript-eslint/no-unnecessary-template-expression */
   ],
 });