Add autofix for preferTemplate rule

jamiebuilds · Apr 28, 2020 · 71cac3e · 71cac3e
1 parent 2c0f6d7
commit 71cac3e
Show file tree

Hide file tree

Showing 3 changed files with 204 additions and 58 deletions.
diff --git a/packages/@romejs/js-compiler/lint/rules/preferTemplate.test.md b/packages/@romejs/js-compiler/lint/rules/preferTemplate.test.md
@@ -62,3 +62,34 @@ console.log(`${foo}baz`);
 console.log(`${1 * 2}baz`);
 
 ```
+
+### `2`
+
+```
+
+ unknown:1:49 lint/preferTemplate FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ Template literals are preferred over string concatenation
+
+    const foo = 'bar'; const bar = 'foo' console.log(foo + 'baz' + bar + 'bam' + 'boo' + foo)
+                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
+
+  ℹ Recommended fix
+
+  - foo·+·'baz'·+·bar·+·'bam'·+·'boo'·+·foo
+  + `${foo}baz${bar}bamboo${foo}`
+
+━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+✖ Found 1 problem
+
+```
+
+### `2: formatted`
+
+```
+const foo = 'bar';
+const bar = 'foo';
+console.log(`${foo}baz${bar}bamboo${foo}`);
+
+```
diff --git a/packages/@romejs/js-compiler/lint/rules/preferTemplate.test.ts b/packages/@romejs/js-compiler/lint/rules/preferTemplate.test.ts
@@ -26,5 +26,13 @@ test(
         category: 'lint/preferTemplate',
       },
     );
+
+    await testLint(
+      t,
+      `const foo = 'bar'; const bar = 'foo' console.log(foo + 'baz' + bar + 'bam' + 'boo' + foo)`,
+      {
+        category: 'lint/preferTemplate',
+      },
+    );
   },
 );
diff --git a/packages/@romejs/js-compiler/lint/rules/preferTemplate.ts b/packages/@romejs/js-compiler/lint/rules/preferTemplate.ts
@@ -5,82 +5,189 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {Path} from '@romejs/js-compiler';
+import {Path, TransformExitResult} from '@romejs/js-compiler';
 import {
+  AnyExpression,
+  AnyNode,
+  BinaryExpression,
+  StringLiteral,
   TemplateLiteral,
+  stringLiteral,
   templateElement,
   templateLiteral,
 } from '@romejs/js-ast';
 import {descriptions} from '@romejs/diagnostics';
-import {TransformExitResult} from '@romejs/js-compiler/types';
-import {removeShallowLoc} from '@romejs/js-ast-utils';
 
-export default {
-  name: 'preferTemplate',
-  enter(path: Path): TransformExitResult {
-    const {node} = path;
+// expr + expr
+function isBinaryAddExpression(node: AnyNode): node is BinaryExpression {
+  return node.type === 'BinaryExpression' && node.operator === '+';
+}
 
-    if (
-      node.type === 'BinaryExpression' &&
-      node.operator === '+' &&
-      ((node.left.type === 'StringLiteral' && !node.left.value.includes('`')) ||
-      (node.right.type === 'StringLiteral' && !node.right.value.includes('`')))
-    ) {
-      let autofix: undefined | TemplateLiteral;
+// 'str' + 'str'
+// 'str' + expr
+// expr + 'str'
+// expr + (expr + 'str')
+// (expr + 'str') + expr
+// expr * (expr + 'str')
+// (expr * expr) + 'str'
+function isUnnecessaryStringConcatExpression(
+  node: AnyNode,
+): node is BinaryExpression {
 
-      if (node.right.type === 'StringLiteral') {
-        const quasis = [
-          templateElement.create({
-            raw: '',
-            cooked: '',
-          }),
-          templateElement.create({
-            raw: node.right.value,
-            cooked: node.right.value,
-          }),
-        ];
-        const expressions = [removeShallowLoc(node.left)];
-        autofix = templateLiteral.create({
-          expressions,
-          quasis,
-          loc: node.loc,
-        });
+  if (node.type !== 'BinaryExpression') {
+    return false;
+  }
+
+  if (node.left.type === 'BinaryExpression') {
+    if (isUnnecessaryStringConcatExpression(node.left)) {
+      return true;
+    }
+  }
+
+  if (node.right.type === 'BinaryExpression') {
+    if (isUnnecessaryStringConcatExpression(node.right)) {
+      return true;
+    }
+  }
+
+  if (!isBinaryAddExpression(node)) {
+    return false;
+  }
+
+  if (node.left.type === 'StringLiteral' && !node.left.value.includes('`')) {
+    return true;
+  }
+
+  if (node.right.type === 'StringLiteral' && !node.right.value.includes('`')) {
+    return true;
+  }
+
+  return false;
+}
+
+// expr + expr + expr + ...
+function collectBinaryAddExpressionExpressions(
+  node: BinaryExpression,
+): Array<AnyExpression> {
+  let expressions: Array<AnyExpression> = [];
+
+  if (isBinaryAddExpression(node.left)) {
+    expressions = expressions.concat(
+      collectBinaryAddExpressionExpressions(node.left),
+    );
+  } else {
+    expressions.push(node.left);
+  }
+
+  if (isBinaryAddExpression(node.right)) {
+    expressions = expressions.concat(
+      collectBinaryAddExpressionExpressions(node.right),
+    );
+  } else {
+    expressions.push(node.right);
+  }
+
+  return expressions;
+}
+
+// 'str' + 'str' + expr -> 'strstr' + expr
+function reduceBinaryExpressionExpressions(expressions: Array<AnyExpression>) {
+  let reducedExpressions: Array<AnyExpression> = [];
+  let index = 0;
+
+  while (index < expressions.length) {
+    let current = expressions[index];
+
+    if (current.type === 'StringLiteral') {
+      let strings: Array<StringLiteral> = [current];
+
+      while (index + 1 < expressions.length) {
+        let next = expressions[index + 1];
+        if (next.type === 'StringLiteral') {
+          strings.push(next);
+          index++;
+        } else {
+          break;
+        }
       }
 
-      if (node.left.type === 'StringLiteral') {
-        const quasis = [
-          templateElement.create({
-            raw: node.left.value,
-            cooked: node.left.value,
+      if (strings.length === 1) {
+        reducedExpressions.push(current);
+      } else {
+        reducedExpressions.push(
+          stringLiteral.create({
+            value: strings.map((string) => string.value).join(''),
           }),
+        );
+      }
+    } else {
+      reducedExpressions.push(current);
+    }
+
+    index++;
+  }
+
+  return reducedExpressions;
+}
+
+// 'str' + expr + 'str' -> `str${expr}str`
+function convertExpressionsToTemplateLiteral(
+  items: Array<AnyExpression>,
+): TemplateLiteral {
+  let expressions = [];
+  let quasis = [];
+
+  for (let index = 0; index < items.length; index++) {
+    let item = items[index];
+    let isTail = index === items.length - 1;
+    let isHead = index === 0;
+
+    if (item.type === 'StringLiteral') {
+      quasis.push(
+        templateElement.create({
+          cooked: item.value,
+          raw: item.value,
+          tail: isTail,
+        }),
+      );
+    } else {
+      expressions.push(item);
+      if (isTail || isHead) {
+        quasis.push(
           templateElement.create({
-            raw: '',
             cooked: '',
+            raw: '',
+            tail: isTail,
           }),
-        ];
-
-        // We need to remove the location or else if we were to show a preview the source map would resolve to the end of
-        // this node
-        const expressions = [removeShallowLoc(node.right)];
-        autofix = templateLiteral.create({
-          expressions,
-          quasis,
-          loc: node.loc,
-        });
-      }
-
-      if (autofix === undefined) {
-        path.context.addNodeDiagnostic(node, descriptions.LINT.PREFER_TEMPLATE);
-      } else {
-        return path.context.addFixableDiagnostic(
-          {
-            old: node,
-            fixed: autofix,
-          },
-          descriptions.LINT.PREFER_TEMPLATE,
         );
       }
     }
+  }
+
+  return templateLiteral.create({
+    expressions,
+    quasis,
+  });
+}
+
+export default {
+  name: 'preferTemplate',
+  enter(path: Path): TransformExitResult {
+    const {node} = path;
+
+    if (isUnnecessaryStringConcatExpression(node)) {
+      let expressions = collectBinaryAddExpressionExpressions(node);
+      let reducedExpressions = reduceBinaryExpressionExpressions(expressions);
+      let template = convertExpressionsToTemplateLiteral(reducedExpressions);
+
+      return path.context.addFixableDiagnostic(
+        {
+          old: node,
+          fixed: template,
+        },
+        descriptions.LINT.PREFER_TEMPLATE,
+      );
+    }
 
     return node;
   },