[cli] fix check --fix crash on overlapping diagnostic edits

Replace `TextDocument.applyEdits()` (which throws on overlapping edits)
with a custom `applyDiagnosticFixes()` that processes each diagnostic
atomically — if any edit from a diagnostic overlaps with an
already-accepted edit, the entire diagnostic is skipped and retried
in the next iteration.

Also ensures `--error-format=json` is passed last to moc so user-provided
flags like `--error-format=human` don't break the fix pipeline.

Made-with: Cursor

Author: Kamirus

cli/CHANGELOG.md

@@ -1,6 +1,7 @@
 # Mops CLI Changelog
 
 ## Next
+- Fix `mops check --fix` crash on overlapping diagnostic edits (e.g., nested function calls)
 
 ## 2.1.0
 - Add `mops check --fix` subcommand (for Motoko files) with autofix logic

cli/helpers/autofix-motoko.ts

@@ -6,12 +6,6 @@ import {
   type TextEdit,
 } from "vscode-languageserver-textdocument";
 
-interface Fix {
-  file: string;
-  code: string;
-  edit: TextEdit;
-}
-
 interface MocSpan {
   file: string;
   line_start: number;
@@ -46,35 +40,127 @@ export function parseDiagnostics(stdout: string): MocDiagnostic[] {
     .filter((d) => d !== null);
 }
 
-function extractFixes(diagnostics: MocDiagnostic[]): Fix[] {
-  const fixes: Fix[] = [];
+interface DiagnosticFix {
+  code: string;
+  edits: TextEdit[];
+}
+
+function extractDiagnosticFixes(
+  diagnostics: MocDiagnostic[],
+): Map<string, DiagnosticFix[]> {
+  const result = new Map<string, DiagnosticFix[]>();
+
   for (const diag of diagnostics) {
+    const editsByFile = new Map<string, TextEdit[]>();
+
     for (const span of diag.spans) {
       if (
         span.suggestion_applicability === "MachineApplicable" &&
         span.suggested_replacement !== null
       ) {
-        fixes.push({
-          file: span.file,
-          code: diag.code,
-          edit: {
-            range: {
-              start: {
-                line: span.line_start - 1,
-                character: span.column_start - 1,
-              },
-              end: {
-                line: span.line_end - 1,
-                character: span.column_end - 1,
-              },
+        const file = resolve(span.file);
+        const edits = editsByFile.get(file) ?? [];
+        edits.push({
+          range: {
+            start: {
+              line: span.line_start - 1,
+              character: span.column_start - 1,
+            },
+            end: {
+              line: span.line_end - 1,
+              character: span.column_end - 1,
             },
-            newText: span.suggested_replacement,
           },
+          newText: span.suggested_replacement,
         });
+        editsByFile.set(file, edits);
       }
     }
+
+    for (const [file, edits] of editsByFile) {
+      const existing = result.get(file) ?? [];
+      existing.push({ code: diag.code, edits });
+      result.set(file, existing);
+    }
+  }
+
+  return result;
+}
+
+type Range = TextEdit["range"];
+
+function normalizeRange(range: Range): Range {
+  const { start, end } = range;
+  if (
+    start.line > end.line ||
+    (start.line === end.line && start.character > end.character)
+  ) {
+    return { start: end, end: start };
+  }
+  return range;
+}
+
+interface OffsetEdit {
+  start: number;
+  end: number;
+  newText: string;
+}
+
+/**
+ * Applies diagnostic fixes to a document, processing each diagnostic as
+ * an atomic unit. If any edit from a diagnostic overlaps with an already-accepted
+ * edit, the entire diagnostic is skipped (picked up in subsequent iterations).
+ * Based on vscode-languageserver-textdocument's TextDocument.applyEdits.
+ */
+function applyDiagnosticFixes(
+  doc: TextDocument,
+  fixes: DiagnosticFix[],
+): { text: string; appliedCodes: string[] } {
+  const acceptedEdits: OffsetEdit[] = [];
+  const appliedCodes: string[] = [];
+
+  for (const fix of fixes) {
+    const offsets: OffsetEdit[] = fix.edits.map((e) => {
+      const range = normalizeRange(e.range);
+      return {
+        start: doc.offsetAt(range.start),
+        end: doc.offsetAt(range.end),
+        newText: e.newText,
+      };
+    });
+
+    const overlaps = offsets.some((o) =>
+      acceptedEdits.some((a) => o.start < a.end && o.end > a.start),
+    );
+    if (overlaps) {
+      continue;
+    }
+
+    acceptedEdits.push(...offsets);
+    appliedCodes.push(fix.code);
+  }
+
+  acceptedEdits.sort((a, b) => a.start - b.start);
+
+  const text = doc.getText();
+  const spans: string[] = [];
+  let lastOffset = 0;
+
+  for (const edit of acceptedEdits) {
+    if (edit.start < lastOffset) {
+      continue;
+    }
+    if (edit.start > lastOffset) {
+      spans.push(text.substring(lastOffset, edit.start));
+    }
+    if (edit.newText.length) {
+      spans.push(edit.newText);
+    }
+    lastOffset = edit.end;
   }
-  return fixes;
+
+  spans.push(text.substring(lastOffset));
+  return { text: spans.join(""), appliedCodes };
 }
 
 const MAX_FIX_ITERATIONS = 10;
@@ -93,47 +179,33 @@ export async function autofixMotoko(
   const fixedFilesCodes = new Map<string, string[]>();
 
   for (let iteration = 0; iteration < MAX_FIX_ITERATIONS; iteration++) {
-    const allFixes: Fix[] = [];
+    const fixesByFile = new Map<string, DiagnosticFix[]>();
 
     for (const file of files) {
       const result = await execa(
         mocPath,
-        [file, "--error-format=json", ...mocArgs],
+        [file, ...mocArgs, "--error-format=json"],
         { stdio: "pipe", reject: false },
       );
 
       const diagnostics = parseDiagnostics(result.stdout);
-      allFixes.push(...extractFixes(diagnostics));
+      for (const [targetFile, fixes] of extractDiagnosticFixes(diagnostics)) {
+        const existing = fixesByFile.get(targetFile) ?? [];
+        existing.push(...fixes);
+        fixesByFile.set(targetFile, existing);
+      }
     }
 
-    if (allFixes.length === 0) {
+    if (fixesByFile.size === 0) {
       break;
     }
 
-    const fixesByFile = new Map<string, Fix[]>();
-    for (const fix of allFixes) {
-      const normalizedPath = resolve(fix.file);
-      const existing = fixesByFile.get(normalizedPath) ?? [];
-      existing.push(fix);
-      fixesByFile.set(normalizedPath, existing);
-    }
-
     let progress = false;
 
     for (const [file, fixes] of fixesByFile) {
       const original = await readFile(file, "utf-8");
       const doc = TextDocument.create(`file://${file}`, "motoko", 0, original);
-
-      let result: string;
-      try {
-        result = TextDocument.applyEdits(
-          doc,
-          fixes.map((f) => f.edit),
-        );
-      } catch (err) {
-        console.warn(`Warning: could not apply fixes to ${file}: ${err}`);
-        continue;
-      }
+      const { text: result, appliedCodes } = applyDiagnosticFixes(doc, fixes);
 
       if (result === original) {
         continue;
@@ -143,9 +215,7 @@ export async function autofixMotoko(
       progress = true;
 
       const existing = fixedFilesCodes.get(file) ?? [];
-      for (const fix of fixes) {
-        existing.push(fix.code);
-      }
+      existing.push(...appliedCodes);
       fixedFilesCodes.set(file, existing);
     }
 

cli/tests/__snapshots__/check-fix.test.ts.snap

@@ -37,9 +37,9 @@ persistent actor {
 `;
 
 exports[`check --fix M0236: fix output 1`] = `
-"Fixed run/M0236.mo (2 fixes: M0236)
+"Fixed run/M0236.mo (1 fix: M0236)
 
-✓ 2 fixes applied to 1 file"
+✓ 1 fix applied to 1 file"
 `;
 
 exports[`check --fix M0237 1`] = `
@@ -64,7 +64,8 @@ exports[`check --fix M0237: fix output 1`] = `
 `;
 
 exports[`check --fix edit-suggestions 1`] = `
-"import Map "mo:core/Map";
+"import Array "mo:core/Array";
+import Map "mo:core/Map";
 import Nat "mo:core/Nat";
 import Text "mo:core/Text";
 import { type Order } "mo:core/Order";
@@ -173,7 +174,7 @@ do {
   ignore data.put(1, "a");
 
   // implicit at the end -> M0236 + M0237
-  ignore data.find(1, );
+  ignore Impl.find(data, 1, );
   ignore data.sort1(); // -> M0236 + M0237
   ignore Impl.sort2(data, ); // no dot suggestion (notSelf), M0237 only
 
@@ -199,20 +200,28 @@ do {
     "John",
   ); // warn M0223 + M0236 + M0237
 };
+
+// Overlapping fixable errors
+do {
+  let ar = [1, 2, 3];
+  let _ = ar.filter(func(x) { x > 0 }).filter(
+    func(x) { x > 0 },
+  );
+};
 "
 `;
 
 exports[`check --fix edit-suggestions: fix output 1`] = `
-"Fixed run/edit-suggestions.mo (41 fixes: M0223, M0236, M0237)
+"Fixed run/edit-suggestions.mo (33 fixes: M0223, M0236, M0237)
 
-✓ 41 fixes applied to 1 file"
+✓ 33 fixes applied to 1 file"
 `;
 
 exports[`check --fix transitive imports: fix output 1`] = `
-"Fixed run/transitive-lib.mo (2 fixes: M0236)
+"Fixed run/transitive-lib.mo (1 fix: M0236)
 Fixed run/transitive-main.mo (1 fix: M0223)
 
-✓ 3 fixes applied to 2 files"
+✓ 2 fixes applied to 2 files"
 `;
 
 exports[`check --fix transitive imports: lib file 1`] = `

cli/tests/check-fix.test.ts

@@ -74,8 +74,8 @@ describe("check --fix", () => {
 
   test("edit-suggestions", async () => {
     await testCheckFix("edit-suggestions.mo", {
-      M0223: 2,
-      M0236: 11,
+      M0223: 3,
+      M0236: 12,
       M0237: 17,
     });
   });
@@ -100,6 +100,25 @@ describe("check --fix", () => {
     expect(countCodes(afterResult.stdout)).toEqual({});
   });
 
+  test("--error-format=human does not break --fix", async () => {
+    const runFilePath = copyFixture("M0223.mo");
+
+    const fixResult = await cli(
+      [
+        "check",
+        runFilePath,
+        "--fix",
+        "--",
+        warningFlags,
+        "--error-format=human",
+      ],
+      { cwd: fixDir },
+    );
+
+    expect(fixResult.stdout).toContain("1 fix applied");
+    expect(readFileSync(runFilePath, "utf-8")).not.toContain("<Nat>");
+  });
+
   test("verbose", async () => {
     const result = await cli(["check", "Ok.mo", "--fix", "--verbose"], {
       cwd: fixDir,

cli/tests/check/fix/edit-suggestions.mo

@@ -1,3 +1,4 @@
+import Array "mo:core/Array";
 import Map "mo:core/Map";
 import Nat "mo:core/Nat";
 import Text "mo:core/Text";
@@ -141,3 +142,12 @@ do {
     "John",
   ); // warn M0223 + M0236 + M0237
 };
+
+// Overlapping fixable errors
+do {
+  let ar = [1, 2, 3];
+  let _ = Array.filter<Nat>(
+    Array.filter<Nat>(ar, func(x) { x > 0 }),
+    func(x) { x > 0 },
+  );
+};