From 23862af1b57413e09abf394f6e4f8369f6355d5a Mon Sep 17 00:00:00 2001 From: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com> Date: Thu, 8 Oct 2026 12:15:24 -0400 Subject: [PATCH] fix(javascript): preserve HTML carriage return source ranges (#1092) * test(javascript): reproduce HTML carriage return source coordinates Compare actual resolved and unresolved artifact graph evidence with parse5 source locations across HTML line terminators and UTF-16 columns. Exercise both public built CLI analysis commands. Tests only; production and workflows unchanged. Signed-off-by: Rudy Celekli * fix(javascript): preserve HTML carriage return source ranges Treat LF, CRLF, and bare CR as HTML line endings when projecting actual script opening-tag offsets to retained source ranges. Preserve UTF-16 column semantics and document the source-coordinate contract. Signed-off-by: Rudy Celekli --------- Signed-off-by: Rudy Celekli --- docs/javascript-artifact-reconstruction.md | 2 + .../javascript/JavaScriptArtifactAnalysis.ts | 2 +- .../cli/javascriptApplicationProgress.test.ts | 43 +++++++ .../javascriptHtmlEntrypoints.test.ts | 106 ++++++++++++++++++ 4 files changed, 152 insertions(+), 1 deletion(-) diff --git a/docs/javascript-artifact-reconstruction.md b/docs/javascript-artifact-reconstruction.md index 4707ef0859..4cb46cfd23 100644 --- a/docs/javascript-artifact-reconstruction.md +++ b/docs/javascript-artifact-reconstruction.md @@ -268,3 +268,5 @@ document base and query/fragment rules. CommonJS module lookups retain extension and directory resolution. Unresolved HTML references retain their declaration, source range, and resolution reason in the renderer observations. +HTML script source ranges follow the HTML parser across LF, CRLF, and bare CR +line endings, preserving UTF-16 columns. diff --git a/src/application/javascript/JavaScriptArtifactAnalysis.ts b/src/application/javascript/JavaScriptArtifactAnalysis.ts index aba362b768..d7af23bba0 100644 --- a/src/application/javascript/JavaScriptArtifactAnalysis.ts +++ b/src/application/javascript/JavaScriptArtifactAnalysis.ts @@ -407,7 +407,7 @@ const pointForOffset = ( text: string, offset: number, ): JavaScriptSourcePoint => { - const lines = text.slice(0, offset).split("\n"); + const lines = text.slice(0, offset).split(/\r\n|\r|\n/u); return { line: lines.length, column: lines.at(-1)?.length ?? 0 }; }; diff --git a/tests/boundary/cli/javascriptApplicationProgress.test.ts b/tests/boundary/cli/javascriptApplicationProgress.test.ts index a4a0863e4d..06fc1c8cb0 100644 --- a/tests/boundary/cli/javascriptApplicationProgress.test.ts +++ b/tests/boundary/cli/javascriptApplicationProgress.test.ts @@ -4,6 +4,7 @@ import { join } from "node:path"; import { expect } from "vitest"; import { z } from "zod"; +import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; import { parseEvidence } from "../../../src/domain/evidence.js"; import { analyzeJavaScriptApplication } from "../../../src/application/javascript/JavaScriptApplicationService.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -70,3 +71,45 @@ cliTest( expect(updates.every(({ terminal }) => terminal !== true)).toBe(true); }, ); + +for (const command of ["analyze-javascript-application", "analyze"]) { + cliTest( + `${command} retains HTML source ranges after a bare carriage return`, + async ({ cli }) => { + const root = await createTestTempDirectory("rea-html-coordinate-cli-"); + const opening = '`, + ), + writeFile(join(root, "actual.js"), "export const actual = true;"), + ]); + const output = await cli.run({ arguments: [command, root, "--json"] }); + expect(output.exitCode).toBe(0); + const evidence = parseEvidence(output.json); + const result = javascriptApplicationAnalysisResultSchema.parse( + evidence.normalized_result, + ); + const loads = result.graph.edges.filter( + ({ relation, properties }) => + relation === "loads" && properties.script_path === "actual.js", + ); + expect(loads).toHaveLength(1); + expect(loads[0]).toMatchObject({ + properties: { resolved_path: "actual.js" }, + evidence: { + location: { + available: true, + value: { + kind: "source-range", + source: "index.html", + start: { line: 2, column: 0 }, + end: { line: 2, column: opening.length }, + }, + }, + }, + }); + }, + ); +} diff --git a/tests/boundary/filesystem/javascriptHtmlEntrypoints.test.ts b/tests/boundary/filesystem/javascriptHtmlEntrypoints.test.ts index f25d7fb9e8..a251469b01 100644 --- a/tests/boundary/filesystem/javascriptHtmlEntrypoints.test.ts +++ b/tests/boundary/filesystem/javascriptHtmlEntrypoints.test.ts @@ -2,6 +2,11 @@ import { createHash } from "node:crypto"; import { mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; +import { + defaultTreeAdapter, + parse, + type DefaultTreeAdapterTypes, +} from "parse5"; import { expect, it } from "vitest"; import { reconstructJavaScriptArtifact } from "../../../src/application/javascript/JavaScriptArtifactReconstruction.js"; @@ -85,3 +90,104 @@ it("preserves source digest, opening-tag range and decoded local base resolution }, }); }); + +const coordinateCases = [ + { name: "LF", prefix: "before\n", opening: '`; + const expected = htmlScriptRanges(html); + expect(expected).toHaveLength(2); + await Promise.all([ + writeFile(join(root, "index.html"), html), + writeFile(join(root, "actual.js"), "export const actual = true;"), + ]); + const result = await reconstructJavaScriptArtifact({ input_path: root }); + const resolved = result.graph.edges.filter( + ({ relation, properties }) => + relation === "loads" && properties.script_path === "actual.js", + ); + const unresolved = result.graph.nodes + .flatMap(({ observations }) => observations) + .filter( + ({ properties }) => + properties.mechanism === "html-script-reference" && + properties.script_path === "missing.js", + ); + expect(resolved).toHaveLength(1); + expect(unresolved).toHaveLength(1); + for (const [index, observation] of [resolved[0], unresolved[0]].entries()) { + expect(observation).toMatchObject({ + evidence: { + artifact: { + available: true, + sha256: createHash("sha256").update(html).digest("hex"), + }, + location: { + available: true, + value: { + kind: "source-range", + source: "index.html", + ...expected[index], + }, + }, + }, + }); + } + }, +); + +const htmlScriptRanges = (html: string) => { + const pending: DefaultTreeAdapterTypes.Node[] = [ + parse(html, { sourceCodeLocationInfo: true }), + ]; + const ranges = []; + while (pending.length > 0) { + const node = pending.pop(); + if (node === undefined) continue; + if (defaultTreeAdapter.isElementNode(node) && node.tagName === "script") { + const location = node.sourceCodeLocation?.startTag; + if (location === undefined) + throw new Error("Missing parse5 script range"); + ranges.push({ + start: { line: location.startLine, column: location.startCol - 1 }, + end: { line: location.endLine, column: location.endCol - 1 }, + }); + } + if ("childNodes" in node) { + for (const child of node.childNodes.toReversed()) pending.push(child); + } + } + return ranges; +};