Be more defensive mapping diagnostics. (#1064)

The document may have changed in the meantime so notice when lines don't
make sense. I've also added a specific check for the scenario where
diagnostics have to < from.

Closes #1062

Author: microbit-matt-hillsdon

src/editor/codemirror/language-server/diagnostics.ts

@@ -24,7 +24,7 @@ export const diagnosticsMapping = (
       let from = positionToOffset(document, range.start);
       let to = positionToOffset(document, range.end);
       // Skip if we can't map to the current document.
-      if (from !== undefined && to !== undefined) {
+      if (from !== undefined && to !== undefined && to >= from) {
         return {
           from,
           to,

src/editor/codemirror/language-server/positions.test.ts

@@ -0,0 +1,102 @@
+import { Text } from "@codemirror/state";
+import { positionToOffset } from "./positions";
+
+describe("positionToOffset", () => {
+  it("empty doc", () => {
+    const doc = Text.of([""]);
+    expect(
+      positionToOffset(doc, {
+        line: 0,
+        character: 0,
+      })
+    ).toEqual(0);
+
+    expect(
+      positionToOffset(doc, {
+        line: 1,
+        character: 0,
+      })
+    ).toBeUndefined();
+
+    expect(
+      positionToOffset(doc, {
+        line: 0,
+        character: 1,
+      })
+    ).toBeUndefined();
+  });
+
+  it("1 char doc", () => {
+    const doc = Text.of(["x"]);
+    expect(
+      positionToOffset(doc, {
+        line: 0,
+        character: 0,
+      })
+    ).toEqual(0);
+
+    expect(
+      positionToOffset(doc, {
+        line: 0,
+        character: 1,
+      })
+    ).toEqual(1);
+
+    expect(
+      positionToOffset(doc, {
+        line: 0,
+        character: 2,
+      })
+    ).toBeUndefined();
+  });
+
+  it("2 line doc", () => {
+    const doc = Text.of(["x", "y"]);
+    expect(
+      positionToOffset(doc, {
+        line: 0,
+        character: 0,
+      })
+    ).toEqual(0);
+
+    expect(
+      positionToOffset(doc, {
+        line: 1,
+        character: 0,
+      })
+    ).toEqual(2);
+
+    expect(
+      positionToOffset(doc, {
+        line: 1,
+        character: 1,
+      })
+    ).toEqual(3);
+
+    expect(
+      positionToOffset(doc, {
+        line: 1,
+        character: 2,
+      })
+    ).toBeUndefined();
+  });
+
+  it("maps to incorrect line", () => {
+    const doc = Text.of(["hello", "there"]);
+    // Initial checks to confirm boundary
+    expect(
+      positionToOffset(doc, {
+        line: 0,
+        character: 5,
+      })
+    ).toEqual(5);
+    expect(doc.sliceString(0, 5)).toEqual("hello");
+    // Actual test that goes one too far
+    expect(
+      positionToOffset(doc, {
+        line: 0,
+        character: 6,
+      })
+    ).toBeUndefined();
+  });
+});

src/editor/codemirror/language-server/positions.ts

@@ -15,8 +15,11 @@ export const positionToOffset = (
   if (position.line >= document.lines) {
     return undefined;
   }
-  const offset = document.line(position.line + 1).from + position.character;
-  if (offset > document.length) return;
+  const line = document.line(position.line + 1);
+  const offset = line.from + position.character;
+  if (offset > line.to) {
+    return undefined;
+  }
   return offset;
 };