diff --git a/scripts/annotate.mjs b/scripts/annotate.mjs index 0b10ded..60ab95d 100644 --- a/scripts/annotate.mjs +++ b/scripts/annotate.mjs @@ -6,6 +6,9 @@ import fs from "node:fs"; import path from "node:path"; const MAX_ANNOTATION_MESSAGE_LENGTH = 4000; +const LINE_ENDINGS_MESSAGE = "Text differed by line endings."; +const WINDOWS_LINE_ENDINGS_HINT = + "Git on Windows runners checks out files with CRLF line endings, so consider only running this action on Linux: https://github.com/dprint/check#windows-line-endings"; const jsonlPath = process.argv[2]; if (jsonlPath == null) { @@ -29,11 +32,13 @@ const entries = fs.readFileSync(jsonlPath, "utf8") for (const entry of entries) { const relativePath = toRelativePath(entry.file, workspace); - const diff = entry.diff == null ? "File is not valid utf-8." : makePrintable(entry.diff); + // a diff that only changes line endings is every line of the file, so + // summarize it like dprint's default output does + const lineEndings = entry.diff == null ? undefined : getLineEndingsOnlyChange(entry.diff); console.log(`from ${relativePath}:`); - console.log(diff); + console.log(describeDiff(entry.diff, lineEndings)); console.log("--"); - console.log(annotation(relativePath, entry.diff)); + console.log(annotation(relativePath, entry.diff, lineEndings)); } if (entries.length > 0) { @@ -41,9 +46,19 @@ if (entries.length > 0) { console.log(`Found ${entries.length} not formatted ${suffix}. Run dprint fmt to fix.`); } +/** Gets the diff in a readable form, or a short message when there's nothing useful to show. */ +function describeDiff(diff, lineEndings) { + if (diff == null) { + return "File is not valid utf-8."; + } + return lineEndings != null ? LINE_ENDINGS_MESSAGE : makePrintable(diff); +} + /** Builds the `::error` workflow command for a file that isn't formatted. */ -function annotation(relativePath, diff) { - const range = firstHunkRange(diff); +function annotation(relativePath, diff, lineEndings) { + // the whole file differs when only the line endings do, so point at the top + // of it rather than highlighting every line + const range = lineEndings != null ? { line: 1, endLine: 1 } : firstHunkRange(diff); const properties = { file: relativePath, line: range.line, title: "dprint" }; if (range.endLine !== range.line) { properties.endLine = range.endLine; @@ -52,7 +67,12 @@ function annotation(relativePath, diff) { .map(([key, value]) => `${key}=${escapeProperty(String(value))}`) .join(","); let message = "File is not formatted. Run `dprint fmt` to fix."; - if (diff != null) { + if (lineEndings != null) { + message += "\n" + LINE_ENDINGS_MESSAGE; + if (lineEndings.original === "crlf" && process.env.RUNNER_OS === "Windows") { + message += " " + WINDOWS_LINE_ENDINGS_HINT; + } + } else if (diff != null) { message += "\n" + truncate(makePrintable(stripDiffHeader(diff)), MAX_ANNOTATION_MESSAGE_LENGTH); } return `::error ${propertiesText}::${escapeMessage(message)}`; @@ -74,6 +94,46 @@ function firstHunkRange(diff) { return { line, endLine: line + count - 1 }; } +/** + * Gets the line ending the original file had when a unified diff only changes + * line endings, which is found by rebuilding both sides of the diff and + * comparing them without carriage returns. Returns `undefined` otherwise. + */ +function getLineEndingsOnlyChange(diff) { + const oldLines = []; + const newLines = []; + let originalHasCarriageReturn = false; + // the sides the previous line was added to, so a "no newline at end of + // file" marker can be applied to it + let previousSides = []; + let inHunk = false; + for (const line of diff.split("\n")) { + if (line.startsWith("@@")) { + inHunk = true; + continue; + } + if (!inHunk) { + continue; + } + if (line.startsWith("\\")) { + for (const side of previousSides) { + side[side.length - 1] += ""; + } + continue; + } + const sign = line[0]; + previousSides = sign === "-" ? [oldLines] : sign === "+" ? [newLines] : sign === " " ? [oldLines, newLines] : []; + if (sign === "-" && line.endsWith("\r")) { + originalHasCarriageReturn = true; + } + for (const side of previousSides) { + side.push(line.slice(1).replace(/\r$/, "")); + } + } + const isOnlyLineEndings = oldLines.length === newLines.length && oldLines.every((line, i) => line === newLines[i]); + return isOnlyLineEndings ? { original: originalHasCarriageReturn ? "crlf" : "lf" } : undefined; +} + function toRelativePath(filePath, workspace) { const relativePath = path.relative(workspace, filePath); const isOutside = relativePath === ".." || relativePath.startsWith(`..${path.sep}`) || path.isAbsolute(relativePath); diff --git a/scripts/annotate_test.ts b/scripts/annotate_test.ts index a7bf3f8..0a40f74 100644 --- a/scripts/annotate_test.ts +++ b/scripts/annotate_test.ts @@ -3,12 +3,12 @@ import { fromFileUrl } from "jsr:@std/path@1"; const scriptPath = fromFileUrl(new URL("./annotate.mjs", import.meta.url)); -async function runAnnotate(workspace: string, entries: Record[]) { +async function runAnnotate(workspace: string, entries: Record[], runnerOs = "Linux") { const jsonlPath = `${workspace}/dprint-check.jsonl`; await Deno.writeTextFile(jsonlPath, entries.map((entry) => JSON.stringify(entry)).join("\n") + "\n"); const output = await new Deno.Command("node", { args: [scriptPath, jsonlPath], - env: { GITHUB_WORKSPACE: workspace }, + env: { GITHUB_WORKSPACE: workspace, RUNNER_OS: runnerOs }, }).output(); return { code: output.code, @@ -63,13 +63,13 @@ Deno.test("makes carriage returns visible and escapes the annotation", async () try { const result = await runAnnotate(workspace, [{ file: `${workspace}/100%/a,b:c.json`, - diff: "--- original\n+++ formatted\n@@ -1 +1 @@\n-{ \"ok\": true }\r\n+{ \"ok\": true }\n", + diff: "--- original\n+++ formatted\n@@ -1 +1 @@\n-{\"ok\":true}\r\n+{ \"ok\": true }\n", }]); assertEquals(result.code, 0); - assertStringIncludes(result.stdout, "-{ \"ok\": true }\\r\n+{ \"ok\": true }\n"); + assertStringIncludes(result.stdout, "-{\"ok\":true}\\r\n+{ \"ok\": true }\n"); assertStringIncludes( result.stdout, - "::error file=100%25/a%2Cb%3Ac.json,line=1,title=dprint::File is not formatted. Run `dprint fmt` to fix.%0A@@ -1 +1 @@%0A-{ \"ok\": true }\\r%0A+{ \"ok\": true }", + "::error file=100%25/a%2Cb%3Ac.json,line=1,title=dprint::File is not formatted. Run `dprint fmt` to fix.%0A@@ -1 +1 @@%0A-{\"ok\":true}\\r%0A+{ \"ok\": true }", ); assertStringIncludes(result.stdout, "Found 1 not formatted file."); } finally { @@ -77,6 +77,76 @@ Deno.test("makes carriage returns visible and escapes the annotation", async () } }); +Deno.test("summarizes a diff that only changes line endings", async () => { + const workspace = await Deno.makeTempDir(); + try { + const result = await runAnnotate(workspace, [{ + file: `${workspace}/crlf.md`, + diff: "--- original\n+++ formatted\n@@ -1,3 +1,3 @@\n-# Title\r\n-\r\n-Some text.\r\n+# Title\n+\n+Some text.\n", + }, { + // a file without a final newline on both sides still only differs by line endings + file: `${workspace}/no-newline.md`, + diff: + "--- original\n+++ formatted\n@@ -1,2 +1,2 @@\n-a\r\n-b\n\\ No newline at end of file\n+a\n+b\n\\ No newline at end of file\n", + }, { + // adding a final newline is a real change, not a line ending one + file: `${workspace}/missing-newline.md`, + diff: "--- original\n+++ formatted\n@@ -1 +1 @@\n-a\n\\ No newline at end of file\n+a\n", + }]); + assertEquals(result.stderr, ""); + assertEquals(result.code, 0); + assertEquals(result.stdout.split("\n"), [ + "from crlf.md:", + "Text differed by line endings.", + "--", + "::error file=crlf.md,line=1,title=dprint::File is not formatted. Run `dprint fmt` to fix.%0AText differed by line endings.", + "from no-newline.md:", + "Text differed by line endings.", + "--", + "::error file=no-newline.md,line=1,title=dprint::File is not formatted. Run `dprint fmt` to fix.%0AText differed by line endings.", + "from missing-newline.md:", + "--- original", + "+++ formatted", + "@@ -1 +1 @@", + "-a", + "\\ No newline at end of file", + "+a", + "--", + "::error file=missing-newline.md,line=1,title=dprint::File is not formatted. Run `dprint fmt` to fix.%0A@@ -1 +1 @@%0A-a%0A\\ No newline at end of file%0A+a", + "Found 3 not formatted files. Run dprint fmt to fix.", + "", + ]); + } finally { + await Deno.remove(workspace, { recursive: true }); + } +}); + +Deno.test("recommends only running on linux when a windows checkout has crlf line endings", async () => { + const workspace = await Deno.makeTempDir(); + try { + const result = await runAnnotate(workspace, [{ + file: `${workspace}/crlf.md`, + diff: "--- original\n+++ formatted\n@@ -1 +1 @@\n-a\r\n+a\n", + }, { + // the config wants crlf, so the runner's checkout isn't the problem + file: `${workspace}/lf.md`, + diff: "--- original\n+++ formatted\n@@ -1 +1 @@\n-a\n+a\r\n", + }], "Windows"); + assertEquals(result.stderr, ""); + assertEquals(result.code, 0); + assertStringIncludes( + result.stdout, + "::error file=crlf.md,line=1,title=dprint::File is not formatted. Run `dprint fmt` to fix.%0AText differed by line endings. Git on Windows runners checks out files with CRLF line endings, so consider only running this action on Linux: https://github.com/dprint/check#windows-line-endings\n", + ); + assertStringIncludes( + result.stdout, + "::error file=lf.md,line=1,title=dprint::File is not formatted. Run `dprint fmt` to fix.%0AText differed by line endings.\n", + ); + } finally { + await Deno.remove(workspace, { recursive: true }); + } +}); + Deno.test("handles a file that isn't valid utf-8 and files outside the workspace", async () => { const workspace = await Deno.makeTempDir(); const elsewhere = await Deno.makeTempDir();