diff --git a/lib/src/commands/scan.ts b/lib/src/commands/scan.ts index 1a53cb3..fe6eb37 100644 --- a/lib/src/commands/scan.ts +++ b/lib/src/commands/scan.ts @@ -12,8 +12,10 @@ import { import chalk from "chalk"; import { asScanLimitInfo, + getLastScannedCommit, initiateClassify, initiateScan, + MAX_SCAN_RENAMES, scanLimitError, uploadCandidatesToS3, } from "../http/scan"; @@ -22,7 +24,12 @@ import { formatDirectoryBreakdown, formatOverLimitMessage, } from "../scan/analyzeDirectories"; -import { readGitContext } from "../scan/git"; +import { + GitContext, + GitRename, + readGitContext, + readRenames, +} from "../scan/git"; import initAPIToken from "../services/apiToken/initAPIToken"; import appContext from "../utils/appContext"; import DittoError, { ErrorType } from "../utils/DittoError"; @@ -211,11 +218,13 @@ export const scan = async ( const token = await initAPIToken(); appContext.setAuthToken(token); + const renamesSinceLastScan = await readRenamesSinceLastScan(gitContext); + const { candidatesSignedS3Url, record: { _id: recordId }, planLimit, - } = await initiateScan(resolvedInput, gitContext); + } = await initiateScan(resolvedInput, gitContext, renamesSinceLastScan); // Fail before the wasted upload when the candidates we already extracted exceed it. if ( @@ -269,3 +278,27 @@ export const scan = async ( await quit(null, 0); } }; + +/** + * What git says moved between the last scan of this repo and HEAD, so a renamed/moved file + * keeps its links instead of reading as a delete plus a create. Empty when there is + * no git context, no earlier scan, or no way to reach the earlier commit. + */ +async function readRenamesSinceLastScan( + gitContext: GitContext | null +): Promise { + if (!gitContext) return []; + + const lastScannedCommit = await getLastScannedCommit(gitContext.repoKey); + if (!lastScannedCommit) return []; + + const renames = await readRenames(gitContext.repoRoot, lastScannedCommit); + if (renames.length > MAX_SCAN_RENAMES) { + logger.writeLine( + logger.warnText( + `[ditto scan] ${renames.length} files moved since the last scan; keeping the history of the first ${MAX_SCAN_RENAMES}\n` + ) + ); + } + return renames; +} diff --git a/lib/src/http/scan.test.ts b/lib/src/http/scan.test.ts index 768e29f..6c7095f 100644 --- a/lib/src/http/scan.test.ts +++ b/lib/src/http/scan.test.ts @@ -1,5 +1,5 @@ import { GitContext } from "../scan/git"; -import { buildInitiateScanBody } from "./scan"; +import { buildInitiateScanBody, MAX_SCAN_RENAMES } from "./scan"; import { ZInitiateScanBodySchema } from "./types"; const context: GitContext = { @@ -71,6 +71,26 @@ describe("buildInitiateScanBody", () => { ]); }); + test("sends the renames git found since the last scan", () => { + const renames = [{ from: "src/Old.tsx", to: "src/New.tsx" }]; + const body = buildInitiateScanBody("/Users/dev/cli", context, renames); + expect(body.renamesSinceLastScan).toEqual(renames); + }); + + test("omits the renames entirely when git found none", () => { + const body = buildInitiateScanBody("/Users/dev/cli", context, []); + expect(body).not.toHaveProperty("renamesSinceLastScan"); + }); + + test("caps a refactor that moved more files than one scan carries", () => { + const renames = Array.from({ length: MAX_SCAN_RENAMES + 5 }, (_, i) => ({ + from: `src/Old${i}.tsx`, + to: `src/New${i}.tsx`, + })); + const body = buildInitiateScanBody("/Users/dev/cli", context, renames); + expect(body.renamesSinceLastScan).toHaveLength(MAX_SCAN_RENAMES); + }); + test("every body satisfies the request schema", () => { for (const c of [null, context, { ...context, branch: null }]) { expect(() => diff --git a/lib/src/http/scan.ts b/lib/src/http/scan.ts index 1306ec7..c0d92d9 100644 --- a/lib/src/http/scan.ts +++ b/lib/src/http/scan.ts @@ -2,12 +2,13 @@ import { DittoScanCandidate } from "@dittowords/text-extract"; import axios, { AxiosError } from "axios"; import { Blob } from "buffer"; import { relative, sep } from "node:path"; -import { GitContext } from "../scan/git"; +import { GitContext, GitRename } from "../scan/git"; import DittoError, { ErrorType } from "../utils/DittoError"; import getHttpClient from "./client"; import { IInitiateScanBody, IInitiateScanResponse, + ZGetLastScannedCommitResponse, ZInitiateScanResponse, } from "./types"; @@ -106,13 +107,19 @@ function scannedScope(root: string | undefined) { }; } +/** + * The most renames one scan carries, matching `MAX_SCAN_RENAMES` in ditto-app + */ +export const MAX_SCAN_RENAMES = 1000; + /** * Builds the `POST /v2/scan` body. Without git context the body is exactly what * the CLI has always sent, so a scan outside a repo is unaffected. */ export function buildInitiateScanBody( path: string, - gitContext?: GitContext | null + gitContext?: GitContext | null, + renamesSinceLastScan: GitRename[] = [] ): IInitiateScanBody { if (!gitContext) return { path }; const root = repoRelativeRoot(path, gitContext.repoRoot); @@ -122,14 +129,20 @@ export function buildInitiateScanBody( gitCommitSha: gitContext.commitSha, gitBranch: gitContext.branch, ...scannedScope(root), + ...(renamesSinceLastScan.length + ? { + renamesSinceLastScan: renamesSinceLastScan.slice(0, MAX_SCAN_RENAMES), + } + : {}), }; } export async function initiateScan( path: string, - gitContext?: GitContext | null + gitContext?: GitContext | null, + renamesSinceLastScan: GitRename[] = [] ): Promise { - const body = buildInitiateScanBody(path, gitContext); + const body = buildInitiateScanBody(path, gitContext, renamesSinceLastScan); try { const httpClient = getHttpClient({}); @@ -145,6 +158,27 @@ export async function initiateScan( } } +/** + * The commit the last scan of this repo read, or `null` when the server has none, + * doesn't know the route, or can't be reached. + */ +export async function getLastScannedCommit( + repoKey: string +): Promise { + try { + const httpClient = getHttpClient({}); + const response = await httpClient.get("/v2/scan/last-scanned-commit", { + params: { repoKey }, + }); + return ( + ZGetLastScannedCommitResponse.parse(response.data).lastScannedCommit ?? + null + ); + } catch { + return null; + } +} + export async function initiateClassify(scanId: string): Promise { try { const httpClient = getHttpClient({}); diff --git a/lib/src/http/types.ts b/lib/src/http/types.ts index cd2ee24..e8d4ca4 100644 --- a/lib/src/http/types.ts +++ b/lib/src/http/types.ts @@ -188,6 +188,9 @@ export type IExportSwiftFileRequest = z.infer; export const ZInitiateScanBodySchema = z.object({ path: z.string(), + renamesSinceLastScan: z + .array(z.object({ from: z.string(), to: z.string() })) + .optional(), repoKey: z.string().optional(), gitCommitSha: z.string().optional(), gitBranch: z.string().nullable().optional(), @@ -209,3 +212,7 @@ export const ZInitiateScanResponse = z.object({ .nullish(), }); export type IInitiateScanResponse = z.infer; + +export const ZGetLastScannedCommitResponse = z.object({ + lastScannedCommit: z.string().nullish(), +}); diff --git a/lib/src/scan/git.test.ts b/lib/src/scan/git.test.ts index 2b0b846..cfb7771 100644 --- a/lib/src/scan/git.test.ts +++ b/lib/src/scan/git.test.ts @@ -7,7 +7,12 @@ import path from "node:path"; * Real git repos, not mocks: this checkout, plus a throwaway repo in the temp * directory for the awkward states. Needs `git` on PATH and a real `.git`. */ -import { normalizeRepoKey, readGitContext, REPO_KEY_PATTERN } from "./git"; +import { + normalizeRepoKey, + readGitContext, + readRenames, + REPO_KEY_PATTERN, +} from "./git"; describe("normalizeRepoKey", () => { const cases: [string, string | null][] = [ @@ -147,3 +152,62 @@ describe("readGitContext", () => { }); }); }); + +describe("readRenames", () => { + let dir: string; + let firstSha: string; + + const run = (...args: string[]) => + execFileSync("git", ["-c", "commit.gpgsign=false", ...args], { + cwd: dir, + stdio: "ignore", + }); + const sha = () => + execFileSync("git", ["rev-parse", "HEAD"], { cwd: dir }).toString().trim(); + const write = (file: string, contents: string) => { + fs.mkdirSync(path.dirname(path.join(dir, file)), { recursive: true }); + fs.writeFileSync(path.join(dir, file), contents); + }; + + beforeAll(() => { + dir = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), "git-mv-")); + run("init", "-b", "main"); + run("config", "user.email", "test@example.com"); + run("config", "user.name", "Test"); + write("src/Old.tsx", "export const label = 'Save';\n"); + write("src/Kept.tsx", "export const other = 'Cancel';\n"); + run("add", "."); + run("commit", "-m", "init"); + firstSha = sha(); + }); + + afterAll(() => fs.rmSync(dir, { recursive: true, force: true })); + + test("no change between two commits gives no renames", async () => { + await expect(readRenames(dir, firstSha)).resolves.toEqual([]); + }); + + test("reads a moved file, and only that file", async () => { + fs.mkdirSync(path.join(dir, "src/nested")); + run("mv", "src/Old.tsx", "src/nested/New.tsx"); + run("commit", "-m", "move"); + + await expect(readRenames(dir, firstSha)).resolves.toEqual([ + { from: "src/Old.tsx", to: "src/nested/New.tsx" }, + ]); + }); + + test("a delete plus an unrelated add is not a rename", async () => { + const before = sha(); + fs.rmSync(path.join(dir, "src/Kept.tsx")); + write("src/Unrelated.tsx", "export const totally = 'Different';\n"); + run("add", "-A"); + run("commit", "-m", "replace"); + + await expect(readRenames(dir, before)).resolves.toEqual([]); + }); + + test("a sha this clone does not have gives no renames", async () => { + await expect(readRenames(dir, "0".repeat(40))).resolves.toEqual([]); + }); +}); diff --git a/lib/src/scan/git.ts b/lib/src/scan/git.ts index 33131a9..dde1330 100644 --- a/lib/src/scan/git.ts +++ b/lib/src/scan/git.ts @@ -117,3 +117,45 @@ export async function readGitContext( dirty: Boolean(status), }; } + +/** + * The `to` and `from` paths for a given file between renames (relative to repo root) + */ +export interface GitRename { + from: string; + to: string; +} + +/** + * Reads the files git says were renamed between `previousSha` and `HEAD`, using + * git's similarity detection. A copy is left out. + * + * @returns `[]` when the diff fails, as it does on a shallow + * clone or with a sha this clone doesn't have. + */ +export async function readRenames( + repoRoot: string, + previousSha: string +): Promise { + const out = await git( + [ + "diff", + "-M", + "--diff-filter=R", + "--name-status", + "-z", + previousSha, + "HEAD", + ], + repoRoot + ); + if (!out) return []; + + // `-z` gives NUL-separated fields: a status, then the `from` and `to` paths. + const fields = out.split("\0").filter(Boolean); + const renames: GitRename[] = []; + for (let i = 0; i + 2 < fields.length; i += 3) { + renames.push({ from: fields[i + 1], to: fields[i + 2] }); + } + return renames; +} diff --git a/package.json b/package.json index ff713df..15021f9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@dittowords/cli", - "version": "5.9.0", + "version": "5.9.1", "description": "Command Line Interface for Ditto (dittowords.com).", "license": "MIT", "main": "bin/ditto.js",