feat: summarize diffs that only change line endings

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,21 +32,33 @@ 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) {
   const suffix = entries.length === 1 ? "file" : "files";
   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] += "<no newline>";
+      }
+      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);

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<string, unknown>[]) {
+async function runAnnotate(workspace: string, entries: Record<string, unknown>[], 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,20 +63,90 @@ 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 {
     await Deno.remove(workspace, { recursive: true });
   }
 });
 
+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();