From f64790bfc32db330b2be5bdff61af555a9d26067 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 12 Oct 2026 04:33:38 +0800 Subject: [PATCH] refactor(javascript): reuse validated source-map decoding Pass typed map objects into the codec and reuse validated TraceMap instances for regular and indexed lookups. Remove whole-map serialization, duplicate leaf decoding, an array-copy wrapper, and the unused offset-validation mode in artifact declaration collection. Validation: source-map and browser consumer suites (126 tests), declaration tests, and artifact reconstruction boundaries pass. Section overlap checks and raw source declarations remain intact. --- .../javascript/JavaScriptArtifactAnalysis.ts | 9 +--- src/domain/sourceMapEnvelope.test.ts | 26 +++------- src/domain/sourceMapEnvelope.ts | 28 +---------- src/javascript/sourceMaps/DecodedSourceMap.ts | 5 +- src/javascript/sourceMaps/SourceMapFormat.ts | 49 ++++++++++--------- src/javascript/sourceMaps/TraceSourceMap.ts | 22 ++++++++- 6 files changed, 61 insertions(+), 78 deletions(-) diff --git a/src/application/javascript/JavaScriptArtifactAnalysis.ts b/src/application/javascript/JavaScriptArtifactAnalysis.ts index 1e41529b0..81714ada0 100644 --- a/src/application/javascript/JavaScriptArtifactAnalysis.ts +++ b/src/application/javascript/JavaScriptArtifactAnalysis.ts @@ -529,7 +529,7 @@ const parseSourceMap = ( void cause; return invalidSourceMap(file, "Source map is not valid JSON."); } - const maps = flattenSourceMaps(value); + const maps = flattenSourceMapLeaves(value); if (maps === undefined) return invalidSourceMap(file, "Source map is not a version 3 map."); return collectSourceMapOriginals(file, maps); @@ -594,13 +594,6 @@ const unavailableSourceMap = ( }; }; -const flattenSourceMaps = ( - root: unknown, -): Readonly>[] | undefined => { - const leaves = flattenSourceMapLeaves(root); - return leaves === undefined ? undefined : [...leaves]; -}; - const invalidSourceMap = ( file: JavaScriptArtifactFile, limitation: string, diff --git a/src/domain/sourceMapEnvelope.test.ts b/src/domain/sourceMapEnvelope.test.ts index 27dfbbaa4..6359ef889 100644 --- a/src/domain/sourceMapEnvelope.test.ts +++ b/src/domain/sourceMapEnvelope.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { flattenSourceMapLeaves, isVersion3Map } from "./sourceMapEnvelope.js"; +import { flattenSourceMapLeaves } from "./sourceMapEnvelope.js"; const leaf = { version: 3, @@ -10,14 +10,7 @@ const leaf = { }; describe("source-map envelope walk", () => { - it("accepts version-3 maps and rejects other shapes", () => { - expect(isVersion3Map(leaf)).toBe(true); - expect(isVersion3Map({ version: 2, mappings: "AAAA" })).toBe(false); - expect(isVersion3Map(null)).toBe(false); - expect(isVersion3Map([{ version: 3 }])).toBe(false); - }); - - it("collects indexed section leaves without validating offsets by default", () => { + it("collects indexed section leaves without interpreting generated offsets", () => { const root = { version: 3, sections: [ @@ -28,24 +21,21 @@ describe("source-map envelope walk", () => { expect(flattenSourceMapLeaves(root)).toHaveLength(2); }); - it("rejects non-map sections and enforces offsets when requested", () => { + it("rejects sections without embedded maps", () => { const missing = { version: 3, sections: [{ offset: { line: 0, column: 0 } }], }; expect(flattenSourceMapLeaves(missing)).toBeUndefined(); - const badOffset = { - version: 3, - sections: [{ offset: { line: -1, column: 0 }, map: leaf }], - }; - expect( - flattenSourceMapLeaves(badOffset, { validateOffsets: true }), - ).toBeUndefined(); - expect(flattenSourceMapLeaves(badOffset)).toHaveLength(1); }); it("rejects non-version-3 roots and nested maps", () => { expect(flattenSourceMapLeaves({ version: 3 })).toHaveLength(1); + expect(flattenSourceMapLeaves(null)).toBeUndefined(); + expect(flattenSourceMapLeaves([{ version: 3 }])).toBeUndefined(); + expect( + flattenSourceMapLeaves({ version: 2, mappings: "AAAA" }), + ).toBeUndefined(); expect(flattenSourceMapLeaves({ mappings: "AAAA" })).toBeUndefined(); expect( flattenSourceMapLeaves({ diff --git a/src/domain/sourceMapEnvelope.ts b/src/domain/sourceMapEnvelope.ts index 65ac9f9c6..7be21703e 100644 --- a/src/domain/sourceMapEnvelope.ts +++ b/src/domain/sourceMapEnvelope.ts @@ -1,11 +1,4 @@ -/** - * Structural walk over indexed source-map envelopes. Both the browser - * source-map fetcher and the artifact source-map reader accept version-3 - * maps with recursively indexed `sections`; this module owns that walk so - * the two validators cannot drift. Leaf content checks (mappings, sources, - * names, contents) stay with the callers, which enforce different - * strictness. - */ +/** Artifact source declarations do not require generated-position decoding. */ type SourceMapRecord = Readonly>; /** @@ -25,17 +18,9 @@ const isRecord = (value: unknown): value is SourceMapRecord => typeof value === "object" && value !== null && !Array.isArray(value); /** Whether a parsed value has the version-3 map shape. */ -export const isVersion3Map = (value: unknown): value is SourceMapRecord => +const isVersion3Map = (value: unknown): value is SourceMapRecord => isRecord(value) && value.version === 3; -const isValidOffset = (value: unknown): boolean => - typeof value === "number" && Number.isSafeInteger(value) && value >= 0; - -export interface FlattenSourceMapOptions { - /** Require each section offset to carry safe-integer line/column. */ - readonly validateOffsets?: boolean; -} - /** * Collect leaf maps from an indexed source-map envelope, or undefined when * the envelope is not a version-3 map. A map carrying neither `mappings` @@ -44,7 +29,6 @@ export interface FlattenSourceMapOptions { */ export const flattenSourceMapLeaves = ( root: unknown, - options?: FlattenSourceMapOptions, ): readonly SourceMapRecord[] | undefined => { if (!isVersion3Map(root)) return undefined; const leaves: SourceMapRecord[] = []; @@ -55,14 +39,6 @@ export const flattenSourceMapLeaves = ( if (Array.isArray(map.sections)) { for (const section of map.sections) { if (!isRecord(section) || !isRecord(section.map)) return undefined; - if (options?.validateOffsets === true) { - if (!isRecord(section.offset)) return undefined; - if ( - !isValidOffset(section.offset.line) || - !isValidOffset(section.offset.column) - ) - return undefined; - } pending.push(section.map); } } else leaves.push(map); diff --git a/src/javascript/sourceMaps/DecodedSourceMap.ts b/src/javascript/sourceMaps/DecodedSourceMap.ts index b329cd68e..60bfcfc0e 100644 --- a/src/javascript/sourceMaps/DecodedSourceMap.ts +++ b/src/javascript/sourceMaps/DecodedSourceMap.ts @@ -51,12 +51,13 @@ export const decodeValidatedSourceMapLeaves = ( check?: () => void, ): readonly { readonly leaf: SourceMapLeaf; + readonly map: TraceMap; readonly rows: ReturnType; readonly resolvedSources: readonly string[]; }[] => leaves.map((leaf) => { check?.(); - const map = new TraceMap(JSON.stringify(leaf.map), mapUrl); + const map = new TraceMap(leaf.codecMap, mapUrl); const rows = decodedMappings(map); for (const [line, row] of rows.entries()) { if ((line & 255) === 0) check?.(); @@ -78,5 +79,5 @@ export const decodeValidatedSourceMapLeaves = ( ); } } - return { leaf, rows, resolvedSources: map.resolvedSources }; + return { leaf, map, rows, resolvedSources: map.resolvedSources }; }); diff --git a/src/javascript/sourceMaps/SourceMapFormat.ts b/src/javascript/sourceMaps/SourceMapFormat.ts index d59c8f3e5..fc3d3e8cd 100644 --- a/src/javascript/sourceMaps/SourceMapFormat.ts +++ b/src/javascript/sourceMaps/SourceMapFormat.ts @@ -2,6 +2,7 @@ import { z } from "zod"; import { WEB_SOURCE_MAP_LIMITS } from "../../domain/webSourceLocation.js"; import { hasValidSourceMapContents } from "../../domain/sourceMapContents.js"; import { sourceMapJsonText } from "../../domain/sourceMapEnvelope.js"; +import type { EncodedSourceMapXInput } from "@jridgewell/trace-mapping"; const leafSchema = z.object({ version: z.literal(3), @@ -74,10 +75,27 @@ export interface SourceMapSourceDeclaration { /** Validated leaf with absolute first-line offsets and its enclosing section boundary. */ export interface SourceMapLeaf { readonly map: z.output; + readonly codecMap: EncodedSourceMapXInput; readonly offset: Offset; readonly stop: Offset | null; } +const codecLeaf = ( + map: z.output, +): EncodedSourceMapXInput => ({ + version: 3, + ...(map.file === undefined ? {} : { file: map.file }), + ...(typeof map.sourceRoot === "string" ? { sourceRoot: map.sourceRoot } : {}), + sources: map.sources, + ...(map.sourcesContent == null ? {} : { sourcesContent: map.sourcesContent }), + names: map.names, + mappings: map.mappings, + ...(map.ignoreList === undefined ? {} : { ignoreList: map.ignoreList }), + ...(map.x_google_ignoreList === undefined + ? {} + : { x_google_ignoreList: map.x_google_ignoreList }), +}); + const compare = (a: Offset, b: Offset): number => a.line - b.line || a.column - b.column; const addOffset = (base: Offset, offset: Offset): Offset => { @@ -283,30 +301,10 @@ const inspectLeaf = ( /** Validate structure before allocation, retaining raw declarations and section boundaries. */ export const inspectSourceMap = ( text: string, -): { - readonly jsonText: string; - readonly format: "regular" | "indexed"; - readonly reportedFile: string | null; - readonly declarations: readonly SourceMapSourceDeclaration[]; - readonly leaves: readonly SourceMapLeaf[]; -} => { +): ReturnType => { const jsonText = sourceMapJsonText(text); const value: unknown = JSON.parse(jsonText); - const inspected = inspectSourceMapValue(value); - const normalized = - inspected.format === "regular" - ? inspected.leaves[0]?.map - : { - version: 3, - sections: inspected.leaves.map((leaf) => ({ - offset: leaf.offset, - map: leaf.map, - })), - }; - return { - jsonText: JSON.stringify(normalized), - ...inspected, - }; + return inspectSourceMapValue(value); }; /** Return validated leaves when a consumer needs decoded rows after preflight. */ @@ -341,7 +339,12 @@ export const inspectSourceMapValue = ( continue; } const leaf = inspectLeaf(node, budget, options); - leaves.push({ map: leaf, offset: node.offset, stop: node.stop }); + leaves.push({ + map: leaf, + codecMap: codecLeaf(leaf), + offset: node.offset, + stop: node.stop, + }); if (options.profile !== "browser-collection") { const ignoredSources = new Set( leaf.ignoreList ?? leaf.x_google_ignoreList ?? [], diff --git a/src/javascript/sourceMaps/TraceSourceMap.ts b/src/javascript/sourceMaps/TraceSourceMap.ts index 8d2c81fc9..d85f79b36 100644 --- a/src/javascript/sourceMaps/TraceSourceMap.ts +++ b/src/javascript/sourceMaps/TraceSourceMap.ts @@ -34,7 +34,6 @@ export const traceSourceMap = ( position: WebSourcePosition, ): WebSourceMapReport => { const inspected = inspectSourceMap(text); - const map = new AnyMap(inspected.jsonText, url); const decodedLeaves = decodeValidatedSourceMapLeaves(inspected.leaves, url); for (const { leaf, rows } of decodedLeaves) for (const [line, row] of rows.entries()) @@ -44,6 +43,27 @@ export const traceSourceMap = ( "format", "Embedded source-map mappings overlap the following section boundary.", ); + let map: TraceMap; + if (inspected.format === "regular") { + const decoded = decodedLeaves[0]; + if (decoded === undefined) + throw new SourceMapFormatFailure( + "format", + "Regular source map has no validated leaf.", + ); + map = decoded.map; + } else { + map = new AnyMap( + { + version: 3, + sections: decodedLeaves.map(({ leaf, map: sectionMap }) => ({ + offset: leaf.offset, + map: sectionMap, + })), + }, + url, + ); + } const rows = inspected.format === "regular" ? (decodedLeaves[0]?.rows ?? [])