Handle CRLF in multi line attributes (#148)

Author: maskmaster

packages/analyzer/src/analyzer.ts

@@ -154,8 +154,8 @@ export class Analyzer {
 
 export function parserErrorToAnalyzerIssue(error: ParserError): AnalyzerIssue {
   return {
-    start: error.range.start,
-    end: error.range.end,
+    start: error.start,
+    end: error.end,
     message: error.description,
     name: "knuckles/parser",
     severity: AnalyzerSeverity.Error,

packages/language-server/src/features/diagnostics.ts

@@ -35,8 +35,10 @@ function translateIssueToDiagnostic(
   const start = issue.start ?? Position.fromOffset(0, text);
   const end = issue.end ?? Position.fromOffset(start.offset + 1, text);
   const range = vscode.Range.create(
-    vscode.Position.create(start.line, start.column),
-    vscode.Position.create(end.line, end.column),
+    start.line,
+    start.column,
+    end.line,
+    end.column,
   );
   const severity = {
     [AnalyzerSeverity.Error]: vscode.DiagnosticSeverity.Error,

packages/location/src/position.spec.ts

@@ -38,4 +38,12 @@ describe("Position", () => {
       offset: 6,
     });
   });
+
+  it("Converts offset in middle of CRLF to line and column", () => {
+    const position = Position.fromOffset(9, "foo\r\nbar\r\n");
+    expect(position).toMatchObject({
+      line: 2,
+      column: 0,
+    });
+  });
 });

packages/location/src/position.ts

@@ -19,7 +19,7 @@ export default class Position {
     if (nl) {
       for (const s of text.split(nl)) {
         if (i - s.length <= 0) {
-          column = i;
+          column = Math.max(i, 0);
           i = 0;
           break;
         } else {

packages/parser/src/error.ts

@@ -1,10 +1,11 @@
-import type { Range } from "@knuckles/location";
+import type { Position } from "@knuckles/location";
 
 export class ParserError extends Error {
   constructor(
-    readonly range: Range,
+    readonly start: Position,
+    readonly end: Position | undefined,
     readonly description: string,
   ) {
-    super(`${range.start.format()}: ${description}`);
+    super(`${start.format()}: ${description}`);
   }
 }

packages/parser/src/parser.ts

@@ -50,8 +50,12 @@ export default class Parser {
   }
 
   //#region Utils
-  #error(range: Range, description: string) {
-    const error = new ParserError(range, description);
+  #error(range: Range, description: string): ParserError;
+  #error(start: Position, description: string): ParserError;
+  #error(location: Range | Position, description: string): ParserError {
+    const start = location instanceof Range ? location.start : location;
+    const end = location instanceof Range ? location.end : undefined;
+    const error = new ParserError(start, end, description);
     this.errors.push(error);
     return error;
   }
@@ -391,6 +395,17 @@ export default class Parser {
     const range = parse5LocationToRange(
       node.sourceCodeLocation!.attrs![attr.name]!,
     );
+    const valueRange = new Range(
+      Position.fromOffset(
+        range.start.offset +
+          // name
+          attr.name.length +
+          // equal
+          1,
+        this.#string,
+      ),
+      range.end,
+    );
 
     return new Attribute({
       name: new Identifier({
@@ -404,19 +419,13 @@ export default class Parser {
         ),
       }),
       value: new StringLiteral({
-        value: attr.value,
-        quote,
-        range: new Range(
-          Position.fromOffset(
-            range.start.offset +
-              // name
-              attr.name.length +
-              // equal
-              1,
-            this.#string,
-          ),
-          range.end,
+        // parse5 replaces all occurrences of CRLF with LF, which breaks mappings.
+        value: this.#string.slice(
+          valueRange.start.offset + (quoted ? 1 : 0),
+          valueRange.end.offset - (quoted ? 1 : 0),
         ),
+        quote,
+        range: valueRange,
       }),
       namespace: attr.namespace,
       prefix: attr.prefix,

packages/parser/src/utils/char-iter.ts

@@ -1,5 +1,5 @@
 import { ParserError } from "../error.js";
-import { Range } from "@knuckles/location";
+import { Position } from "@knuckles/location";
 
 export default class CharIter {
   #index: number;
@@ -31,7 +31,8 @@ export default class CharIter {
   next() {
     if (this.#index >= this.chars.length - 1) {
       throw new ParserError(
-        Range.fromOffsets(this.#index, this.#index + 1, this.string),
+        Position.fromOffset(this.#index, this.string),
+        undefined,
         "Unexpected end of input.",
       );
     } else {
@@ -53,7 +54,8 @@ export default class CharIter {
     for (const char of chars) {
       if (this.char() !== char) {
         throw new ParserError(
-          Range.fromOffsets(start, start + string.length, this.string),
+          Position.fromOffset(start, this.string),
+          Position.fromOffset(start + string.length, this.string),
           `Expected "${string}".`,
         );
       }

packages/ssr/src/renderer.ts

@@ -14,7 +14,7 @@ import type {
   ModuleProviderResolveResult,
 } from "./module-provider.js";
 import type { Plugin, Self, Sibling } from "./plugin.js";
-import { type Range } from "@knuckles/location";
+import { Position, Range } from "@knuckles/location";
 import { parse } from "@knuckles/parser";
 import {
   Element,
@@ -222,7 +222,11 @@ export class Renderer {
           createDiagnostic({
             type: "error",
             message: error.description,
-            range: error.range,
+            range: new Range(
+              error.start,
+              error.end ??
+                Position.translate(error.start, 1, this.modified.original),
+            ),
             cause: error,
           }),
         ),