Repository: nicoespeon/abracadabra (ts)

Issue:
Improve the scope of the refactor 'move to existing file'
## Describe what's missing

This refactor only seem to work for function definitions. But doesnt work for arrow functions, moving consts, interfaces, and the like. 
These are also things that would be commonly moved to an existing file.

## How would it work

The move to new file should also move these arrow functions/consts/interfaces/types etc as well as just function definitions.


## Expected behavior

Evoking the refactor while selecting this expanded range of objects will result in them getting moved to a new file, with imports updates, rather than the plugin reporting back that 'i didnt find the code to move from the current selection'

## Screenshots

n/a

## Additional information

- Version of the extension used: v6.11.0

Pull request description:
Move exported functions/types/interfaces to existing files
Closes #514

https://user-images.githubusercontent.com/1094774/156850806-5ecc4533-912e-4a9e-b105-f70f3dcb6509.mp4


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5445129c..c8f89d79 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Move to existing file now supports moving exported elements. It will preserve the export, so your module API won't break.
+
 ## [6.11.0] - 2022-02-19 - Dos Oruguitas 🦋
 
 ### Fixed
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 4ecf3af2..3c165085 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
@@ -109,11 +109,10 @@ function updateOtherFileCode(
 
 export function createVisitor(
   selection: Selection,
-  onMatch: (path: t.RootNodePath, movableNode: MovableNode) => void
+  onMatch: (path: t.NodePath, movableNode: MovableNode) => void
 ): t.Visitor {
   return {
     FunctionDeclaration(path) {
-      if (!t.isRootNodePath(path)) return;
       if (!t.hasNodeId(path)) return;
       if (!selection.isInsidePath(path)) return;
 
@@ -123,21 +122,64 @@ export function createVisitor(
       const bodySelection = Selection.fromAST(body.node.loc);
       if (selection.end.isAfter(bodySelection.start)) return;
 
-      onMatch(path, new MovableFunctionDeclaration(path));
+      if (t.isRootNodePath(path)) {
+        onMatch(path, new MovableFunctionDeclaration(path));
+        return;
+      }
+
+      const { parentPath } = path;
+      if (!t.isRootNodePath(parentPath)) return;
+
+      const exportDeclaration = getExportDeclaration(parentPath);
+      if (exportDeclaration) {
+        onMatch(
+          path,
+          new ExportedMovableFunctionDeclaration(path, exportDeclaration)
+        );
+        return;
+      }
     },
     TSTypeAliasDeclaration(path) {
-      if (!t.isRootNodePath(path)) return;
       if (!t.hasNodeId(path)) return;
       if (!selection.isInsidePath(path)) return;
 
-      onMatch(path, new MovableTSTypeDeclaration(path));
+      if (t.isRootNodePath(path)) {
+        onMatch(path, new MovableTSTypeDeclaration(path));
+        return;
+      }
+
+      const { parentPath } = path;
+      if (!t.isRootNodePath(parentPath)) return;
+
+      const exportDeclaration = getExportDeclaration(parentPath);
+      if (exportDeclaration) {
+        onMatch(
+          path,
+          new ExportedMovableTSTypeDeclaration(path, exportDeclaration)
+        );
+        return;
+      }
     },
     TSInterfaceDeclaration(path) {
-      if (!t.isRootNodePath(path)) return;
       if (!t.hasNodeId(path)) return;
       if (!selection.isInsidePath(path)) return;
 
-      onMatch(path, new MovableTSTypeDeclaration(path));
+      if (t.isRootNodePath(path)) {
+        onMatch(path, new MovableTSTypeDeclaration(path));
+        return;
+      }
+
+      const { parentPath } = path;
+      if (!t.isRootNodePath(parentPath)) return;
+
+      const exportDeclaration = getExportDeclaration(parentPath);
+      if (exportDeclaration) {
+        onMatch(
+          path,
+          new ExportedMovableTSTypeDeclaration(path, exportDeclaration)
+        );
+        return;
+      }
     }
   };
 }
@@ -215,6 +257,62 @@ class MovableFunctionDeclaration implements MovableNode {
   }
 }
 
+class ExportedMovableFunctionDeclaration implements MovableNode {
+  private _value: t.WithId<t.FunctionDeclaration>;
+  private _hasReferencesThatCantBeImported: boolean;
+  private _referencedImportDeclarations: t.ImportDeclaration[];
+
+  constructor(
+    path: t.PathWithId<t.NodePath<t.FunctionDeclaration>>,
+    private exportDeclaration: ExportDeclaration
+  ) {
+    // 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,
+      exportDeclaration.parentPath
+    );
+    this._referencedImportDeclarations = t.getReferencedImportDeclarations(
+      path,
+      exportDeclaration.parentPath
+    );
+  }
+
+  get value(): t.FunctionDeclaration {
+    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
+        }
+      };
+    });
+  }
+
+  removeFrom(relativePath: RelativePath) {
+    t.addImportDeclaration(
+      this.exportDeclaration.parentPath,
+      this._value.id,
+      relativePath.withoutExtension
+    );
+    this.exportDeclaration.replaceWith(this._value.id);
+  }
+}
+
 class MovableTSTypeDeclaration implements MovableNode {
   private _value: t.WithId<t.TypeDeclaration>;
   private _hasReferencesThatCantBeImported: boolean;
@@ -263,3 +361,93 @@ class MovableTSTypeDeclaration implements MovableNode {
     this.path.remove();
   }
 }
+
+class ExportedMovableTSTypeDeclaration implements MovableNode {
+  private _value: t.WithId<t.TypeDeclaration>;
+  private _hasReferencesThatCantBeImported: boolean;
+  private _referencedImportDeclarations: t.ImportDeclaration[];
+
+  constructor(
+    path: t.PathWithId<t.NodePath<t.TypeDeclaration>>,
+    private exportedDeclaration: ExportDeclaration
+  ) {
+    this._value = path.node;
+    this._hasReferencesThatCantBeImported =
+      t.hasTypeReferencesDefinedInSameScope(path);
+    this._referencedImportDeclarations = t.getTypeReferencedImportDeclarations(
+      path,
+      exportedDeclaration.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
+        }
+      };
+    });
+  }
+
+  removeFrom(relativePath: RelativePath) {
+    t.addImportDeclaration(
+      this.exportedDeclaration.parentPath,
+      this._value.id,
+      relativePath.withoutExtension
+    );
+    this.exportedDeclaration.replaceWith(this._value.id);
+  }
+}
+
+function getExportDeclaration(path: t.RootNodePath): ExportDeclaration | null {
+  return path.isExportNamedDeclaration()
+    ? new NamedExportDeclaration(path)
+    : path.isExportDefaultDeclaration()
+    ? new DefaultExportDeclaration(path)
+    : null;
+}
+
+interface ExportDeclaration {
+  readonly parentPath: t.NodePath<t.Program>;
+  replaceWith(id: t.Identifier): void;
+}
+
+class NamedExportDeclaration implements ExportDeclaration {
+  constructor(private path: t.RootNodePath<t.ExportNamedDeclaration>) {}
+
+  get parentPath(): t.NodePath<t.Program> {
+    return this.path.parentPath;
+  }
+
+  replaceWith(id: t.Identifier) {
+    this.path.node.specifiers.push(t.exportSpecifier(id, id));
+    this.path.node.declaration = null;
+  }
+}
+
+class DefaultExportDeclaration implements ExportDeclaration {
+  constructor(private path: t.RootNodePath<t.ExportDefaultDeclaration>) {}
+
+  get parentPath(): t.NodePath<t.Program> {
+    return this.path.parentPath;
+  }
+
+  replaceWith(id: t.Identifier) {
+    this.path.node.declaration = id;
+  }
+}


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 706b75eb..3d5d7861 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
@@ -212,6 +212,111 @@ export interface Data {
   response: {
     value: Value;
   };
+}`
+        }
+      },
+      {
+        description: "an exported function (preserve the export)",
+        setup: {
+          currentFile: `export function [cursor]sayHello() {
+  console.log("hello");
+}
+
+sayHello();`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { sayHello } from "./other-file";
+export { sayHello };
+
+sayHello();`,
+          otherFile: `export function sayHello() {
+  console.log("hello");
+}`
+        }
+      },
+      {
+        description: "an exported interface",
+        setup: {
+          currentFile: `export interface [cursor]Data {
+  value: string;
+}
+
+let someData: Data;`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { Data } from "./other-file";
+export type { Data };
+
+let someData: Data;`,
+          otherFile: `export interface Data {
+  value: string;
+}`
+        }
+      },
+      {
+        description: "an exported type",
+        setup: {
+          currentFile: `export type [cursor]Data = {
+  value: string;
+};
+
+let someData: Data;`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { Data } from "./other-file";
+export type { Data };
+
+let someData: Data;`,
+          otherFile: `export type Data = {
+  value: string;
+};`
+        }
+      },
+      {
+        description: "an exported function (default export)",
+        setup: {
+          currentFile: `export default function [cursor]sayHello() {
+  console.log("hello");
+}
+
+sayHello();`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { sayHello } from "./other-file";
+export default sayHello;
+
+sayHello();`,
+          otherFile: `export function sayHello() {
+  console.log("hello");
+}`
+        }
+      },
+      {
+        description: "an exported interface (default export)",
+        setup: {
+          currentFile: `export default interface [cursor]Data {
+  value: string;
+}
+
+let someData: Data;`,
+          otherFile: "",
+          path: new RelativePath("./other-file.ts")
+        },
+        expected: {
+          currentFile: `import { Data } from "./other-file";
+export default Data;
+
+let someData: Data;`,
+          otherFile: `export interface Data {
+  value: string;
 }`
         }
       }
@@ -237,14 +342,6 @@ export interface Data {
   console.log("hello");
 }
 
-sayHello();`
-      },
-      {
-        description: "if function is directly exported",
-        code: `export function [cursor]sayHello() {
-  console.log("hello");
-}
-
 sayHello();`
       },
       {