V2: Improve testing codemods in connect-migration (#1138)

connectrpc · timostamm · Nov 4, 2024 · Jun 14, 2024 · Jun 25, 2024 · Jun 27, 2024
commit 848dce02bdff055ab46d828a43f2f4d02aff341b
diff --git a/packages/connect-migrate/src/lib/migrate-source-file.spec.ts b/packages/connect-migrate/src/lib/migrate-source-file.spec.ts
@@ -0,0 +1,89 @@
+// Copyright 2021-2024 The Connect Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { updateSourceFileInMemory } from "./migrate-source-files";
+import j from "jscodeshift";
+
+describe("updateSourceFileInMemory", function () {
+  it("should return modified = false for unmodified", () => {
+    const transform: j.Transform = (file, { j }) => {
+      return j(file.source).toSource();
+    };
+    const result = updateSourceFileInMemory(
+      transform,
+      `
+      const foo = 123;
+    `,
+      "foo.ts",
+    );
+    expect(result.modified).toBe(false);
+  });
+  it("should parse js", () => {
+    const transform: j.Transform = (file, { j }) => {
+      return j(file.source)
+        .findVariableDeclarators("foo")
+        .renameTo("bar")
+        .toSource();
+    };
+    const result = updateSourceFileInMemory(
+      transform,
+      `
+      const foo = 123;
+    `,
+      "foo.ts",
+    );
+    expect(result.modified).toBe(true);
+    expect(result.source).toBe(`
+      const bar = 123;
+    `);
+  });
+  it("should parse ts", () => {
+    const transform: j.Transform = (file, { j }) => {
+      return j(file.source)
+        .findVariableDeclarators("foo")
+        .renameTo("bar")
+        .toSource();
+    };
+    const result = updateSourceFileInMemory(
+      transform,
+      `
+      const foo: number = 123;
+    `,
+      "foo.ts",
+    );
+    expect(result.modified).toBe(true);
+    expect(result.source).toBe(`
+      const bar: number = 123;
+    `);
+  });
+  it("should parse tsx", () => {
+    const transform: j.Transform = (file, { j }) => {
+      return j(file.source)
+        .findVariableDeclarators("foo")
+        .renameTo("bar")
+        .toSource();
+    };
+    const result = updateSourceFileInMemory(
+      transform,
+      `
+      const foo = () => (<body>body</body>);
+    `,
+      "foo.tsx",
+    );
+    expect(result.modified).toBe(true);
+    expect(result.source).toBe(`
+      const bar = () => (<body>body</body>);
+    `);
+  });
+});
diff --git a/packages/connect-migrate/src/lib/migrate-source-files.ts b/packages/connect-migrate/src/lib/migrate-source-files.ts
@@ -13,9 +13,9 @@
 // limitations under the License.
 
 import { Scanned } from "./scan";
-import j from "jscodeshift";
 import { Logger, PrintFn } from "./logger";
-import { updateSourceFile } from "./update-source-file";
+import j, { Transform } from "jscodeshift";
+import { readFileSync, writeFileSync } from "node:fs";
 
 interface MigrateSourceFilesResult {
   sourceFileErrors: number;
@@ -57,3 +57,69 @@ export function migrateSourceFiles(
     sourceFileErrors,
   };
 }
+
+const codeshiftTs = j.withParser("ts");
+const codeshiftTsx = j.withParser("tsx");
+const codeshiftBabel = j.withParser("babel");
+
+interface UpdateSourceFileResult {
+  ok: boolean;
+  modified: boolean;
+}
+
+export function updateSourceFile(
+  transform: Transform,
+  path: string,
+  logger?: Logger,
+): UpdateSourceFileResult {
+  logger?.log(`transform ${path}`);
+  try {
+    const source = readFileSync(path, "utf8");
+    const result = updateSourceFileInMemory(transform, source, path);
+    if (!result.modified) {
+      logger?.log(`skipped`);
+      return { ok: true, modified: false };
+    }
+    writeFileSync(path, result.source, "utf-8");
+    logger?.log(`modified`);
+    return { ok: true, modified: true };
+  } catch (e) {
+    logger?.error(`caught error: ${String(e)}`);
+    if (e instanceof Error && e.stack !== undefined) {
+      logger?.error(e.stack);
+    }
+    return { ok: false, modified: false };
+  }
+}
+
+export function updateSourceFileInMemory(
+  transform: Transform,
+  source: string,
+  path: string,
+): { modified: boolean; source: string } {
+  let jscs: j.JSCodeshift;
+  if (path.endsWith(".tsx")) {
+    jscs = codeshiftTsx;
+  } else if (path.endsWith(".ts")) {
+    jscs = codeshiftTs;
+  } else {
+    jscs = codeshiftBabel;
+  }
+  const result = transform(
+    { path, source },
+    {
+      jscodeshift: jscs,
+      j: jscs,
+      stats: () => {},
+      report: () => {},
+    },
+    {},
+  );
+  if (typeof result != "string") {
+    return { modified: false, source };
+  }
+  if (result.trim() === source.trim()) {
+    return { modified: false, source };
+  }
+  return { modified: true, source: result };
+}
diff --git a/packages/connect-migrate/src/lib/update-source-file.ts b/packages/connect-migrate/src/lib/update-source-file.ts
diff --git a/packages/connect-migrate/src/migration.ts b/packages/connect-migrate/src/migration.ts
@@ -15,7 +15,7 @@
 import { Scanned } from "./lib/scan";
 import { CommandLineArgs } from "./arguments";
 import { Logger, PrintFn } from "./lib/logger";
-import { updateSourceFile } from "./lib/update-source-file";
+import { updateSourceFile } from "./lib/migrate-source-files";
 import { writePackageJsonFile } from "./lib/package-json";
 import { runInstall } from "./lib/run";
 

diff --git a/packages/connect-migrate/src/migrations/v0.13.1-transform.spec.ts b/packages/connect-migrate/src/migrations/v0.13.1-transform.spec.ts
@@ -12,67 +12,60 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import jscodeshift from "jscodeshift";
 import transform from "./v0.13.1-transform";
-
-function t(
-  source: string,
-  parser: "tsx" | "babel" | "ts" | "babylon" | "flow" = "tsx",
-) {
-  const shift = jscodeshift.withParser(parser);
-  return transform(
-    { path: "test-file", source },
-    {
-      jscodeshift: shift,
-      j: shift,
-      stats: () => {},
-      report: () => {},
-    },
-    {},
-  );
-}
+import { updateSourceFileInMemory } from "../lib/migrate-source-files";
 
 describe("modify-imports", () => {
   it("should modify import", () => {
-    const got = `import a from "@bufbuild/connect";`;
-    const want = `import a from "@connectrpc/connect";`;
-    expect(t(got)?.trim()).toBe(want.trim());
+    const input = `import a from "@bufbuild/connect";`;
+    const output = `import a from "@connectrpc/connect";`;
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
   it("should modify type imports", () => {
-    const got = `import type {a} from "@bufbuild/connect";`;
-    const want = `import type {a} from "@connectrpc/connect";`;
-    expect(t(got)?.trim()).toBe(want.trim());
+    const input = `import type {a} from "@bufbuild/connect";`;
+    const output = `import type {a} from "@connectrpc/connect";`;
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
   it("should modify import with single quotes", () => {
-    const got = `import a from '@bufbuild/connect';`;
-    const want = `import a from '@connectrpc/connect';`;
-    expect(t(got)?.trim()).toBe(want.trim());
+    const input = `import a from '@bufbuild/connect';`;
+    const output = `import a from '@connectrpc/connect';`;
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
   it("should modify subpath import", () => {
-    const got = `import a from "@bufbuild/connect/protocol";`;
-    const want = `import a from "@connectrpc/connect/protocol";`;
-    expect(t(got)?.trim()).toBe(want.trim());
+    const input = `import a from "@bufbuild/connect/protocol";`;
+    const output = `import a from "@connectrpc/connect/protocol";`;
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
   it("should not modify irrelevant import", () => {
-    const got = `import a from "@foobar/connect";`;
-    expect(t(got)).toBe(got);
+    const input = `import a from "@foobar/connect";`;
+    const output = input;
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
   it("should modify require", () => {
-    const got = `const a = require("@bufbuild/connect");`;
-    const want = `const a = require("@connectrpc/connect");`;
-    expect(t(got)?.trim()).toBe(want.trim());
+    const input = `const a = require("@bufbuild/connect");`;
+    const output = `const a = require("@connectrpc/connect");`;
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
   it("should not modify irrelevant require", () => {
-    const got = `const a = require("@foobar/connect");`;
-    expect(t(got)).toBe(got);
+    const input = `const a = require("@foobar/connect");`;
+    const output = input;
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
   it("should modify require with single quotes", () => {
-    const got = `const a = require('@bufbuild/connect');`;
-    const want = `const a = require('@connectrpc/connect');`;
-    expect(t(got)?.trim()).toBe(want.trim());
+    const input = `const a = require('@bufbuild/connect');`;
+    const output = `const a = require('@connectrpc/connect');`;
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
   it("should parse generics using the ts parser", () => {
-    const got = `
+    const input = `
     async function doSomething() {
       return <UnaryResponse<I, O>>{
         stream: false,
@@ -81,7 +74,7 @@ describe("modify-imports", () => {
         trailer,
       };
     }`;
-    const want = `
+    const output = `
     async function doSomething() {
       return <UnaryResponse<I, O>>{
         stream: false,
@@ -90,6 +83,7 @@ describe("modify-imports", () => {
         trailer,
       };
     }`;
-    expect(t(got, "ts")?.trim()).toBe(want.trim());
+    const result = updateSourceFileInMemory(transform, input, "foo.ts");
+    expect(result.source).toBe(output);
   });
 });
diff --git a/packages/connect-migrate/src/migrations/v0.13.1.ts b/packages/connect-migrate/src/migrations/v0.13.1.ts
@@ -21,8 +21,10 @@ import {
   DependencyMigration,
   migrateDependencies,
 } from "../lib/migrate-dependencies";
-import { updateSourceFile } from "../lib/update-source-file";
-import { migrateSourceFiles } from "../lib/migrate-source-files";
+import {
+  migrateSourceFiles,
+  updateSourceFile,
+} from "../lib/migrate-source-files";
 import { migratePackages } from "../lib/migrate-packages";
 import { migrateLockFiles } from "../lib/migrate-lock-files";