From ecc2a3ccc1beb3f055cb3ec1ee73f0a9d3de7b10 Mon Sep 17 00:00:00 2001 From: voiys Date: Sat, 24 Oct 2026 02:37:55 +0200 Subject: [PATCH] Report parser offsets in input coordinates for CRLF files The parser normalizes \r\n to \n before parsing, so every offset it reported (on nodes and on syntax errors) was an offset into the normalized text. For a CRLF file the Nth line was off by N characters, which breaks anything that slices the original source with them. Map the offsets back to the input after parsing. Line and column are unchanged, and files without \r\n skip the mapping entirely. --- .bumpy/parser-crlf-locations.md | 5 + packages/env-spec-parser/README.md | 3 + packages/env-spec-parser/src/index.ts | 73 +++++++- .../env-spec-parser/test/locations.test.ts | 156 ++++++++++++++++++ .../src/content/docs/env-spec/reference.mdx | 4 + 5 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 .bumpy/parser-crlf-locations.md create mode 100644 packages/env-spec-parser/test/locations.test.ts diff --git a/.bumpy/parser-crlf-locations.md b/.bumpy/parser-crlf-locations.md new file mode 100644 index 000000000..654e0b3ec --- /dev/null +++ b/.bumpy/parser-crlf-locations.md @@ -0,0 +1,5 @@ +--- +"@env-spec/parser": patch +--- + +Fix item and syntax error offsets being wrong for files that use Windows (CRLF) line endings diff --git a/packages/env-spec-parser/README.md b/packages/env-spec-parser/README.md index 6d9ad0cc8..b103bc8c6 100644 --- a/packages/env-spec-parser/README.md +++ b/packages/env-spec-parser/README.md @@ -136,6 +136,9 @@ Each has a key, an optional value, and optional attached comments. +## Line endings +Files may use `\n` or `\r\n` line endings. Both are treated the same, so parsed values are identical either way. Node and syntax error locations (offset, line, column) always refer to the text you passed in, including any `\r` characters. + ## Common Value-Handling Rules Values are interpreted similarly for config item values, decorator values, and values within function call arguments. Values may be wrapped in quotes or not, but handling varies slightly. diff --git a/packages/env-spec-parser/src/index.ts b/packages/env-spec-parser/src/index.ts index 8911342e1..7f9907cba 100644 --- a/packages/env-spec-parser/src/index.ts +++ b/packages/env-spec-parser/src/index.ts @@ -8,6 +8,77 @@ export { autoCoerce } from './helpers'; // @ts-ignore import * as peggyParser from './grammar.js'; +type Position = { offset: number; line: number; column: number }; +type Location = { start: Position; end: Position }; + +/** + * The grammar only knows about `\n`, so we normalize `\r\n` before parsing. That makes every + * offset it reports an offset into the normalized text. This returns a function that maps such + * an offset back to an offset into the original source (or undefined if nothing was normalized). + * Line and column are the same in both, because a `\r\n` only ever ends a line. + */ +function createOffsetMapper(source: string): ((offset: number) => number) | undefined { + // position (in normalized text) of the `\n` of every `\r\n` we collapsed + const collapsedAt: Array = []; + let idx = source.indexOf('\r\n'); + while (idx !== -1) { + collapsedAt.push(idx - collapsedAt.length); + idx = source.indexOf('\r\n', idx + 2); + } + if (!collapsedAt.length) return undefined; + + // each collapsed newline before `offset` added one more character (the `\r`) to the original + return (offset) => { + let lo = 0; + let hi = collapsedAt.length; + while (lo < hi) { + const mid = Math.floor((lo + hi) / 2); + if (collapsedAt[mid] < offset) lo = mid + 1; + else hi = mid; + } + return offset + lo; + }; +} + +function remapLocation(loc: Location, mapOffset: (offset: number) => number) { + loc.start = { ...loc.start, offset: mapOffset(loc.start.offset) }; + loc.end = { ...loc.end, offset: mapOffset(loc.end.offset) }; +} + +/** walks the parsed tree, remapping every `_location` once (some are shared between nodes) */ +function remapTreeLocations( + node: object, + mapOffset: (offset: number) => number, + seen: Set, +) { + if (seen.has(node)) return; + seen.add(node); + for (const [key, val] of Object.entries(node)) { + if (!val || typeof val !== 'object') continue; + if (key === '_location') { + if (!seen.has(val)) { + seen.add(val); + remapLocation(val as Location, mapOffset); + } + } else { + remapTreeLocations(val, mapOffset, seen); + } + } +} + +/** + * Parses the contents of an env-spec file. Line endings can be `\n` or `\r\n`. + * All locations (on parsed nodes and on syntax errors) are in terms of the string passed in. + */ export function parseEnvSpecDotEnvFile(source: string): ParsedEnvSpecFile { - return peggyParser.parse(source.replaceAll('\r\n', '\n')); + const mapOffset = createOffsetMapper(source); + const normalized = mapOffset ? source.replaceAll('\r\n', '\n') : source; + try { + const parsed = peggyParser.parse(normalized) as ParsedEnvSpecFile; + if (mapOffset) remapTreeLocations(parsed, mapOffset, new Set()); + return parsed; + } catch (err) { + if (mapOffset && (err as any)?.location) remapLocation((err as any).location, mapOffset); + throw err; + } } diff --git a/packages/env-spec-parser/test/locations.test.ts b/packages/env-spec-parser/test/locations.test.ts new file mode 100644 index 000000000..ec490cc13 --- /dev/null +++ b/packages/env-spec-parser/test/locations.test.ts @@ -0,0 +1,156 @@ +import { describe, it, expect } from 'vitest'; +import { + parseEnvSpecDotEnvFile, ParsedEnvSpecConfigItem, ParsedEnvSpecDecorator, +} from '../src'; + +const LF_SOURCE = [ + '# header comment', + '# @defaultRequired=false', + '', + '# a comment', + '# @type=string(minLength=2) @sensitive', + 'FOO=foo # trailing comment', + 'BAR="multi', + 'line"', + 'BAZ=concat(one, "two")', + '', +].join('\n'); +const CRLF_SOURCE = LF_SOURCE.replaceAll('\n', '\r\n'); + +type Pos = { offset: number; line: number; column: number }; +type Loc = { start: Pos; end: Pos }; + +/** collects every `_location` found anywhere in the parsed tree */ +function collectLocations(node: unknown, out: Array = [], seen = new Set()): Array { + if (!node || typeof node !== 'object' || seen.has(node)) return out; + seen.add(node); + for (const [key, val] of Object.entries(node)) { + if (key === '_location' && val) out.push(val as Loc); + else collectLocations(val, out, seen); + } + return out; +} + +describe('node locations', () => { + it('are offsets into the input when it uses \\r\\n', () => { + const parsed = parseEnvSpecDotEnvFile(CRLF_SOURCE); + const items = parsed.contents.filter((c) => c instanceof ParsedEnvSpecConfigItem); + expect(items.map((i) => i.key)).toEqual(['FOO', 'BAR', 'BAZ']); + + for (const item of items) { + const loc = item.data._location as Loc; + // the item starts at its pre-comments and its source text contains its own `KEY=` + const text = CRLF_SOURCE.slice(loc.start.offset, loc.end.offset); + expect(text).toContain(`${item.key}=`); + } + + // value locations point right at the value text + const bar = items.find((i) => i.key === 'BAR')!; + const barLoc = bar.data.value!.data._location as Loc; + expect(CRLF_SOURCE.slice(barLoc.start.offset, barLoc.end.offset)).toBe('"multi\r\nline"'); + const baz = items.find((i) => i.key === 'BAZ')!; + const bazLoc = baz.data.value!.data._location as Loc; + expect(CRLF_SOURCE.slice(bazLoc.start.offset, bazLoc.end.offset)).toBe('concat(one, "two")'); + }); + + it('point at decorators when the input uses \\r\\n', () => { + const parsed = parseEnvSpecDotEnvFile(CRLF_SOURCE); + const decs: Array = [ + ...parsed.decoratorsArray, + ...parsed.configItems.flatMap((i) => i.decoratorsArray), + ]; + expect(decs.map((d) => d.name)).toEqual(['defaultRequired', 'type', 'sensitive']); + for (const dec of decs) { + const loc = dec.data._location as Loc; + expect(CRLF_SOURCE.slice(loc.start.offset, loc.end.offset)).toMatch(new RegExp(`^@${dec.name}`)); + } + }); + + it('only differ from the \\n version by the \\r characters before them', () => { + const lfLocs = collectLocations(parseEnvSpecDotEnvFile(LF_SOURCE)); + const crlfLocs = collectLocations(parseEnvSpecDotEnvFile(CRLF_SOURCE)); + expect(lfLocs.length).toBeGreaterThan(10); + expect(crlfLocs.length).toBe(lfLocs.length); + lfLocs.forEach((lf, i) => { + const crlf = crlfLocs[i]; + for (const edge of ['start', 'end'] as const) { + // line and column are the same, and every earlier line break added one `\r` + expect(crlf[edge].line).toBe(lf[edge].line); + expect(crlf[edge].column).toBe(lf[edge].column); + expect(crlf[edge].offset).toBe(lf[edge].offset + (lf[edge].line - 1)); + } + }); + }); + + it('do not change the parsed values', () => { + const values = (src: string) => parseEnvSpecDotEnvFile(src).configItems.map((i) => i.data.value?.toString()); + expect(values(CRLF_SOURCE)).toEqual(values(LF_SOURCE)); + expect(values(CRLF_SOURCE)[1]).toBe('"multi\nline"'); + }); + + it('are unchanged when the input uses \\n', () => { + const parsed = parseEnvSpecDotEnvFile('FOO=foo\nBAR=bar\n'); + const [foo, bar] = parsed.configItems; + expect(foo.data._location.start.offset).toBe(0); + expect(foo.data._location.end.offset).toBe(8); + expect(bar.data._location.start.offset).toBe(8); + expect(bar.data._location.end.offset).toBe(16); + }); + + it('are correct for mixed line endings', () => { + const src = 'FOO=foo\r\nBAR=bar\nBAZ=baz\r\n'; + const parsed = parseEnvSpecDotEnvFile(src); + const slices = parsed.configItems.map((i) => { + const loc = i.data.value!.data._location as Loc; + return src.slice(loc.start.offset, loc.end.offset); + }); + expect(slices).toEqual(['foo', 'bar', 'baz']); + }); +}); + +describe('syntax error locations', () => { + function getError(source: string) { + try { + parseEnvSpecDotEnvFile(source); + } catch (err) { + return err as Error & { location: Loc }; + } + throw new Error('expected a syntax error'); + } + + it('are offsets into the input when it uses \\r\\n', () => { + const lfSource = 'A=1\nB=2\n@oops\n'; + const crlfSource = lfSource.replaceAll('\n', '\r\n'); + const lfError = getError(lfSource); + const crlfError = getError(crlfSource); + expect(crlfError.location.start.line).toBe(lfError.location.start.line); + expect(crlfError.location.start.column).toBe(lfError.location.start.column); + expect(crlfError.location.start.offset).toBe(lfError.location.start.offset + 2); + expect(crlfSource.slice(crlfError.location.start.offset)).toMatch(/^@/); + }); + + it.each([ + ['at the end of the input', 'A=1\nB=2\n# @type='], + ['after a trailing line break', 'A=1\n# @a(\n'], + ['spanning a line break', 'A=1\n# @type=\nB=2\n'], + ])('have the right start and end offsets for an error %s', (_desc, lfSource) => { + const crlfSource = lfSource.replaceAll('\n', '\r\n'); + const lfError = getError(lfSource); + const crlfError = getError(crlfSource); + for (const edge of ['start', 'end'] as const) { + expect(crlfError.location[edge].line).toBe(lfError.location[edge].line); + expect(crlfError.location[edge].column).toBe(lfError.location[edge].column); + expect(crlfError.location[edge].offset).toBe(lfError.location[edge].offset + (lfError.location[edge].line - 1)); + } + // sanity check the \n version is in input coordinates already + expect(lfError.location.end.offset).toBeLessThanOrEqual(lfSource.length); + expect(crlfError.location.end.offset).toBeLessThanOrEqual(crlfSource.length); + }); + + it('point at the end of the input when the error is at the end of a \\r\\n input', () => { + const source = 'A=1\r\nB=2\r\n# @type='; + const { location } = getError(source); + expect(location.start.offset).toBe(source.length); + expect(location.end.offset).toBe(source.length); + }); +}); diff --git a/packages/varlock-website/src/content/docs/env-spec/reference.mdx b/packages/varlock-website/src/content/docs/env-spec/reference.mdx index 54c1400bd..c12ce8679 100644 --- a/packages/varlock-website/src/content/docs/env-spec/reference.mdx +++ b/packages/varlock-website/src/content/docs/env-spec/reference.mdx @@ -149,6 +149,10 @@ ITEM1= ## Common rules +### Line endings + +Files may use `\n` or `\r\n` line endings. Both are treated the same, so parsed values are identical either way. + ### Value handling Values are interpreted similarly for config item values, decorator values, and values within function call arguments. Values may be wrapped in quotes or not, but handling varies slightly: