Broken imports after moving files

loader/src/serdes/renameFileImports.spec.ts

@@ -0,0 +1,110 @@
+import { strict as assert } from "assert";
+import { parse } from "yaml";
+import { join } from "path";
+import { mkdtempSync, mkdirSync, writeFileSync, renameSync, rmSync } from "fs";
+import { tmpdir } from "os";
+import { codeNodeInstance } from "@flyde/core";
+import { createServerReferencedNodeFinder } from "../resolver/server/findReferencedNodeServer";
+import { getFileImportRenameEdits, FileRename } from "./renameFileImports";
+
+const root = join(process.cwd(), "rename-test");
+const flowPath = join(root, "Main.flyde");
+const flow = (path: string, type = "file") => `# flow comment\nnode:\n  id: Main\n  instances:\n    - source:\n        type: ${type}\n        data: ${JSON.stringify(path)} # import comment\n      config: { data: './Node.flyde.ts' }\n`;
+const move = (oldPath: string, newPath: string): FileRename => ({
+  oldPath: join(root, oldPath), newPath: join(root, newPath),
+});
+const rewrite = (text: string, renames: FileRename[], path = flowPath) =>
+  getFileImportRenameEdits(text, path, renames)
+    .sort((a, b) => b.start - a.start)
+    .reduce((result, edit) => result.slice(0, edit.start) + edit.text + result.slice(edit.end), text);
+const source = (text: string) => parse(text).node.instances[0].source.data;
+
+describe("file import renames", () => {
+  it("resolves a code node after moving its file", () => {
+    const directory = mkdtempSync(join(tmpdir(), "flyde-rename-"));
+    try {
+      const oldPath = join(directory, "Node.flyde.js");
+      const newPath = join(directory, "lib/Node.flyde.js");
+      const mainPath = join(directory, "Main.flyde");
+      writeFileSync(oldPath, `exports.Node = { id: 'Node', inputs: {}, outputs: {}, run() {} };`);
+      const finder = createServerReferencedNodeFinder(mainPath);
+      const instance = codeNodeInstance("node", "Node", { type: "file", data: "./Node.flyde.js" });
+      assert.equal(finder(instance).id, "Node");
+      const updated = rewrite(flow("./Node.flyde.js"), [{ oldPath, newPath }], mainPath);
+      mkdirSync(join(directory, "lib"));
+      renameSync(oldPath, newPath);
+      assert.throws(() => finder(instance));
+      instance.source.data = source(updated);
+      assert.equal(finder(instance).id, "Node");
+    } finally {
+      rmSync(directory, { recursive: true, force: true });
+    }
+  });
+  for (const extension of ["flyde", "flyde.ts", "flyde.js"]) {
+    it(`updates a moved ${extension} import and preserves surrounding text`, () => {
+      const text = flow(`./Node.${extension}`);
+      assert.equal(rewrite(text, [move(`Node.${extension}`, `lib/Node.${extension}`)]),
+        text.replace(`"./Node.${extension}"`, `"./lib/Node.${extension}"`));
+    });
+  }
+
+  it("rebases a moved importing flow", () => {
+    assert.equal(source(rewrite(flow("./Node.flyde.ts"), [move("Main.flyde", "flows/Main.flyde")])),
+      "../Node.flyde.ts");
+  });
+
+  it("updates imports when their containing folder moves", () => {
+    assert.equal(source(rewrite(flow("./lib/Node.flyde.ts"), [move("lib", "shared")])),
+      "./shared/Node.flyde.ts");
+  });
+
+  it("leaves relative paths intact when both files move together", () => {
+    const text = flow("./Node.flyde.ts");
+    assert.equal(rewrite(text, [move("lib", "shared")], join(root, "lib/Main.flyde")), text);
+  });
+
+  it("applies batch moves simultaneously", () => {
+    assert.equal(source(rewrite(flow("./Node.flyde.ts"), [
+      move("Node.flyde.ts", "lib/Node.flyde.ts"),
+      move("lib", "other"),
+      move("Main.flyde", "flows/Main.flyde"),
+    ])), "../lib/Node.flyde.ts");
+  });
+
+  it("uses the most specific move and respects folder boundaries", () => {
+    assert.equal(source(rewrite(flow("./lib/Node.flyde.ts"), [
+      move("lib", "shared"), move("lib/Node.flyde.ts", "Node.flyde.ts"),
+    ])), "./Node.flyde.ts");
+    const text = flow("./library/Node.flyde.ts");
+    assert.equal(rewrite(text, [move("lib", "shared")]), text);
+  });
+
+  it("visits current and legacy inline nodes without changing configuration", () => {
+    const text = `node:\n  instances:\n    - source:\n        type: inline\n        data:\n          instances:\n            - source: { type: file, data: './Node.flyde.ts' }\n    - node:\n        instances:\n          - source: { type: file, data: './Node.flyde.ts' }\n      config: { source: { type: file, data: './Node.flyde.ts' } }\n`;
+    const result = parse(rewrite(text, [move("Node.flyde.ts", "lib/Node.flyde.ts")]));
+    assert.equal(result.node.instances[0].source.data.instances[0].source.data, "./lib/Node.flyde.ts");
+    assert.equal(result.node.instances[1].node.instances[0].source.data, "./lib/Node.flyde.ts");
+    assert.equal(result.node.instances[1].config.source.data, "./Node.flyde.ts");
+  });
+
+  it("updates legacy local import keys and leaves package imports intact", () => {
+    const text = `imports:\n  './Node.flyde.ts': [Node] # keep\n  '@flyde/nodes': [Value]\nnode: { instances: [] }\n`;
+    assert.equal(rewrite(text, [move("Node.flyde.ts", "lib/Node.flyde.ts")]),
+      text.replace("'./Node.flyde.ts'", '"./lib/Node.flyde.ts"'));
+  });
+
+  it("leaves package, custom, self, and unrelated file sources intact", () => {
+    for (const type of ["package", "custom", "self"]) {
+      const text = flow("./Node.flyde.ts", type);
+      assert.equal(rewrite(text, [move("Node.flyde.ts", "lib/Node.flyde.ts")]), text);
+    }
+    const text = flow("./Other.flyde.ts");
+    assert.equal(rewrite(text, [move("Node.flyde.ts", "lib/Node.flyde.ts")]), text);
+  });
+
+  it("ignores invalid YAML and non-string paths", () => {
+    for (const text of ["node: [", "node: null", "node: { instances: [{ source: { type: file, data: 123 } }] }"]) {
+      assert.deepEqual(getFileImportRenameEdits(text, flowPath, [move("lib", "shared")]), []);
+    }
+  });
+});

loader/src/serdes/renameFileImports.ts

@@ -0,0 +1,94 @@
+import { dirname, isAbsolute, relative, resolve, sep } from "path";
+import { isMap, isScalar, isSeq, parseDocument } from "yaml";
+
+export interface FileRename {
+  oldPath: string;
+  newPath: string;
+}
+
+export interface ImportPathEdit {
+  start: number;
+  end: number;
+  text: string;
+}
+
+export function getFileImportRenameEdits(
+  contents: string,
+  flowPath: string,
+  renames: readonly FileRename[]
+): ImportPathEdit[] {
+  const document = parseDocument(contents);
+  if (document.errors.length || !isMap(document.contents)) {
+    return [];
+  }
+
+  const moves = renames.map(({ oldPath, newPath }) => ({
+    oldPath: resolve(oldPath),
+    newPath: resolve(newPath),
+  })).sort((a, b) => b.oldPath.length - a.oldPath.length);
+  const movedPath = (path: string) => {
+    const move = moves.find(({ oldPath }) =>
+      path === oldPath || path.startsWith(oldPath + sep)
+    );
+    return move ? move.newPath + path.slice(move.oldPath.length) : path;
+  };
+
+  const oldDirectory = dirname(resolve(flowPath));
+  const newDirectory = dirname(movedPath(resolve(flowPath)));
+  const edits: ImportPathEdit[] = [];
+  const update = (value: unknown) => {
+    if (!isScalar(value) || typeof value.value !== "string" || !value.range || value.anchor) {
+      return;
+    }
+    const oldTarget = resolve(oldDirectory, value.value);
+    const newTarget = movedPath(oldTarget);
+    if (oldTarget === newTarget && oldDirectory === newDirectory) {
+      return;
+    }
+    const newValue = isAbsolute(value.value)
+      ? newTarget
+      : relative(newDirectory, newTarget).split(sep).join("/");
+    const importPath = isAbsolute(newValue) || newValue.startsWith(".")
+      ? newValue
+      : `./${newValue}`;
+    if (importPath !== value.value) {
+      edits.push({ start: value.range[0], end: value.range[1], text: JSON.stringify(importPath) });
+    }
+  };
+
+  const visitNode = (node: unknown) => {
+    if (!isMap(node)) {
+      return;
+    }
+    const instances = node.get("instances", true);
+    if (!isSeq(instances)) {
+      return;
+    }
+    for (const instance of instances.items) {
+      if (!isMap(instance)) {
+        continue;
+      }
+      const source = instance.get("source", true);
+      if (isMap(source)) {
+        if (source.get("type") === "file") {
+          update(source.get("data", true));
+        } else if (source.get("type") === "inline") {
+          visitNode(source.get("data", true));
+        }
+      }
+      visitNode(instance.get("node", true));
+    }
+  };
+  visitNode(document.contents.get("node", true));
+
+  const imports = document.contents.get("imports", true);
+  if (isMap(imports)) {
+    for (const { key } of imports.items) {
+      if (isScalar(key) && typeof key.value === "string" &&
+        (key.value.startsWith(".") || isAbsolute(key.value) || /\.flyde(?:\.[jt]s)?$/.test(key.value))) {
+        update(key);
+      }
+    }
+  }
+  return edits;
+}

loader/src/server.ts

@@ -3,9 +3,10 @@ export * from "./resolver";
 export * from "./serdes";
 export * from "./runtime";
 export * from "./run-flow";
+export * from "./serdes/renameFileImports";
 
 // Server-specific exports
 export { createServerReferencedNodeFinder } from "./resolver/server/findReferencedNodeServer";
 export * from "./resolver/server/resolveImportablePaths";
 export { resolveFlowByPath } from "./resolver/server/resolveFlowServer";
-export { resolveCodeNodeDependencies, isCodeNodePath } from "./resolver/server/serverUtils";
+export { resolveCodeNodeDependencies, isCodeNodePath } from "./resolver/server/serverUtils";

vscode/src/extension.ts

@@ -15,6 +15,7 @@ import { activateReporter, reportEvent, analytics } from "./analytics";
 import { showFirstRunPrivacyNotice, showPrivacySettings } from "./privacyNotice";
 
 import { Template, getTemplates, scaffoldTemplate } from "./templateUtils";
+import { registerFileImportRenames } from "./renameFileImports";
 
 // the application insights key (also known as instrumentation key)
 
@@ -29,6 +30,7 @@ let process: execa.ExecaChildProcess | undefined;
 const FLYDE_DEFAULT_SERVER_PORT = 8545;
 
 export function activate(context: vscode.ExtensionContext) {
+  context.subscriptions.push(registerFileImportRenames());
   // eslint-disable-next-line @typescript-eslint/naming-convention
   const { Uri } = vscode;
   const { fs } = vscode.workspace;

vscode/src/renameFileImports.ts

@@ -0,0 +1,57 @@
+import * as vscode from "vscode";
+import { getFileImportRenameEdits } from "@flyde/loader/dist/server";
+
+export function registerFileImportRenames(): vscode.Disposable {
+  return vscode.workspace.onWillRenameFiles((event) => {
+    event.waitUntil(createFileImportRenameEdit(event));
+  });
+}
+
+export async function createFileImportRenameEdit(
+  event: vscode.FileWillRenameEvent
+): Promise<vscode.WorkspaceEdit> {
+  const edit = new vscode.WorkspaceEdit();
+  const renames = event.files
+    .filter(({ oldUri, newUri }) => oldUri.scheme === "file" && newUri.scheme === "file")
+    .map(({ oldUri, newUri }) => ({ oldPath: oldUri.fsPath, newPath: newUri.fsPath }));
+  if (!renames.length || event.token.isCancellationRequested) {
+    return edit;
+  }
+
+  const uris = await vscode.workspace.findFiles("**/*.flyde", "**/node_modules/**", undefined, event.token);
+  const candidates = new Map(uris.map(uri => [uri.toString(), uri]));
+  for (const document of vscode.workspace.textDocuments) {
+    if (document.uri.path.endsWith(".flyde")) {
+      candidates.set(document.uri.toString(), document.uri);
+    }
+  }
+  for (const { oldUri } of event.files) {
+    if (oldUri.path.endsWith(".flyde")) {
+      candidates.set(oldUri.toString(), oldUri);
+    }
+  }
+
+  const versions = new Map<vscode.TextDocument, number>();
+  for (const uri of candidates.values()) {
+    if (event.token.isCancellationRequested) {
+      return new vscode.WorkspaceEdit();
+    }
+    if (uri.scheme !== "file") {
+      continue;
+    }
+    const document = await vscode.workspace.openTextDocument(uri);
+    const changes = getFileImportRenameEdits(document.getText(), uri.fsPath, renames);
+    if (changes.length) {
+      versions.set(document, document.version);
+      edit.set(uri, changes.map(change => vscode.TextEdit.replace(
+        new vscode.Range(document.positionAt(change.start), document.positionAt(change.end)),
+        change.text
+      )));
+    }
+  }
+  if (event.token.isCancellationRequested ||
+    [...versions].some(([document, version]) => document.version !== version)) {
+    return new vscode.WorkspaceEdit();
+  }
+  return edit;
+}

vscode/src/test/renameFileImports.test.ts

@@ -0,0 +1,87 @@
+import * as assert from "assert";
+import * as vscode from "vscode";
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "fs";
+import { tmpdir } from "os";
+import { join } from "path";
+import { createFileImportRenameEdit, registerFileImportRenames } from "../renameFileImports";
+
+suite("File import rename edits", () => {
+  let directory: string;
+  let cancellation: vscode.CancellationTokenSource;
+
+  setup(() => {
+    directory = mkdtempSync(join(tmpdir(), "flyde-vscode-rename-"));
+    cancellation = new vscode.CancellationTokenSource();
+  });
+
+  teardown(async () => {
+    cancellation.dispose();
+    for (const document of vscode.workspace.textDocuments) {
+      if (document.uri.fsPath.startsWith(directory) && document.isDirty) {
+        await document.save();
+      }
+    }
+    await vscode.commands.executeCommand("workbench.action.closeAllEditors");
+    rmSync(directory, { recursive: true, force: true });
+  });
+
+  async function prepare(importPath = "./Node.flyde.ts") {
+    const uri = vscode.Uri.file(join(directory, "Main.flyde"));
+    writeFileSync(uri.fsPath, `node:\n  instances:\n    - source: { type: file, data: '${importPath}' } # keep\n`);
+    return vscode.workspace.openTextDocument(uri);
+  }
+
+  function event(oldPath: string, newPath: string): vscode.FileWillRenameEvent {
+    return {
+      files: [{ oldUri: vscode.Uri.file(join(directory, oldPath)), newUri: vscode.Uri.file(join(directory, newPath)) }],
+      token: cancellation.token,
+      waitUntil() {},
+    };
+  }
+
+  test("updates a dirty document without saving its buffer", async () => {
+    const document = await prepare();
+    const dirtyEdit = new vscode.WorkspaceEdit();
+    dirtyEdit.insert(document.uri, new vscode.Position(0, 0), "# unsaved\n");
+    assert.ok(await vscode.workspace.applyEdit(dirtyEdit));
+    const edit = await createFileImportRenameEdit(event("Node.flyde.ts", "lib/Node.flyde.ts"));
+    assert.ok(await vscode.workspace.applyEdit(edit));
+    assert.ok(document.getText().startsWith("# unsaved\n"));
+    assert.ok(document.getText().includes('data: "./lib/Node.flyde.ts"'));
+    assert.ok(document.getText().includes("# keep"));
+    assert.ok(document.isDirty);
+  });
+
+  test("rebases a moved flow using its original URI", async () => {
+    const document = await prepare();
+    const edit = await createFileImportRenameEdit(event("Main.flyde", "flows/Main.flyde"));
+    assert.equal(edit.get(document.uri).length, 1);
+    assert.ok(await vscode.workspace.applyEdit(edit));
+    assert.ok(document.getText().includes('data: "../Node.flyde.ts"'));
+  });
+
+  test("keeps a code import working through a real file move", async () => {
+    const document = await prepare();
+    const oldUri = vscode.Uri.file(join(directory, "Node.flyde.ts"));
+    const newUri = vscode.Uri.file(join(directory, "lib/Node.flyde.ts"));
+    writeFileSync(oldUri.fsPath, "export const Node = {};\n");
+    mkdirSync(join(directory, "lib"));
+    const edit = new vscode.WorkspaceEdit();
+    edit.renameFile(oldUri, newUri);
+    const registration = registerFileImportRenames();
+    try {
+      assert.ok(await vscode.workspace.applyEdit(edit));
+    } finally {
+      registration.dispose();
+    }
+    assert.ok(document.getText().includes('data: "./lib/Node.flyde.ts"'));
+    assert.equal((await vscode.workspace.fs.stat(newUri)).type, vscode.FileType.File);
+  });
+
+  test("discards edits for a cancelled rename", async () => {
+    await prepare();
+    cancellation.cancel();
+    const edit = await createFileImportRenameEdit(event("Node.flyde.ts", "lib/Node.flyde.ts"));
+    assert.equal(edit.size, 0);
+  });
+});