Repository: nicoespeon/abracadabra
Pull request #274 (2021-04-22 13:37:58)

Move types & interfaces to existing files
Closes https://github.com/nicoespeon/abracadabra/issues/265

Implement "Move to Existing File" for types & interfaces too! It works the same than for functions.

Thanks @ZakMiller for suggesting it 👍 


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 87eeb967..0681bd38 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Extract Variable won't extract type parameter instantiations anymore, because it shouldn't!
 - Split Declaration from Initialization now works fine with destructured assignments.
 
+### Added
+
+- Move to Existing File now works with types and interfaces! So you can move these to an existing source file.
+
 ## [5.2.0] - 2021-04-15 - Poor Unfortunate Types 🦑
 
 ### Fixed
diff --git a/src/ast/domain.ts b/src/ast/domain.ts
index c1bcbe20..94c129e3 100644
--- a/src/ast/domain.ts
+++ b/src/ast/domain.ts
@@ -13,6 +13,7 @@ export {
   isEmpty,
   replaceWithBodyOf,
   Primitive,
+  TypeDeclaration,
   forEach
 };
 
@@ -106,6 +107,8 @@ type Primitive =
   | t.BooleanLiteral
   | t.BigIntLiteral;
 
+type TypeDeclaration = t.TSTypeAliasDeclaration | t.TSInterfaceDeclaration;
+
 function forEach(
   object: t.Expression,
   params: t.ArrowFunctionExpression["params"],
diff --git a/src/ast/scope.ts b/src/ast/scope.ts
index d8f177ca..183e27be 100644
--- a/src/ast/scope.ts
+++ b/src/ast/scope.ts
@@ -1,7 +1,7 @@
 import { NodePath, Binding } from "@babel/traverse";
 import * as t from "@babel/types";
 import { first } from "../array";
-import { getImportDeclarations } from "./domain";
+import { getImportDeclarations, TypeDeclaration } from "./domain";
 
 import { areEquivalent } from "./identity";
 import { isSelectablePath, SelectablePath } from "./selection";
@@ -17,7 +17,9 @@ export {
   bindingNamesInScope,
   referencesInScope,
   getReferencedImportDeclarations,
-  hasReferencesDefinedInSameScope
+  getTypeReferencedImportDeclarations,
+  hasReferencesDefinedInSameScope,
+  hasTypeReferencesDefinedInSameScope
 };
 
 function findScopePath(path: NodePath<t.Node | null>): NodePath | undefined {
@@ -85,9 +87,9 @@ function isShadowIn(
 }
 
 function findFirstExistingDeclaration(expressionPath: NodePath<t.Expression>) {
-  const existingDeclarations: NodePath<
-    DestructuredVariableDeclarator
-  >[] = Object.values(expressionPath.scope.getAllBindings())
+  const existingDeclarations: NodePath<DestructuredVariableDeclarator>[] = Object.values(
+    expressionPath.scope.getAllBindings()
+  )
     .map(({ path }) => path)
     .filter(
       (path): path is NodePath<DestructuredVariableDeclarator> =>
@@ -185,6 +187,35 @@ function getReferencedImportDeclarations(
   return result;
 }
 
+function getTypeReferencedImportDeclarations(
+  typePath: NodePath<TypeDeclaration>,
+  programPath: NodePath<t.Program>
+): t.ImportDeclaration[] {
+  let result: t.ImportDeclaration[] = [];
+
+  const importDeclarations = getImportDeclarations(programPath);
+  typePath.traverse({
+    TSTypeReference(path) {
+      if (!path.isReferenced()) return;
+
+      importDeclarations.forEach((declaration) => {
+        const matchingSpecifier = declaration.specifiers.find(({ local }) =>
+          areEquivalent(local, path.node.typeName)
+        );
+
+        if (matchingSpecifier) {
+          result.push({
+            ...declaration,
+            specifiers: [matchingSpecifier]
+          });
+        }
+      });
+    }
+  });
+
+  return result;
+}
+
 function hasReferencesDefinedInSameScope(
   functionPath: NodePath<t.FunctionDeclaration>,
   programPath: NodePath<t.Program>
@@ -215,6 +246,26 @@ function hasReferencesDefinedInSameScope(
   return result;
 }
 
+function hasTypeReferencesDefinedInSameScope(
+  typePath: NodePath<TypeDeclaration>
+): boolean {
+  let result = false;
+
+  typePath.traverse({
+    TSTypeReference(path) {
+      if (!path.isReferenced()) return;
+      if (!t.isIdentifier(path.node.typeName)) return;
+
+      if (typePath.scope.hasGlobal(path.node.typeName.name)) {
+        result = true;
+        path.stop();
+      }
+    }
+  });
+
+  return result;
+}
+
 function getNames(node: t.Node | null): string[] {
   if (t.isArrayPattern(node)) return node.elements.flatMap(getNames);
   if (t.isAssignmentPattern(node)) return getNames(node.left);
diff --git a/src/ast/transformation.ts b/src/ast/transformation.ts
index 1c3244b8..8d2a55e9 100644
--- a/src/ast/transformation.ts
+++ b/src/ast/transformation.ts
@@ -15,6 +15,7 @@ const traverseNode = t.traverse;
 const traversePath = traverse;
 
 export { NodePath, Visitor, Scope } from "@babel/traverse";
+export { RootNodePath, isRootNodePath };
 export {
   generate,
   traverseNode,
@@ -251,3 +252,13 @@ function mergeCommentsInto<T extends t.Node>(
     )
   };
 }
+
+type RootNodePath<T = t.Node> = NodePath<T> & {
+  parentPath: NodePath<t.Program>;
+};
+
+function isRootNodePath<T = t.Node>(
+  path: NodePath<T>
+): path is RootNodePath<T> {
+  return path.parentPath.isProgram();
+}
diff --git a/src/refactorings/move-to-existing-file/move-to-existing-file.ts b/src/refactorings/move-to-existing-file/move-to-existing-file.ts
index b151b8e1..13a61361 100644
--- a/src/refactorings/move-to-existing-file/move-to-existing-file.ts
+++ b/src/refactorings/move-to-existing-file/move-to-existing-file.ts
@@ -64,19 +64,16 @@ function updateCode(
 
   const updatedCode = t.transformAST(
     ast,
-    createVisitor(
-      selection,
-      (path, importIdentifier, programPath, matchingMovableNode) => {
-        movableNode = matchingMovableNode;
-
-        t.addImportDeclaration(
-          programPath,
-          importIdentifier,
-          relativePath.withoutExtension
-        );
-        path.remove();
-      }
-    )
+    createVisitor(selection, (path, importIdentifier, matchingMovableNode) => {
+      movableNode = matchingMovableNode;
+
+      t.addImportDeclaration(
+        path.parentPath,
+        importIdentifier,
+        relativePath.withoutExtension
+      );
+      path.remove();
+    })
   );
 
   return {
@@ -113,15 +110,14 @@ function updateOtherFileCode(
 function createVisitor(
   selection: Selection,
   onMatch: (
-    path: t.NodePath,
+    path: t.RootNodePath,
     importIdentifier: t.Identifier,
-    program: t.NodePath<t.Program>,
     movableNode: MovableNode
   ) => void
 ): t.Visitor {
   return {
     FunctionDeclaration(path) {
-      if (!path.parentPath.isProgram()) return;
+      if (!t.isRootNodePath(path)) return;
       if (!path.node.id) return;
       if (!selection.isInsidePath(path)) return;
 
@@ -131,12 +127,21 @@ function createVisitor(
       const bodySelection = Selection.fromAST(body.node.loc);
       if (selection.end.isAfter(bodySelection.start)) return;
 
-      onMatch(
-        path,
-        path.node.id,
-        path.parentPath,
-        new MovableFunctionDeclaration(path, path.parentPath)
-      );
+      onMatch(path, path.node.id, new MovableFunctionDeclaration(path));
+    },
+    TSTypeAliasDeclaration(path) {
+      if (!t.isRootNodePath(path)) return;
+      if (!path.node.id) return;
+      if (!selection.isInsidePath(path)) return;
+
+      onMatch(path, path.node.id, new MovableTSTypeDeclaration(path));
+    },
+    TSInterfaceDeclaration(path) {
+      if (!t.isRootNodePath(path)) return;
+      if (!path.node.id) return;
+      if (!selection.isInsidePath(path)) return;
+
+      onMatch(path, path.node.id, new MovableTSTypeDeclaration(path));
     }
   };
 }
@@ -161,20 +166,17 @@ class MovableFunctionDeclaration implements MovableNode {
   private _hasReferencesThatCantBeImported: boolean;
   private _referencedImportDeclarations: t.ImportDeclaration[];
 
-  constructor(
-    path: t.NodePath<t.FunctionDeclaration>,
-    programPath: t.NodePath<t.Program>
-  ) {
+  constructor(path: t.RootNodePath<t.FunctionDeclaration>) {
     // We need to compute these in constructor because the `path` reference
     // will be removed and not accessible later.
     this._value = path.node;
     this._hasReferencesThatCantBeImported = t.hasReferencesDefinedInSameScope(
       path,
-      programPath
+      path.parentPath
     );
     this._referencedImportDeclarations = t.getReferencedImportDeclarations(
       path,
-      programPath
+      path.parentPath
     );
   }
 
@@ -202,3 +204,44 @@ class MovableFunctionDeclaration implements MovableNode {
     });
   }
 }
+
+class MovableTSTypeDeclaration implements MovableNode {
+  private _value: t.TypeDeclaration;
+  private _hasReferencesThatCantBeImported: boolean;
+  private _referencedImportDeclarations: t.ImportDeclaration[];
+
+  constructor(path: t.RootNodePath<t.TypeDeclaration>) {
+    this._value = path.node;
+    this._hasReferencesThatCantBeImported = t.hasTypeReferencesDefinedInSameScope(
+      path
+    );
+    this._referencedImportDeclarations = t.getTypeReferencedImportDeclarations(
+      path,
+      path.parentPath
+    );
+  }
+
+  get value(): t.TypeDeclaration {
+    return this._value;
+  }
+
+  get hasReferencesThatCantBeImported(): boolean {
+    return this._hasReferencesThatCantBeImported;
+  }
+
+  declarationsToImportFrom(relativePath: RelativePath): t.ImportDeclaration[] {
+    return this._referencedImportDeclarations.map((declaration) => {
+      const importRelativePath = new RelativePath(
+        declaration.source.value
+      ).relativeTo(relativePath);
+
+      return {
+        ...declaration,
+        source: {
+          ...declaration.source,
+          value: importRelativePath.value
+        }
+      };
+    });
+  }
+}


Test patch:
diff --git a/src/refactorings/move-to-existing-file/move-to-existing-file.test.ts b/src/refactorings/move-to-existing-file/move-to-existing-file.test.ts
index 6dfe1e5d..706b75eb 100644
--- a/src/refactorings/move-to-existing-file/move-to-existing-file.test.ts
+++ b/src/refactorings/move-to-existing-file/move-to-existing-file.test.ts
@@ -127,6 +127,91 @@ sayHello();`,
 
 export function sayHello() {
   console.log(HELLO, WORLD);
+}`
+        }
+      },
+      {
+        description: "a type alias",
+        setup: {
+          currentFile: `type [cursor]SomeType = string;
+
+const someValue: SomeType = "irrelevant";`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { SomeType } from "./other-file";
+
+const someValue: SomeType = "irrelevant";`,
+          otherFile: `export type SomeType = string;`
+        }
+      },
+      {
+        description: "a type alias referring to others",
+        setup: {
+          currentFile: `import { OtherType } from "../some-file";
+
+type [cursor]SomeType = OtherType | string;
+
+const someValue: SomeType = "irrelevant";`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { SomeType } from "./other-file";
+import { OtherType } from "../some-file";
+
+const someValue: SomeType = "irrelevant";`,
+          otherFile: `import { OtherType } from "../some-file";
+export type SomeType = OtherType | string;`
+        }
+      },
+      {
+        description: "an interface",
+        setup: {
+          currentFile: `interface [cursor]Data {
+  value: string;
+}
+
+let someData: Data;`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { Data } from "./other-file";
+
+let someData: Data;`,
+          otherFile: `export interface Data {
+  value: string;
+}`
+        }
+      },
+      {
+        description: "an interface referring to others",
+        setup: {
+          currentFile: `import { Value } from "../some-file";
+
+interface [cursor]Data {
+  response: {
+    value: Value;
+  };
+}
+
+let someData: Data;`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { Data } from "./other-file";
+import { Value } from "../some-file";
+
+let someData: Data;`,
+          otherFile: `import { Value } from "../some-file";
+
+export interface Data {
+  response: {
+    value: Value;
+  };
 }`
         }
       }
@@ -185,6 +270,19 @@ function [cursor]doSomething() {
 }
 
 function referencedHere() {}`
+      },
+      {
+        description: "a type alias with references defined in the same file",
+        code: `type [cursor]SomeType = OtherType | string;
+type OtherType = string;`
+      },
+      {
+        description: "an interface with references defined in the same file",
+        code: `interface [cursor]Data {
+  value: Value;
+}
+
+type Value = string;`
       }
     ],
     async ({ code }) => {