diff --git a/CHANGELOG.md b/CHANGELOG.md index 07159d9b..1fe39790 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Patching: Resolve additional patching roundtrip errors from fuzzing harness (3M seeds) ([#293]) +- Stringify: `newLine` is now normalized and validated wherever a format is resolved ([#293]) +- Patching: `TomlDocument.patch()` no longer widens TOML dates when re-applying an object read from `toJsObject` ([#293]) + ## [3.0.3] - 2026-08-21 ### Fixed @@ -399,4 +405,5 @@ This first forked version from [timhall/toml-patch](https://github.com/timhall/t [#289]: https://github.com/DecimalTurn/toml-patch/pull/289 [#290]: https://github.com/DecimalTurn/toml-patch/pull/290 [#292]: https://github.com/DecimalTurn/toml-patch/pull/292 +[#293]: https://github.com/DecimalTurn/toml-patch/pull/293 [0e66e68]: https://github.com/DecimalTurn/toml-patch/commit/0e66e68cbf42a07bc44445e46c3ea7bea97f95c1 diff --git a/docs/bug-notes/fuzz-error-seeds-0-3000000-rerun.md b/docs/bug-notes/fuzz-error-seeds-0-3000000-rerun.md new file mode 100644 index 00000000..9ade56c3 --- /dev/null +++ b/docs/bug-notes/fuzz-error-seeds-0-3000000-rerun.md @@ -0,0 +1,34 @@ +# Fuzz Error Seeds: 0..2,999,999 Rerun + +Source log: [fuzz-sweep-0-3000000-RERUN.md](fuzz-sweep-0-3000000-RERUN.md) + +The rerun reported 16 failures across three one-million-seed ranges. The +`updateOrder` warnings in the source log are not included because they are +expected unsupported-location warnings rather than harness failures. + +## Seeds + +| Seed | Range | Failure | Detail | +| ---: | :--- | :--- | :--- | +| 175924 | 0..999999 | `roundtrip-mismatch` | Re-parse failed at `(79, 1)` | +| 377453 | 0..999999 | `roundtrip-mismatch` | Re-parse failed at `(23, 5)` | +| 771152 | 0..999999 | `roundtrip-mismatch` | Re-parse failed at `(7, 29)` | +| 863664 | 0..999999 | `roundtrip-mismatch` | Re-parse failed at `(51, 5)` | +| 1112646 | 1000000..1999999 | `roundtrip-mismatch` | Re-parse failed at `(122, 22)` | +| 1286183 | 1000000..1999999 | `roundtrip-mismatch` | Re-parse failed at `(42, 274)` | +| 1383962 | 1000000..1999999 | `roundtrip-mismatch` | Re-parse failed at `(20, 3)` | +| 1693919 | 1000000..1999999 | `roundtrip-mismatch` | Re-parse failed at `(15, 101)` | +| 1896226 | 1000000..1999999 | `roundtrip-mismatch` | Re-parse failed at `(64, 1)` | +| 2185943 | 2000000..2999999 | `roundtrip-mismatch` | Re-parse failed at `(118, 16)` | +| 2497422 | 2000000..2999999 | `roundtrip-mismatch` | Re-parse failed at `(9, 92)` | +| 2531104 | 2000000..2999999 | `roundtrip-mismatch` | Re-parse failed at `(17, 5)` | +| 2591153 | 2000000..2999999 | `patch-fail` | `patch()` threw: Node not found at `AKy:}nV@.p8.(J= 0 ? process.argv[index + 1] : fallback; +} + +const seed = Number(arg('--seed')); +const target = resolve(arg('--target', process.cwd())!); +const output = arg('--out'); +const maxPasses = Number(arg('--passes', '4')); +if (!Number.isInteger(seed) || !output) { + throw new Error('Usage: npx -y tsx scripts/distill-seed.ts --seed N --out path [--target path]'); +} + +const importFromTarget = async (relativePath: string) => { + const url = pathToFileURL(resolve(target, relativePath)).href; + return import(url); +}; +const randomizer = await importFromTarget('src/__tests__/randomizer.ts'); +const fuzz = await importFromTarget('src/__tests__/fuzz-patch.ts'); +const api = await importFromTarget('src/index.ts'); +const { randomToml, SeededRandom } = randomizer; +const { parse, patch } = api; +const { generateMutation, applyMutation, deepClone, randomTomlFormat } = fuzz; + +const generated = randomToml({ seed }); +const originalObject = deepClone(parse(generated.toml)); +const mutationCount = 3; +const mutationRng = new SeededRandom(seed + mutationCount * 1_000_000); +const mutations: Mutation[] = []; +const mutationObject = deepClone(originalObject) as any; +const aotKeyPaths = new Set(); +const collectAotKeys = (node: any) => { + if (!node || typeof node !== 'object') return; + if (node.type === 'TableArray' && node.key?.item) { + aotKeyPaths.add((node.key.item.value as string[]).join('.')); + } + if (Array.isArray(node.items)) for (const item of node.items) collectAotKeys(item); + if (node.value) collectAotKeys(node.value); + if (node.item) collectAotKeys(node.item); +}; +collectAotKeys(generated.document); +const isTableLike = (value: unknown) => + value !== null && typeof value === 'object' && !Array.isArray(value) && !(value instanceof Date); +let attempts = 0; +while (mutations.length < mutationCount && attempts < mutationCount * 5) { + attempts++; + const mutation = generateMutation(mutationObject, mutationRng); + if (!mutation) break; + if (mutation.newValue !== undefined && !isTableLike(mutation.newValue)) { + const last = mutation.path.at(-1); + const parentPath = mutation.path.slice(0, -1); + const stringPath = parentPath.filter(segment => typeof segment === 'string').join('.'); + if (typeof last === 'number' && aotKeyPaths.has(stringPath)) continue; + } + applyMutation(mutationObject, mutation); + mutations.push(mutation); +} +const format = randomTomlFormat(new SeededRandom(seed + 500_000)); + +function normalize(value: unknown): unknown { + if (typeof value === 'bigint') return `${value}n`; + if (typeof value === 'number') { + if (Number.isNaN(value)) return 'NaN'; + if (value === Infinity) return 'Infinity'; + if (value === -Infinity) return '-Infinity'; + return value; + } + if (value instanceof Date) return `Date:${value.constructor.name}:${value.getTime()}:${value.toISOString()}`; + if (Array.isArray(value)) return value.map(normalize); + if (value && typeof value === 'object') { + const result: Record = {}; + for (const key of Object.keys(value)) result[key] = normalize((value as any)[key]); + return result; + } + return value; +} + +function getAt(object: any, path: (string | number)[]): unknown { + let current = object; + for (const segment of path) { + if (current == null) return undefined; + current = current[segment as any]; + } + return current; +} + +function canReplay(object: any, mutation: Mutation): boolean { + const parent = getAt(object, mutation.path.slice(0, -1)); + if (parent == null || typeof parent !== 'object') return false; + const last = mutation.path[mutation.path.length - 1]; + if (mutation.kind === 'delete-key' || mutation.kind === 'remove-array-item') { + return getAt(object, mutation.path) !== undefined; + } + if (mutation.kind === 'add-array-item') { + return Array.isArray(parent) && typeof last === 'number' && last <= parent.length; + } + return getAt(object, mutation.path) !== undefined; +} + +function isFailure(source: string, testMutations: Mutation[] = mutations): boolean { + let object: any; + try { + object = deepClone(parse(source)); + } catch { + return false; + } + try { + for (const mutation of testMutations) { + if (!canReplay(object, mutation)) return false; + applyMutation(object, mutation); + } + const result = patch(source, object, format); + const reparsed = parse(result); + return JSON.stringify(normalize(object)) !== JSON.stringify(normalize(reparsed)); + } catch { + return true; + } +} + +function removeRange(lines: string[], start: number, end: number): string { + return lines.slice(0, start).concat(lines.slice(end)).join('\n'); +} + +let lines = generated.toml.split(/\r?\n/); +if (lines.at(-1) === '') lines.pop(); +if (!isFailure(lines.join('\n'))) { + throw new Error(`Seed ${seed} is not a failure under target ${target}`); +} + +for (let pass = 0; pass < maxPasses; pass++) { + let granularity = 2; + let changed = false; + while (granularity <= lines.length) { + const chunkSize = Math.ceil(lines.length / granularity); + let removed = false; + for (let start = 0; start < lines.length; start += chunkSize) { + const end = Math.min(lines.length, start + chunkSize); + const candidate = removeRange(lines, start, end); + if (!candidate || isFailure(candidate)) { + lines.splice(start, end - start); + removed = true; + changed = true; + break; + } + } + if (removed) { + granularity = Math.max(2, granularity - 1); + } else if (granularity < lines.length) { + granularity = Math.min(lines.length, granularity * 2); + } else { + break; + } + } + if (!changed) break; +} + +// Remove mutations that do not contribute to the failure, then run the line reducer +// again because a shorter mutation list often makes unrelated source structure removable. +for (let index = mutations.length - 1; index >= 0; index--) { + const candidateMutations = mutations.slice(0, index).concat(mutations.slice(index + 1)); + if (candidateMutations.length > 0 && isFailure(lines.join('\n'), candidateMutations)) { + mutations.splice(index, 1); + } +} +for (let pass = 0; pass < maxPasses; pass++) { + let changed = false; + for (let index = lines.length - 1; index >= 0; index--) { + const candidate = lines.slice(0, index).concat(lines.slice(index + 1)).join('\n'); + if (candidate && isFailure(candidate)) { + lines.splice(index, 1); + changed = true; + } + } + if (!changed) break; +} + +function valueToSource(value: unknown): string { + if (value === null) return 'null'; + if (typeof value === 'string') return JSON.stringify(value); + if (typeof value === 'number') { + if (Number.isNaN(value)) return 'NaN'; + if (value === Infinity) return 'Infinity'; + if (value === -Infinity) return '-Infinity'; + return String(value); + } + if (typeof value === 'boolean') return String(value); + if (typeof value === 'bigint') return `${value}n`; + if (value instanceof Date) { + return `new Date(Date.UTC(${value.getUTCFullYear()}, ${value.getUTCMonth()}, ${value.getUTCDate()}))`; + } + if (Array.isArray(value)) return `[${value.map(valueToSource).join(', ')}]`; + if (value && typeof value === 'object') { + return `{ ${Object.entries(value as Record) + .map(([key, item]) => `${JSON.stringify(key)}: ${valueToSource(item)}`).join(', ')} }`; + } + return JSON.stringify(value); +} + +function accessor(path: (string | number)[]): string { + return 'obj' + path.map(segment => typeof segment === 'number' + ? `[${segment}]` + : /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(segment) + ? `.${segment}` + : `[${JSON.stringify(segment)}]`).join(''); +} + +function mutationSource(mutation: Mutation): string { + const last = mutation.path.at(-1); + if (mutation.kind === 'delete-key') return `delete ${accessor(mutation.path)};`; + if (mutation.kind === 'remove-array-item') return `${accessor(mutation.path.slice(0, -1))}.splice(${last}, 1);`; + if (mutation.kind === 'add-array-item') { + return `${accessor(mutation.path.slice(0, -1))}.splice(${last}, 0, ${valueToSource(mutation.newValue)});`; + } + return `${accessor(mutation.path)} = ${valueToSource(mutation.newValue)};`; +} + +function formatSource(value: unknown): string { + if (value === undefined) return 'undefined'; + return JSON.stringify(value, null, 2) + .replace(/"([A-Za-z_$][A-Za-z0-9_$]*)":/g, '$1:') + .replace(/"\\r\\n"/g, "'\\r\\n'") + .replace(/"\\n"/g, "'\\n'"); +} + +const source = lines.join('\n'); +const postFixObject: any = deepClone(parse(source)); +for (const mutation of mutations) applyMutation(postFixObject, mutation); +const expected = patch(source, postFixObject, format); +const body = [ + `test.fails('distilled regression for fuzz seed ${seed}', () => {`, + ' const src = dedent`', + ...source.split('\n').map(line => ` ${line.replaceAll('`', '\\`').replaceAll('${', '\\${')}`), + ' `;', + '', + ' const obj = parse(src) as any;', + ...mutations.map(mutation => ` ${mutationSource(mutation)}`), + '', + ` const result = patch(src, obj, ${formatSource(format)});`, + ' expect(parse(result)).toEqual(obj);', + ' // TODO: assert exact output after the implementation fix.', + ` // expect(result).toEqual(${JSON.stringify(expected)});`, + '});', + '' +].join('\n'); +writeFileSync(resolve(output), body); +console.log(JSON.stringify({ seed, target, lines: lines.length, mutations: mutations.map(mutation => `${mutation.kind} ${mutation.path.join('.')}`) })); diff --git a/src/__tests__/__js__/patch.test.mjs b/src/__tests__/__js__/patch.test.mjs index 557f6f9c..7094a754 100644 --- a/src/__tests__/__js__/patch.test.mjs +++ b/src/__tests__/__js__/patch.test.mjs @@ -16,7 +16,7 @@ */ import { vi } from 'vitest'; -import { patch, TomlFormat } from "../../../dist/toml-patch.js"; +import { parse, patch, TomlFormat } from "../../../dist/toml-patch.js"; describe('patch() Function JavaScript Integration', () => { const originalToml = `# Configuration file @@ -268,6 +268,37 @@ cache = true }); }); + describe('newer public behavior', () => { + it('should preserve large integers as BigInt when patching', () => { + const original = 'count = 9007199254740993\n'; + const updated = { count: 9007199254740995n }; + + const result = patch(original, updated); + + expect(result).toBe('count = 9007199254740995\n'); + expect(parse(result).count).toBe(9007199254740995n); + }); + + it('should reorder root keys when updateOrder is enabled', () => { + const original = 'first = 1\nsecond = 2\n'; + const updated = { second: 2, first: 1 }; + + const result = patch(original, updated, { updateOrder: true }); + + expect(result).toBe('second = 2\nfirst = 1\n'); + }); + + it('should append entries to an existing array of tables', () => { + const original = '[[servers]]\nname = "web"\n'; + const updated = { servers: [{ name: 'web' }, { name: 'api' }] }; + + const result = patch(original, updated); + + expect(result).toBe('[[servers]]\nname = "web"\n\n[[servers]]\nname = "api"\n'); + expect(parse(result).servers).toEqual(updated.servers); + }); + }); + describe('complex format objects', () => { it('should work with multiple format properties using object literals', () => { const updatedObject = { ...baseUpdatedObject }; @@ -838,6 +869,31 @@ port = 5432 } }); + it.each([ + '\r', + '\\r', + '', + 'invalid' + ])('should reject unsupported newLine value %j', (newLine) => { + expect(() => patch(originalToml, baseUpdatedObject, { newLine })) + .toThrow('Invalid newLine value: expected LF or CRLF'); + }); + + it.each([ + ['\n', '\n'], + ['\r\n', '\r\n'], + ['\\n', '\n'], + ['\\r\\n', '\r\n'], + ['lf', '\n'], + ['CrLf', '\r\n'], + ['unix', '\n'], + ['DOS', '\r\n'] + ])('should accept lenient newLine value %j as %j', (newLine, expectedNewLine) => { + const result = patch(originalToml, baseUpdatedObject, { newLine }); + const lineEnding = result.includes('\r\n') ? '\r\n' : '\n'; + expect(lineEnding).toBe(expectedNewLine); + }); + it('should handle TomlFormat constructor with all optional parameters', () => { // Test parameterless constructor - should work and use defaults expect(() => { diff --git a/src/__tests__/__js__/stringify.test.mjs b/src/__tests__/__js__/stringify.test.mjs index 99785ec9..da11a4ff 100644 --- a/src/__tests__/__js__/stringify.test.mjs +++ b/src/__tests__/__js__/stringify.test.mjs @@ -230,6 +230,8 @@ describe('stringify() Function JavaScript Integration', () => { const format = { bracketSpacing: true, trailingComma: true, + // The two-character escape spelling now resolves to a real LF; it used + // to be written out verbatim between every pair of lines. newLine: "\\n", trailingNewline: 2 }; @@ -238,7 +240,7 @@ describe('stringify() Function JavaScript Integration', () => { expect(result).toContain('servers = [ "web", "api", "db", ]'); expect(result).toContain('auth = [ "basic", "oauth", ]'); - expect(result.endsWith('\\n\\n')).toBe(true); + expect(result.endsWith('\n\n')).toBe(true); }); it('should work with format objects created from prototypes', () => { diff --git a/src/__tests__/parse.test.ts b/src/__tests__/parse.test.ts index 335575ec..844d7567 100644 --- a/src/__tests__/parse.test.ts +++ b/src/__tests__/parse.test.ts @@ -207,3 +207,28 @@ k33 = 4597 c: [{ b: 2 }] }); }); + +// BUG: a single-line literal string cannot hold an odd number of double quotes. +// Literal strings have no escaping at all -- everything between the delimiters is +// taken verbatim -- so `'has " inside'` is valid TOML and should parse. It does +// not: an odd count fails while an even count succeeds, which means the tokenizer +// is pairing double quotes in a context where they carry no meaning. +// +// a = '"' -> throws a = '""' -> ok +// a = '"""' -> throws a = '""""' -> ok +// +// Multiline literal strings are unaffected, so this is specific to the +// single-quote scanner. Predates the current work and is reproducible on 3.0.3. +// The official toml-test suite passes, so it does not cover this case. +test.fails('a single-line literal string holds an odd number of double quotes', () => { + expect(parse(`a = '"'\n`)).toEqual({ a: '"' }); +}); + +test('a single-line literal string holds an even number of double quotes', () => { + expect(parse(`a = '""'\n`)).toEqual({ a: '""' }); +}); + +test('a multiline literal string holds double quotes at any count', () => { + expect(parse(`a = '''"'''\n`)).toEqual({ a: '"' }); + expect(parse(`a = '''"""'''\n`)).toEqual({ a: '"""' }); +}); diff --git a/src/__tests__/patch.fuzz.test.ts b/src/__tests__/patch.fuzz.test.ts index 47abe7c8..41b7349c 100644 --- a/src/__tests__/patch.fuzz.test.ts +++ b/src/__tests__/patch.fuzz.test.ts @@ -1,7 +1,25 @@ import patch from '../patch'; import { parse } from '../'; +import diff from '../diff'; import dedent from 'dedent'; +import { fuzzOne } from './fuzz-patch'; + +const historicalFuzzSeeds = [ + 19506, 21525, 30330, 31662, 32801, 35943, 37465, 39363, 40181, 41613, + 43159, 43199, 46522, 54607, 61827, 62163, 62263, 65785, 67221, 68244, + 68861, 78079, 79938, 80004, 82825, 86547, 86724, 121096, 129645, 136292, + 136865, 175924, 179377, 186384, 208822, 224081, 272851, 299772, 358055, + 377453, 421965, 460447, 599513, 742554, 771152, 863085, 863664, 1020868, + 1024477, 1112646, 1137525, 1285105, 1286183, 1383962, 1428499, 1657445, + 1674968, 1693919, 1845422, 1896226, 1947810, 2185943, 2497422, 2531104, + 2591153, 2667551, 2824408, 2858114 +]; + +test.each(historicalFuzzSeeds)('historical fuzz seed %d still passes the full harness', (seed) => { + const result = fuzzOne(seed, 3); + expect(result.status, result.error).toBe('ok'); +}); test('replacing an object in a nested multiline array preserves trailing siblings (seed 1112646)', () => { const src = dedent` @@ -9,10 +27,6 @@ test('replacing an object in a nested multiline array preserves trailing sibling 0, 1, 2, - 3, - 4, - 5, - 6, [ { old = ''' old content @@ -26,12 +40,54 @@ test('replacing an object in a nested multiline array preserves trailing sibling ] `; + const original = parse(src) as any; const obj = parse(src) as any; - obj.values[7][0] = { + // Replace the inline table in the nested array with a new inline table that has a different structure and values. + obj.values[3][0] = { primary: -4807.689925655723, details: { values: [-2765, new Date(Date.UTC(2016, 6, 12))] } }; + const changes = diff(original, obj); + expect(changes).toHaveLength(3); + expect(changes).toMatchInlineSnapshot(` + Array [ + Object { + "path": Array [ + "values", + 3, + 0, + "old", + ], + "type": "Remove", + }, + Object { + "path": Array [ + "values", + 3, + 0, + "primary", + ], + "type": "Add", + }, + Object { + "path": Array [ + "values", + 3, + 0, + "details", + ], + "type": "Add", + }, + ] + `); + + // TODO: Decide if we should consider making the inline table multiline to preserve the + // original formatting of the inline table that is being replaced (or using the fact + // that we are already inside a multiline array). Alternatively, we could just have the + // decision of using a multi-line inline table be based on the desired print-width (only + // using MLIT if they allow to shrink the inline table to fit the print-width). + // Currently, it is being converted to a single-line inline table. const result = patch(src, obj); expect(parse(result)).toEqual(obj); expect(result).toEqual(dedent` @@ -39,10 +95,6 @@ test('replacing an object in a nested multiline array preserves trailing sibling 0, 1, 2, - 3, - 4, - 5, - 6, [ { primary = -4807.689925655723, details = { values = [ -2765, 2016-07-12T00:00:00.000Z, ], }, }, false, @@ -3367,3 +3419,534 @@ test('converting an AOT into a scalar-first mixed array (seed 136865 alt.2)', () `); }); + +// The cases below are reductions of fuzz seeds, distilled down to the structure +// that actually triggers each bug: the random keys, values and padding the seed +// happened to generate are renamed or removed, since the reproduction depends on +// the shape (multiline strings inside multiline inline containers, how many +// writes land in one container) and not on the characters. The seeds themselves +// stay covered byte-for-byte by the historical harness test above. +// +// Each asserts the exact TOML produced, not just that the result round-trips. +// Most of them only round-trip because patch() validates its first attempt and +// retries transactionally, and that retry rewrites whole multiline inline +// containers: multiline strings collapse to basic strings, non-decimal integer +// bases are normalised, dotted keys expand into nested inline tables. The +// snapshots record that, so recovering any original formatting shows up as a +// reviewable diff. Regenerate with `vitest -u` once a change is confirmed good. + +// Deleting a root key shifts every offset below it, and the nested array is +// replaced by a shorter one plus a spliced duplicate. The removals are emitted +// in original-array coordinates, so reorder() has to place them before the +// same-array additions or a later removal addresses an index that is gone. +test('removing a root key while shrinking a nested array in a multiline inline table (seed 2591153)', () => { + const src = dedent` + removed = 2049-04-03T21:46:36.579301 + ["host config"] + enabled = true + options = { + label = "a value", + nested.items.list = [967985, -25027300000000694064, true, true, 50_953.25829, ["inner", 0b100101, -650416], """ + ml text"""], + tail = 'another value', + } + `; + + const obj = parse(src) as any; + delete obj.removed; + obj['host config'].options.nested.items.list = [true, false, true, 3173]; + obj['host config'].options.nested.items.list.splice(3, 0, 3173); + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(` + "[\\"host config\\"] + enabled = true + options = { + label = \\"a value\\", + nested.items.list = [true, false, true, 3173, 3173], + tail = 'another value', + }" + `); + expect(parse(result)).toEqual(obj); +}); + +// The dotted key loses its last segment and becomes an array, while the +// multiline inline array below it must keep its own layout untouched. +test('replacing a dotted-key subtable with an array beside a multiline inline array (seed 1112646)', () => { + const src = dedent` + [["release notes"]] + "doc path".summary.detail = ''' + ok''' + items = [ + ''' + ,''', + [{ "inner key" = ''' + text!''' }, false, 287_173, "a plain string value", 1984-04-16T12:37:13Z], + ] + `; + + const obj = parse(src) as any; + obj['release notes'][0]['doc path'].summary = [new Date(Date.UTC(2018, 5, 14)), -2069, 784]; + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(` + "[[\\"release notes\\"]] + \\"doc path\\".summary = [ 2018-06-14T00:00:00.000Z, -2069, 784, ] + items = [ + ''' + ,''', + [{ \\"inner key\\" = ''' + text!''' }, false, 287_173, \\"a plain string value\\", 1984-04-16T12:37:13Z], + ]" + `); + expect(parse(result)).toEqual(obj); +}); + +// The deleted key is the first member of a multiline inline table whose +// remaining members span two more lines, so every following offset moves. +test('deleting a deep dotted key inside a multiline inline table (seed 1286183)', () => { + const src = dedent` + header = """ + ab""" + [section] + outer."quoted key" = { + first.second.third = { alpha.beta.gamma = ''' + ''' }, + filler = { one.two.three = -65079.12231, flag = false, "q one".mid.leaf = false, group = { count.total = 770012, note.body = """ + first inline note""", a-b.c.d = 'short lit', e.f = 36740, "q two".g = false, empty = "", "q three".h = 527558, "q four".date.stamp = 2080-12-04, "q five" = 31256.58522, i.j = 819597, k = 0x08, l = -284092, MM = 0o64, n.o = true }, "q six".p.q = 2081-06-28T00:40:23Z, "q seven".r = """ + second inline note""", s.t = {}, u.v = 90692.79376, w."q eight" = true }, + } + `; + + const obj = parse(src) as any; + delete obj.section.outer['quoted key'].first.second.third.alpha.beta.gamma; + + const result = patch(src, obj, { + inlineTableStart: 0, + trailingComma: false, + bracketSpacing: true, + updateOrder: false, + trailingNewline: 1, + newLine: "\r\n", + leadingBom: false, + truncateZeroTimeInDates: true, + useTabsForIndentation: false + }); + expect(result).toMatchInlineSnapshot(` + "header = \\"\\"\\" + ab\\"\\"\\" + [section] + outer.\\"quoted key\\" = { first = { second = { third = { alpha = { beta = {} } } } }, filler = { one = { two = { three = -65079.12231 } }, flag = false, \\"q one\\" = { mid = { leaf = false } }, group = { count = { total = 770012 }, note = { body = \\"first inline note\\" }, a-b = { c = { d = \\"short lit\\" } }, e = { f = 36740 }, \\"q two\\" = { g = false }, empty = \\"\\", \\"q three\\" = { h = 527558 }, \\"q four\\" = { date = { stamp = 2080-12-04 } }, \\"q five\\" = 31256.58522, i = { j = 819597 }, k = 8, l = -284092, MM = 52, n = { o = true } }, \\"q six\\" = { p = { q = 2081-06-28T00:40:23Z } }, \\"q seven\\" = { r = \\"second inline note\\" }, s = { t = {} }, u = { v = 90692.79376 }, w = { \\"q eight\\" = true } } } + " + `); + expect(parse(result)).toEqual(obj); +}); + +// The array of tables entry collapses to a single date value while the +// multiline inline array above it, holding a bigint and -inf, stays put. +test('replacing an AOT subtable with a date beside a multiline inline array (seed 1383962)', () => { + const src = dedent` + "quoted key".items = [{ inner.values = [true, 47577.29573, true, 2077-04-05, 282_582, """ + ml""", 76968.6746, 'literal text', 2069-08-08T04:34:33, "basic text"] }, 574310, 28569, -63757100000000418476, """ + second ml""", -50759.88688, -inf, true] + [[entries]] + label.name = 'another literal' + `; + + const obj = parse(src) as any; + obj.entries[0].label = new Date(Date.UTC(2006, 1, 14)); + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(` + "\\"quoted key\\".items = [{ inner.values = [true, 47577.29573, true, 2077-04-05, 282_582, \\"\\"\\" + ml\\"\\"\\", 76968.6746, 'literal text', 2069-08-08T04:34:33, \\"basic text\\"] }, 574310, 28569, -63757100000000418476, \\"\\"\\" + second ml\\"\\"\\", -50759.88688, -inf, true] + [[entries]] + label = 2006-02-14T00:00:00.000Z" + `); + expect(parse(result)).toEqual(obj); +}); + +// The replacement is deeper than what it replaces, so the container grows +// while a sibling multiline array below it has to keep its own positions. +test('replacing an inline-table member with a nested object in a multiline inline table (seed 1693919)', () => { + const src = dedent` + [outer.inner] + "".middle.target = { + first = { nested = """ + indented text""" }, + "quoted a"."quoted b" = [""" + ml""", nan, "plain text"], + } + `; + + const obj = parse(src) as any; + obj.outer.inner[''].middle.target.first = { alpha: { beta: false, gamma: false, delta: [new Date(Date.UTC(2036, 8, 23)), true, -1287.6224634237587] } }; + + const result = patch(src, obj, { + inlineTableStart: 1, + trailingComma: true, + bracketSpacing: true, + updateOrder: false, + trailingNewline: 1, + newLine: "\r\n", + leadingBom: false, + truncateZeroTimeInDates: false, + useTabsForIndentation: true + }); + expect(result).toMatchInlineSnapshot(` + "[outer.inner] + \\"\\".middle.target = { first = { alpha = { beta = false, gamma = false, delta = [ 2036-09-23T00:00:00.000Z, true, -1287.6224634237587, ], }, }, \\"quoted a\\" = { \\"quoted b\\" = [ \\"ml\\", nan, \\"plain text\\", ], }, } + " + `); + expect(parse(result)).toEqual(obj); +}); + +// Emptying the inline table removes a multiline string, which shortens the +// enclosing array and leaves its closing bracket and comma offsets stale. +test('deleting the only member of an inline table inside a multiline array (seed 175924)', () => { + const src = dedent` + [[outer."=".inner]] + empty = { + } + items = ["plain text", 1988-12-04T04:19:06, [ + 0b000000110001011, + { target = """ + ml""" }, + -907693, + ], 65260.050825] + `; + + const obj = parse(src) as any; + delete obj.outer['='].inner[0].items[2][1].target; + + const result = patch(src, obj, { + trailingComma: false, + bracketSpacing: true, + updateOrder: false, + trailingNewline: 1, + newLine: "\r\n", + leadingBom: true, + truncateZeroTimeInDates: false, + useTabsForIndentation: false + }); + expect(result).toMatchInlineSnapshot(` + "[[outer.\\"=\\".inner]] + empty = { + } + items = [ \\"plain text\\", 1988-12-04T04:19:06, [ 395, {}, -907693 ], 65260.050825 ] + " + `); + expect(parse(result)).toEqual(obj); +}); + +// The removed inline table contains a multiline string, so the elements after +// it in the same array shift by more than one line. +test('deleting a nested inline table inside a multiline inline array (seed 1896226)', () => { + const src = dedent` + top = ''' + lit text''' + [section.a.b] + first.leaf = { one.two.three = true, four.five = "basic one", six.seven.eight = -792779, nine.ten.eleven = 0xd45, twelve = false } + second.middle."quoted" = ["basic two", -836_768, { outer.inner = { "quoted a" = "basic three", nested.leaf = """ + """, deep.path."quoted b" = '', "quoted c" = 0o22, other = 0b010011 } }, { + }, 2004-04-25T17:21:11, -51408.63582, 2006-12-08T20:27:41.624602Z, true, nan, 743736] + `; + + const obj = parse(src) as any; + delete obj.section.a.b.second.middle['quoted'][2].outer.inner; + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(` + "top = ''' + lit text''' + [section.a.b] + first.leaf = { one.two.three = true, four.five = \\"basic one\\", six.seven.eight = -792779, nine.ten.eleven = 0xd45, twelve = false } + second.middle.\\"quoted\\" = [ \\"basic two\\", -836768, { outer = {} }, {}, 2004-04-25T17:21:11, -51408.63582, 2006-12-08T20:27:41.624Z, true, nan, 743736 ]" + `); + expect(parse(result)).toEqual(obj); +}); + +// The deletion empties one branch of a dotted key while its sibling member, +// another multiline literal string, has to keep its own layout. +test('deleting a dotted key inside an inline table in a multiline array (seed 2185943)', () => { + const src = dedent` + top.name = """ + ml""" + [[entries]] + empty = [ + ] + values = [46674.18719, 48559.13327, 2091-08-20T06:58:11, 2024-05-10, 275_068, { + outer = { inner."quoted"."" = ''' + lit''' }, + other = ''' + lit two''', + }, 22787.072880, 79051.79185, true] + "quoted key".middle.leaf = """ + ml two""" + `; + + const obj = parse(src) as any; + delete obj.entries[0].values[5].outer.inner['quoted']; + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(` + "top.name = \\"\\"\\" + ml\\"\\"\\" + [[entries]] + empty = [ + ] + values = [ 46674.18719, 48559.13327, 2091-08-20T06:58:11, 2024-05-10, 275068, { outer = { inner = {} }, other = \\"lit two\\" }, 22787.07288, 79051.79185, true ] + \\"quoted key\\".middle.leaf = \\"\\"\\" + ml two\\"\\"\\"" + `); + expect(parse(result)).toEqual(obj); +}); + +// The inline table becomes empty mid-array, and the array continues with a +// nested array that itself spans lines. +test('deleting the only member of an inline table holding a multiline string (seed 2497422)', () => { + const src = dedent` + "quoted key" = [' ', -78860.81892, { target = """ + ml text""" }, "basic one", 36709.83314, [true, "basic two", 945e+85, -30134.83738, { + }, """ + ml two""", "basic three", 17:57:10, false, false], -31687.66292] + `; + + const obj = parse(src) as any; + delete obj['quoted key'][2].target; + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(`"\\"quoted key\\" = [\\" \\", -78860.81892, {}, \\"basic one\\", 36709.83314, [true, \\"basic two\\", 9.45e+87, -30134.83738, {}, \\"ml two\\", \\"basic three\\", 17:57:10, false, false], -31687.66292]"`); + expect(parse(result)).toEqual(obj); +}); + +// The replacement drops the multiline string the original member held, so the +// array shrinks by a line while later elements keep their positions. +test('replacing an inline table inside a multiline array (seed 2531104)', () => { + const src = dedent` + top.name = ''' + lit text''' + outer = { + inner."quoted" = [true, { "a b".c.d = 115668, e.f."g h" = -92131.88369, flag = false, other.leaf = """ + ml""" }, 745102, 96226.27680, 0o21016, false, 802344, "basic text"], + tail."other q" = 00:16:17, + } + `; + + const obj = parse(src) as any; + obj.outer.inner['quoted'][1] = { alpha: 1640, beta: true }; + + const result = patch(src, obj, { + inlineTableStart: 1, + trailingComma: true, + bracketSpacing: false, + updateOrder: true, + trailingNewline: 1, + newLine: "\n", + leadingBom: true, + truncateZeroTimeInDates: false, + useTabsForIndentation: false, + minimumDecimals: 1 + }); + expect(result).toMatchInlineSnapshot(` + "top.name = ''' + lit text''' + [outer] + inner = {quoted = [true, {alpha = 1640.0, beta = true,}, 745102.0, 96226.2768, 8718.0, false, 802344.0, \\"basic text\\",],} + tail = {\\"other q\\" = 00:16:17,} + " + `); + expect(parse(result)).toEqual(obj); +}); + +// The removed element sits between two multiline basic strings, so the writer +// has to close the gap without disturbing either delimiter. +test('removing a date element from a multiline array of multiline strings (seed 2667551)', () => { + const src = dedent` + [[entries]] + empty = { + } + outer.items = ['', """ + ml one""", 2088-10-24T00:54:56, """ + ml two"""] + `; + + const obj = parse(src) as any; + obj.entries[0].outer.items.splice(2, 1); + + const result = patch(src, obj, { + inlineTableStart: 1, + trailingComma: false, + bracketSpacing: false, + updateOrder: true, + trailingNewline: 1, + newLine: "\n", + leadingBom: false, + truncateZeroTimeInDates: true, + useTabsForIndentation: false + }); + expect(result).toMatchInlineSnapshot(` + "[[entries]] + empty = { + } + outer.items = [\\"\\", \\"ml one\\", \\"ml two\\"] + " + `); + expect(parse(result)).toEqual(obj); +}); + +// The replaced member holds both a multiline literal and a multiline basic +// string, so the container collapses from three lines to one. +test('replacing an inline-table member with a single-key object (seed 2824408)', () => { + const src = dedent` + [build.rollout] + job.settings = { + artifacts = { logpath = "staging", db.host = true, deploy.stages.rollback = ''' + rollback plan a''', credential.id = """ + a generated token for the deploy job""" }, + "a b".r = "checked against the last recorded release set", + } + report.summaries.artifacts = [ + ] + note.tag = { + } + `; + + const obj = parse(src) as any; + obj.build.rollout.job.settings.artifacts = { alpha: -188 }; + + const result = patch(src, obj, { + trailingComma: true, + bracketSpacing: false, + updateOrder: true, + trailingNewline: 2, + newLine: "\n", + leadingBom: false, + truncateZeroTimeInDates: false, + useTabsForIndentation: false, + minimumDecimals: 1 + }); + expect(result).toMatchInlineSnapshot(` + "[build.rollout] + job.settings = {artifacts = {alpha = -188.0,}, \\"a b\\" = {r = \\"checked against the last recorded release set\\",},} + report.summaries.artifacts = [ + ] + note.tag = { + } + + " + `); + expect(parse(result)).toEqual(obj); +}); + +// The array holds a multiline string and ends with an inline table on its own +// lines, both of which have to survive the element removal. +test('removing an element from a multiline array before an inline table (seed 2858114)', () => { + const src = dedent` + outer.values = [true, 22866.4194, 890412, 399573, "one", 0x703714, "two", "three", """ + ml""", { + inner = 59108.35246, + }] + other.name = ''' + lit''' + `; + + const obj = parse(src) as any; + obj.outer.values.splice(5, 1); + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(` + "outer.values = [true, 22866.4194, 890412, 399573, \\"one\\", \\"two\\", \\"three\\", \\"\\"\\" + ml\\"\\"\\", { + inner = 59108.35246, + }] + other.name = ''' + lit'''" + `); + expect(parse(result)).toEqual(obj); +}); + +// The replaced element is a dense inline table spanning two lines; the array +// around it keeps its trailing scalars. +test('replacing an inline-table element inside a multiline array (seed 377453)', () => { + const src = dedent` + outer = { + values = [false, true, ''' + lit with = sign''', -49_687.079945, { a."q one" = "basic one", b.c.d = 0x5c5fa0, "q two" = "basic two", e."q three".f = true, g.h.i = 0o26771, j.k.l = 0xcfe, m.n.o = 15:08:47, p."q four".r = """ + ml""" }, "basic three", 753862, 'z', -64864.2541], + tail.leaf = 0o4755211, + } + `; + + const obj = parse(src) as any; + obj.outer.values[4] = { alpha: -4472 }; + + const result = patch(src, obj, { + trailingComma: true, + bracketSpacing: true, + updateOrder: false, + trailingNewline: 1, + newLine: "\n", + leadingBom: false, + truncateZeroTimeInDates: true, + useTabsForIndentation: false, + minimumDecimals: 1 + }); + expect(result).toMatchInlineSnapshot(` + "[outer] + values = [ false, true, \\"lit with = sign\\", -49687.079945, { alpha = -4472.0, }, \\"basic three\\", 753862.0, \\"z\\", -64864.2541, ] + tail = { leaf = 1301129.0, } + " + `); + expect(parse(result)).toEqual(obj); +}); + +// Two writes land in the same multiline inline array through the nested inline +// table, and they overlap via the array's stale end position. +test('replacing a dotted-key subtable inside a nested inline table (seed 771152)', () => { + const src = dedent` + build.artifacts = [1, ''' + alpha''', 0o34, 2.5, true, [{ meta.notes.summary = """ + detail""" }, 1986-03-02]] + `; + + const obj = parse(src) as any; + obj.build.artifacts[5][0].meta.notes = { width: 3, label: 'measured value' }; + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(`"build.artifacts = [1, \\"alpha\\", 28, 2.5, true, [{meta = {notes = {width = 3, label = \\"measured value\\"}}}, 1986-03-02]]"`); + expect(parse(result)).toEqual(obj); +}); + +// The replaced table is declared after a multiline array of tables value, so +// the whole preceding block has to keep its positions. +test('replacing a table with a scalar after a multiline AOT array (seed 863664)', () => { + const src = dedent` + [report."by target".details] + summary."run label".entries = [ + { size.bytes = 0xd5c289d, built = 2075-01-15T23:10:01.402080, stats.counts = { lines.total = 597050, words.total = 939925, chars."with space" = 785203, files = 465053, dirs = 822110, "ratio a".value = 412e-33, cached = false, delta."ratio b".amount = -282456, mean.load.value = 70302.033641, min.load = -75001.099256, max.load = 54219.46102 }, "group a".index."sub key" = 482015, "group b"."sub b".enabled = true, "group c" = "basic one", tag = 'lit', "".marker."" = { note.body.text = """ + ml""" }, flags."opt in" = false, mask = 0b111111110111000 }, + ''' + ''', + ] + [replaced.sub.leaf] + `; + + const obj = parse(src) as any; + obj.replaced = 'replacement'; + + const result = patch(src, obj, undefined); + expect(result).toMatchInlineSnapshot(` + "replaced = \\"replacement\\" + + [report.\\"by target\\".details] + summary.\\"run label\\".entries = [ + { size.bytes = 0xd5c289d, built = 2075-01-15T23:10:01.402080, stats.counts = { lines.total = 597050, words.total = 939925, chars.\\"with space\\" = 785203, files = 465053, dirs = 822110, \\"ratio a\\".value = 412e-33, cached = false, delta.\\"ratio b\\".amount = -282456, mean.load.value = 70302.033641, min.load = -75001.099256, max.load = 54219.46102 }, \\"group a\\".index.\\"sub key\\" = 482015, \\"group b\\".\\"sub b\\".enabled = true, \\"group c\\" = \\"basic one\\", tag = 'lit', \\"\\".marker.\\"\\" = { note.body.text = \\"\\"\\" + ml\\"\\"\\" }, flags.\\"opt in\\" = false, mask = 0b111111110111000 }, + ''' + ''', + ]" + `); + expect(parse(result)).toEqual(obj); +}); diff --git a/src/__tests__/patch.test.ts b/src/__tests__/patch.test.ts index 5940775e..3bfc35b4 100644 --- a/src/__tests__/patch.test.ts +++ b/src/__tests__/patch.test.ts @@ -5,6 +5,125 @@ import { example } from '../__fixtures__'; import dedent from 'dedent'; import { TomlFormat } from '../toml-format'; +// A `"""` or `'''` can appear in content -- inside a basic string, or in a +// comment -- without the document containing any multiline string at all. The +// cheap pre-filter in hasMultilineStringDelimiter() cannot tell the difference and +// lets these through; hasTransactionCandidate() then finds no multiline inline +// container and verification is skipped. That is the harmless direction, but only +// because the output is right either way, which is what these pin. +describe('multiline delimiters appearing in content', () => { + test('a basic string containing three apostrophes patches normally', () => { + const original = dedent` + note = "contains ''' inside" + port = 8080 + ` + '\n'; + + const updated = parse(original); + updated.port = 9090; + + expect(patch(original, updated)).toBe(dedent` + note = "contains ''' inside" + port = 9090 + ` + '\n'); + }); + + test('a comment containing three apostrophes patches normally', () => { + const original = dedent` + # see ''' for the quoting rules + port = 8080 + ` + '\n'; + + const updated = parse(original); + updated.port = 9090; + + expect(patch(original, updated)).toBe(dedent` + # see ''' for the quoting rules + port = 9090 + ` + '\n'); + }); + + test('a comment containing three quotes patches normally', () => { + const original = dedent` + # see """ for the quoting rules + port = 8080 + ` + '\n'; + + const updated = parse(original); + updated.port = 9090; + + expect(patch(original, updated)).toBe(dedent` + # see """ for the quoting rules + port = 9090 + ` + '\n'); + }); + + // A multiline literal string may hold three double quotes verbatim, so this + // trips the `"""` half of the pre-filter. It is genuinely multiline, but sits + // at the top level rather than inside an inline container, so it is still not + // a transaction candidate. + test('a multiline literal string holding three quotes patches normally', () => { + const original = dedent` + note = ''' + holds """ fine''' + port = 8080 + ` + '\n'; + + const updated = parse(original); + updated.port = 9090; + + expect(patch(original, updated)).toBe(dedent` + note = ''' + holds """ fine''' + port = 9090 + ` + '\n'); + }); + + // Same three quotes inside a multiline inline container, which IS a transaction + // candidate, so this one runs the full verification path. + test('a multiline literal string holding three quotes inside an inline table', () => { + const original = dedent` + cfg = { + note = ''' + holds """ fine''', + retries = 2, + } + port = 8080 + ` + '\n'; + + const updated = parse(original); + updated.cfg.retries = 3; + + expect(patch(original, updated)).toBe(dedent` + cfg = { + note = ''' + holds """ fine''', + retries = 3, + } + port = 8080 + ` + '\n'); + }); + + // A multiline literal string may hold up to two consecutive apostrophes. This + // one is genuinely multiline, but sits at the top level rather than inside an + // inline container, so it is still not a transaction candidate. + test('a multiline literal string holding two apostrophes patches normally', () => { + const original = dedent` + note = ''' + it can hold '' safely''' + port = 8080 + ` + '\n'; + + const updated = parse(original); + updated.port = 9090; + + expect(patch(original, updated)).toBe(dedent` + note = ''' + it can hold '' safely''' + port = 9090 + ` + '\n'); + }); +}); + test('it should apply edit to key-value', () => { const value = parse(example); value.owner.name = 'Tim Hall'; diff --git a/src/__tests__/toml-document.test.ts b/src/__tests__/toml-document.test.ts index 783f2fb9..804b27b1 100644 --- a/src/__tests__/toml-document.test.ts +++ b/src/__tests__/toml-document.test.ts @@ -1909,4 +1909,191 @@ describe('TomlDocument', () => { }); }); }); + + describe('patch result verification', () => { + // Structurally distilled from fuzz seed 771152. The fine-grained writer + // leaves the enclosing array's delimiter offsets stale here and produces + // TOML that does not parse; the internal retry is what saves it. + const trickyToml = dedent` + build.artifacts = [1, ''' + alpha''', 0o34, 2.5, true, [{ meta.notes.summary = """ + detail""" }, 1986-03-02]] + ` + '\n'; + + it('produces valid TOML where the fine-grained writer would not', () => { + const doc = new TomlDocument(trickyToml); + const obj = doc.toJsObject; + obj.build.artifacts[5][0].meta.notes = { width: 3, label: 'measured value' }; + doc.patch(obj); + + // The retry rewrites the whole multiline container, losing the multiline + // literal string and the octal base, but the result is valid TOML and the + // untouched local date keeps its original spelling. + expect(doc.toTomlString).toBe( + 'build.artifacts = [1, "alpha", 28, 2.5, true, ' + + '[{meta = {notes = {width = 3, label = "measured value"}}}, 1986-03-02]]\n' + ); + expect(() => new TomlDocument(doc.toTomlString).toJsObject).not.toThrow(); + }); + + // The getter changes the requested value on every read, so no output can ever + // round-trip. Neither attempt can satisfy it, and the document commits the + // fine-grained result rather than throwing, which is what earlier versions + // produced. The point is that it stays usable afterwards. + it('stays usable when no attempt can round-trip', () => { + const doc = new TomlDocument('value = 0\n'); + let reads = 0; + const updated: Record = {}; + Object.defineProperty(updated, 'value', { enumerable: true, get: () => ++reads }); + + expect(() => doc.patch(updated)).not.toThrow(); + expect(doc.toTomlString).toMatch(/^value = \d+\n$/); + + const obj = doc.toJsObject; + obj.value = 7; + doc.patch(obj); + expect(doc.toTomlString).toBe('value = 7\n'); + }); + + // TOML has one integer type, so a document read as bigint accepts a plain + // number assigned back into it. Comparing 2 against 2n as different values + // would fail validation on a correct patch and destroy the formatting via a + // needless retry. + it.each([ + ['asNeeded' as const, 'bigint'], + [true as const, 'bigint'], + [false as const, 'number'] + ])('does not retry when integersAsBigInt is %j', (integersAsBigInt, expectedType) => { + const original = dedent` + big = 9007199254740993 + note = [ + """ + line""", + 1, + ] + x = 1 + ` + '\n'; + + const doc = new TomlDocument(original, { integersAsBigInt }); + const obj = doc.toJsObject; + expect(typeof obj.big).toBe(expectedType); + + obj.x = 2; + doc.patch(obj); + + // A retry would reflow the multiline array, so the indented `"""` and the + // trailing comma surviving proves the fine-grained path was kept. + const expectedBig = integersAsBigInt === false ? '9007199254740992.0' : '9007199254740993'; + expect(doc.toTomlString).toBe(dedent` + big = ${expectedBig} + note = [ + """ + line""", + 1, + ] + x = 2 + ` + '\n'); + }); + }); + + + // toJsObject() hands out plain Date objects, but the writer derives a value's + // TOML text from toISOString(), which the TOML date classes override to return + // the original spelling. A plain Date handed back therefore used to write as a + // full offset date-time: a single sibling edit in an inline array rewrote every + // date in it, e.g. `1986-03-02` became `1986-03-02T00:00:00.000Z`. + describe('date representation through read-modify-write', () => { + const stamps = 'stamps = [ 1986-03-02, 07:32:00, 1979-05-27T07:32:00, 1979-05-27T07:32:00Z, "tail" ]\n'; + + it('keeps every date kind when only a sibling changes', () => { + const doc = new TomlDocument(stamps); + const obj = doc.toJsObject; + obj.stamps[4] = 'changed'; + doc.patch(obj); + + expect(doc.toTomlString).toBe( + 'stamps = [ 1986-03-02, 07:32:00, 1979-05-27T07:32:00, 1979-05-27T07:32:00Z, "changed" ]\n' + ); + }); + + it('keeps a date when the value beside it forces the container to be rewritten', () => { + const original = dedent` + list = [ 1986-03-02, ''' + alpha''', "tail" ] + ` + '\n'; + + const doc = new TomlDocument(original); + const obj = doc.toJsObject; + obj.list[2] = 'changed'; + doc.patch(obj); + + expect(doc.toTomlString).toBe(dedent` + list = [ 1986-03-02, ''' + alpha''', "changed" ] + ` + '\n'); + }); + + // A caller who changes the instant may or may not still mean a local date, + // and a plain Date cannot express which. The value widens, and the exported + // date classes are how the caller says what they meant. + it('widens a date whose instant actually changed', () => { + const doc = new TomlDocument(stamps); + const obj = doc.toJsObject; + obj.stamps[0] = new Date(Date.UTC(2001, 0, 2)); + doc.patch(obj); + + expect(doc.toTomlString).toBe( + 'stamps = [ 2001-01-02T00:00:00.000Z, 07:32:00, 1979-05-27T07:32:00, 1979-05-27T07:32:00Z, "tail" ]\n' + ); + }); + + it('honours a LocalDate supplied by the caller', () => { + const doc = new TomlDocument(stamps); + const obj = doc.toJsObject; + obj.stamps[0] = new LocalDate('2001-01-02'); + doc.patch(obj); + + expect(doc.toTomlString).toBe( + 'stamps = [ 2001-01-02, 07:32:00, 1979-05-27T07:32:00, 1979-05-27T07:32:00Z, "tail" ]\n' + ); + }); + + // Known limitation: the pre-patch values are matched by position, so a date + // that moves no longer lines up with its original node and widens. Pinned so + // the boundary is visible rather than discovered. + it('widens dates that swap positions', () => { + const original = 'window = [ 1986-03-02, 1991-07-14, "tail" ]\n'; + const doc = new TomlDocument(original); + const obj = doc.toJsObject; + [obj.window[0], obj.window[1]] = [obj.window[1], obj.window[0]]; + doc.patch(obj); + + expect(doc.toTomlString).toBe( + 'window = [ 1991-07-14T00:00:00.000Z, 1986-03-02T00:00:00.000Z, "tail" ]\n' + ); + }); + + it('leaves the caller object untouched', () => { + const doc = new TomlDocument(stamps); + const obj = doc.toJsObject; + obj.stamps[4] = 'changed'; + doc.patch(obj); + + // Still the plain Date toJsObject handed out, not the internal class. + expect(obj.stamps[0].constructor).toBe(Date); + expect(obj.stamps[0]).toBeInstanceOf(Date); + }); + + it('still exposes plain Date objects from toJsObject', () => { + const doc = new TomlDocument(stamps); + const dates = doc.toJsObject.stamps.slice(0, 4); + + for (const value of dates) { + expect(value.constructor).toBe(Date); + } + // The standard Date contract, which the TOML classes deliberately break. + expect(dates[0].toISOString()).toBe('1986-03-02T00:00:00.000Z'); + }); + }); + }); diff --git a/src/__tests__/toml-format.test.ts b/src/__tests__/toml-format.test.ts index 1eec0d19..e3d9e4c0 100644 --- a/src/__tests__/toml-format.test.ts +++ b/src/__tests__/toml-format.test.ts @@ -1,5 +1,5 @@ import { TomlFormat, detectNewline, countTrailingNewlines, validateFormatObject, resolveTomlFormat } from '../toml-format'; -import { patch } from '../index'; +import { patch, stringify } from '../index'; import parseTOML from '../parse-toml'; import toTOML from '../to-toml'; import { stripLeadingBom } from '../decode-utf8'; @@ -128,13 +128,10 @@ describe('TomlFormat comprehensive tests', () => { expect(format.bracketSpacing).toBe(true); // Default value }); - test('should handle empty string as newLine', () => { - const format = new TomlFormat('', 1); - - expect(format.newLine).toBe(''); - expect(format.trailingNewline).toBe(1); - expect(format.trailingComma).toBe(false); // Default value - expect(format.bracketSpacing).toBe(true); // Default value + // An empty newLine runs every line together (`a = 1b = 2`), so the + // constructor rejects it rather than producing TOML that cannot parse. + test('should reject empty string as newLine', () => { + expect(() => new TomlFormat('', 1)).toThrow('Invalid newLine value: expected LF or CRLF'); }); test('should handle zero as trailingNewline', () => { @@ -723,4 +720,86 @@ describe('updateOrder option wiring (docs/PLAN-Update-Order.md)', () => { expect(spy).not.toHaveBeenCalled(); spy.mockRestore(); }); -}); \ No newline at end of file +}); + +// Before this moved into resolveTomlFormat, only patch() normalized newLine. +// stringify() took the raw value, so `{ newLine: 'LF' }` wrote the literal text +// "LF" between lines and `{ newLine: '\r' }` produced TOML that cannot parse, +// both silently. These cover every entry point that resolves a format. +describe('newLine normalization', () => { + const config = { title: 'example', port: 8080 }; + + describe.each([ + ['LF', '\n'], + ['CRLF', '\r\n'], + ['lf', '\n'], + ['CrLf', '\r\n'], + ['unix', '\n'], + ['DOS', '\r\n'], + ['\n', '\n'], + ['\r\n', '\r\n'], + ['\n', '\n'], + ['\r\n', '\r\n'] + ])('accepts %j as %j', (newLine, expected) => { + it('via stringify', () => { + expect(stringify(config, { newLine })).toBe(`title = "example"${expected}port = 8080${expected}`); + }); + + it('via patch', () => { + const original = 'title = "example"\nport = 8080\n'; + expect(patch(original, { title: 'example', port: 9090 }, { newLine })) + .toBe(`title = "example"${expected}port = 9090${expected}`); + }); + + it('via the TomlFormat constructor', () => { + expect(new TomlFormat(newLine).newLine).toBe(expected); + }); + + it('via validateFormatObject', () => { + expect(validateFormatObject({ newLine }).newLine).toBe(expected); + }); + }); + + // '\r' alone is a legal string but not a legal TOML line ending, and '' runs + // every line together. Both used to pass straight through to the writer. + describe.each(['\r', '\r', '', 'invalid', 'CR', '\n\n'])('rejects %j', (newLine) => { + const message = 'Invalid newLine value: expected LF or CRLF'; + + it('via stringify', () => { + expect(() => stringify(config, { newLine })).toThrow(message); + }); + + it('via patch', () => { + expect(() => patch('title = "example"\n', { title: 'other' }, { newLine })).toThrow(message); + }); + + it('via the TomlFormat constructor', () => { + expect(() => new TomlFormat(newLine)).toThrow(message); + }); + }); + + it('reports a non-string newLine as a type error, not a value error', () => { + expect(() => stringify({ title: 'example' }, { newLine: 42 as any })) + .toThrow('Invalid types for format properties: newLine (expected string, got number)'); + }); + + it('treats a missing newLine as absent rather than invalid', () => { + expect(new TomlFormat().newLine).toBe('\n'); + expect(new TomlFormat(undefined).newLine).toBe('\n'); + expect(new TomlFormat(null as any).newLine).toBe('\n'); + expect(validateFormatObject({}).newLine).toBeUndefined(); + }); + + // A TomlFormat instance is returned from resolveTomlFormat untouched, so the + // constructor is the only place that can normalize what it carries. + it('normalizes through a TomlFormat instance passed to stringify', () => { + expect(stringify(config, new TomlFormat('CRLF'))) + .toBe('title = "example"\r\nport = 8080\r\n'); + }); + + it('does not resolve aliases through Object.prototype', () => { + for (const newLine of ['constructor', 'toString', 'valueOf', '__proto__']) { + expect(() => new TomlFormat(newLine)).toThrow('Invalid newLine value: expected LF or CRLF'); + } + }); +}); diff --git a/src/diff.ts b/src/diff.ts index dc4629e7..6a8e23f6 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -30,6 +30,8 @@ export function isEdit(change: Change): change is Edit { export interface Remove { type: ChangeType.Remove; path: Path; + /** Internal marker for array removes whose index refers to the original array. */ + coordinate?: 'source'; } export function isRemove(change: Change): change is Remove { return change.type === ChangeType.Remove; @@ -327,6 +329,11 @@ function compareArrays(before: any[], after: any[], path: Path = [], options: Di return false; }; const multilineArray = before.some(hasMultilineValue) || after.some(hasMultilineValue); + const hasInlineObject = (value: any) => + value !== null && typeof value === 'object' && !Array.isArray(value) && !(value instanceof Date); + const hasMixedStringAndObject = (values: any[]) => + values.length > 4 && values.some(value => typeof value === 'string') && values.some(hasInlineObject); + const layoutSensitiveArray = multilineArray || hasMixedStringAndObject(before) || hasMixedStringAndObject(after); // Simulation of the actual VALUES, mutated in lockstep with before_stable. // The "removed -> edited in place" branch below must diff the element the @@ -345,6 +352,15 @@ function compareArrays(before: any[], after: any[], path: Path = [], options: Di // descending index order (see reorder() in patch.ts), so each emitted // index must stay valid against the un-shifted array. let removedBefore = 0; + let sameArrayAddEmitted = false; + + const removeChange = (index: number): Remove => { + const change: Remove = { type: ChangeType.Remove, path: path.concat(index) }; + if (!sameArrayAddEmitted || removedBefore > 0) { + Object.defineProperty(change, 'coordinate', { value: 'source' }); + } + return change; + }; // 2. Step through after array making changes to before array as-needed for (let index = 0; index < after_stable.length; index++) { @@ -383,11 +399,8 @@ function compareArrays(before: any[], after: any[], path: Path = [], options: Di // from a multiline inline array dropped a line of a multiline string; // fuzz seed 35943: removing one of several duplicate scalars above a // nested multiline array chain-moved the array and corrupted its tail). - if (multilineArray && (after_stable.indexOf(before_stable[index]) === -1 || surplusDuplicate)) { - changes.push({ - type: ChangeType.Remove, - path: path.concat(index + removedBefore) - }); + if (layoutSensitiveArray && (after_stable.indexOf(before_stable[index]) === -1 || surplusDuplicate)) { + changes.push(removeChange(index + removedBefore)); before_stable.splice(index, 1); before_sim.splice(index, 1); removedBefore++; @@ -407,10 +420,7 @@ function compareArrays(before: any[], after: any[], path: Path = [], options: Di // `multilineArray` is set, but kept for the non-multiline case where a // prior splice already shifted indices and the surplus is unambiguous.) if (removedBefore > 0 && surplusDuplicate) { - changes.push({ - type: ChangeType.Remove, - path: path.concat(index + removedBefore) - }); + changes.push(removeChange(index + removedBefore)); before_stable.splice(index, 1); before_sim.splice(index, 1); removedBefore++; @@ -425,11 +435,12 @@ function compareArrays(before: any[], after: any[], path: Path = [], options: Di // anyway, and the writer corrupts their content (fuzz seed 62263: a // nested array above a multiline inline table replaced by a duplicate // scalar emitted Remove + Move + Add and mangled the table). - if (multilineArray && removedBefore > 0) { + if (layoutSensitiveArray && removedBefore > 0) { changes.push({ type: ChangeType.Add, path: path.concat(index) }); + sameArrayAddEmitted = true; before_stable.splice(index, 0, value); before_sim.splice(index, 0, after[index]); // The fresh copy is spliced in BEFORE the original (at `from`), so it @@ -498,6 +509,7 @@ function compareArrays(before: any[], after: any[], path: Path = [], options: Di type: ChangeType.Add, path: path.concat(index) }); + sameArrayAddEmitted = true; before_stable.splice(index, 0, value); before_sim.splice(index, 0, after[index]); // Same source-index bookkeeping as the refused-move Add above: splicing a @@ -512,10 +524,7 @@ function compareArrays(before: any[], after: any[], path: Path = [], options: Di // 3. Remove any remaining overflow items for (let i = after_stable.length; i < before_stable.length; i++) { - changes.push({ - type: ChangeType.Remove, - path: path.concat(i + removedBefore) - }); + changes.push(removeChange(i + removedBefore)); } return changes; diff --git a/src/patch-validate.ts b/src/patch-validate.ts new file mode 100644 index 00000000..373a0cb4 --- /dev/null +++ b/src/patch-validate.ts @@ -0,0 +1,115 @@ +import parseTOML from './parse-toml'; +import toJS from './to-js'; +import { stripLeadingBom } from './decode-utf8'; +import { isTemporal, stableStringify } from './utils'; +import type { IntegersAsBigInt } from './parse-options'; + +/** + * How the produced TOML should be read back when verifying a patch. + * + * Both fields must describe the SAME representation the caller's object uses, + * otherwise the comparison reports a mismatch for a correct patch and forces a + * needless retry. `temporal` therefore follows the updated object (see + * {@link hasTemporal}) rather than any parse-time setting, and + * `integersAsBigInt` must match whatever produced the object being compared. + */ +export interface PatchComparison { + /** Read date/time values back as Temporal objects rather than Date subclasses. */ + temporal: boolean; + /** Read integers back the way the caller's object represents them. Default: 'asNeeded'. */ + integersAsBigInt?: IntegersAsBigInt; +} + +/** + * Whether the source contains a multiline string delimiter anywhere. + * + * This is only a cheap pre-filter, not the decision: it says nothing about where + * the delimiter sits. `hasTransactionCandidate()` in patch.ts is what checks the + * condition that actually matters, namely a multiline string inside a multiline + * inline container, and the two are used together. This runs first because it is + * two indexOf calls against a string that is already in hand, and it rules out + * most documents before anything walks the tree. + * + * Sound as a pre-filter because TOML has no other way to spell a string that + * spans lines: a raw newline is rejected inside single-quoted and double-quoted + * strings, and a line-ending backslash is only legal within `"""` delimiters. So a + * String node whose span crosses lines implies one of these delimiters is present. + * False positives are fine and cost only the tree walk; a false negative would + * silently skip verification, which is why the check is stated this loosely. + */ +export function hasMultilineStringDelimiter(existing: string): boolean { + return existing.indexOf('"""') !== -1 || existing.indexOf("'''") !== -1; +} + +/** + * Recursively checks if an object graph contains any Temporal values. + * Used to auto-detect whether temporal mode should be enabled for patching, + * and to decide how date/time values are read back when verifying a result. + * Cycle-safe. + */ +export function hasTemporal(obj: any, seen: WeakSet = new WeakSet()): boolean { + if (obj == null || typeof obj !== 'object') return false; + if (isTemporal(obj)) return true; + if (seen.has(obj)) return false; + seen.add(obj); + for (const v of Object.values(obj)) { + if (hasTemporal(v, seen)) return true; + } + return false; +} + +/** + * Re-parses TOML that a patch produced and reports whether it round-trips back + * to `updated`. A false result means the written TOML disagrees with the object + * it was supposed to represent, or is not parseable at all. + */ +export function patchResultMatches(updated: any, toml: string, comparison: PatchComparison): boolean { + try { + const parsed = Array.from(parseTOML(stripLeadingBom(toml))); + const actual = toJS(parsed, '', { + temporal: comparison.temporal, + integersAsBigInt: comparison.integersAsBigInt ?? 'asNeeded' + }); + // normalizePatchComparison is idempotent, so `updated` is normalized once + // here rather than twice as it was when `expected` was a separate local. + return stableStringify(normalizePatchComparison(actual)) === stableStringify(normalizePatchComparison(updated)); + } catch { + return false; + } +} + +function normalizePatchComparison(value: any): any { + if (value === undefined) return undefined; + if (typeof value === 'string') return value.replace(/\r\n?/g, '\n'); + if (Array.isArray(value)) return value.map(normalizePatchComparison); + // TOML has a single integer type, so `2` and `2n` denote the same value and + // must compare equal: a document read with integersAsBigInt yields bigints, + // and assigning a plain number back into it is legitimate. Canonicalising + // both to a decimal string keeps that working while still catching genuine + // precision loss, where the two decimal strings differ. + if (typeof value === 'bigint') return `Int:${value.toString()}`; + if (typeof value === 'number' && Number.isInteger(value)) return `Int:${BigInt(value).toString()}`; + if (value instanceof Date) { + if (value.getUTCFullYear() <= 0) { + const hours = String(value.getUTCHours()).padStart(2, '0'); + const minutes = String(value.getUTCMinutes()).padStart(2, '0'); + const seconds = String(value.getUTCSeconds()).padStart(2, '0'); + const milliseconds = String(value.getUTCMilliseconds()).padStart(3, '0'); + return `Time:${hours}:${minutes}:${seconds}.${milliseconds}`; + } + return `Date:${value.getTime()}`; + } + if (isTemporal(value)) return value.toString(); + if (value && typeof value.toJSON === 'function') { + return normalizePatchComparison(value.toJSON()); + } + if (value && typeof value === 'object') { + const normalized: Record = {}; + for (const key of Object.keys(value)) { + const normalizedValue = normalizePatchComparison(value[key]); + if (normalizedValue !== undefined) normalized[key] = normalizedValue; + } + return normalized; + } + return value; +} diff --git a/src/patch.ts b/src/patch.ts index 66d1ca9f..78a221d5 100644 --- a/src/patch.ts +++ b/src/patch.ts @@ -53,6 +53,7 @@ import { } from './comment-alignment'; import { getSpan } from './location'; import { stripLeadingBom, UTF8_BOM } from './decode-utf8'; +import { hasTemporal, hasMultilineStringDelimiter, patchResultMatches } from './patch-validate'; import traverse from './traverse'; /** @@ -63,6 +64,12 @@ import traverse from './traverse'; * and updated data, then strategically applies only the necessary changes to maintain the * original document structure as much as possible. * + * The result is verified internally: the output is re-parsed and checked against + * `updated`, and if it does not round-trip the patch is retried with a coarser + * writer that rewrites whole multiline inline containers. A patch that neither + * attempt can satisfy returns the fine-grained result, so this never fails where + * earlier versions succeeded. + * * @param existing - The original TOML document as a string * @param updated - The updated JavaScript object with desired changes * @param format - Optional formatting options to apply to new or modified sections @@ -86,23 +93,28 @@ export default function patch(existing: string, updated: any, format?: Partial (fmt.leadingBom ? `${UTF8_BOM}${toml}` : toml); -/** - * Recursively checks if an object graph contains any Temporal values. - * Used to auto-detect whether temporal mode should be enabled for patching. - */ -function hasTemporal(obj: any, seen: WeakSet = new WeakSet()): boolean { - if (obj == null || typeof obj !== 'object') return false; - if (isTemporal(obj)) return true; - if (seen.has(obj)) return false; - seen.add(obj); - for (const v of Object.values(obj)) { - if (hasTemporal(v, seen)) return true; - } - return false; + if (!needsVerification) return withBom(patchedToml); + + // Detected once and threaded into both comparisons; patchCst() derives its + // own copy internally for the diff. + const comparison = { temporal: hasTemporal(updated) }; + if (patchResultMatches(updated, patchedToml, comparison)) return withBom(patchedToml); + + const retryCst = Array.from(parseTOML(stripLeadingBom(existing), createNewlineScanState())); + const retriedToml = patchCst(retryCst, updated, fmt, true).tomlString; + // Neither attempt round-trips: return the fine-grained result, which is what + // earlier versions produced. Nothing here makes the output worse than before. + if (!patchResultMatches(updated, retriedToml, comparison)) return withBom(patchedToml); + return withBom(retriedToml); } /** @@ -253,7 +265,45 @@ function normalizeAotEntryComments(doc: Document): void { } } -export function patchCst(existing_cst: CST, updated: any, format: TomlFormat): { tomlString: string; document: Document } { +/** + * Sound over-approximation of "the transactional retry could change the output". + * + * The planner only produces a transaction for a multiline inline container that + * holds a multiline string and carries no comment. If the document has no such + * container, the retry reproduces the first attempt exactly, so verifying it + * cannot change what patch() returns. One walk of a CST that is already parsed is + * far cheaper than the re-parse and structural comparison it avoids. + * + * Deliberately looser than the planner in two ways: it does not exclude containers + * holding a comment, and it does not check that a change actually lands inside one. + * Both would narrow it further, and both are easy to get subtly wrong; erring wide + * only costs a verification that turns out to be unnecessary, whereas erring narrow + * would skip one that was needed. + */ +export function hasTransactionCandidate(cst: CST): boolean { + let found = false; + const spansLines = (node: TreeNode) => node.loc.end.line > node.loc.start.line; + + const scan = (node: TreeNode, insideMultilineContainer: boolean): void => { + if (found) return; + if (isString(node) && spansLines(node) && insideMultilineContainer) { + found = true; + return; + } + const nested = insideMultilineContainer || + ((isInlineTable(node) || isInlineArray(node)) && spansLines(node)); + if (isKeyValue(node)) scan(node.value, nested); + else if (isInlineItem(node)) scan(node.item, nested); + else if (hasItems(node)) { + for (const item of node.items as TreeNode[]) scan(item, nested); + } + }; + + for (const block of cst) scan(block as TreeNode, false); + return found; +} + +export function patchCst(existing_cst: CST, updated: any, format: TomlFormat, useMultilineTransactions = false): { tomlString: string; document: Document } { const items = [...existing_cst]; // Auto-detect Temporal in the updated JS object so that the internal @@ -324,7 +374,7 @@ export function patchCst(existing_cst: CST, updated: any, format: TomlFormat): { // stay eligible for R2 too, even though its object identity postdates the snapshot. const commentEligibleNodes = collectPrePatchNodes(existing_document); - const patched_document = applyChanges(existing_document, updated_document, changes, format, useTemporal, commentEligibleNodes, updated); + const patched_document = applyChanges(existing_document, updated_document, changes, format, useTemporal, commentEligibleNodes, updated, useMultilineTransactions); const tomlString = normalizeInlineCommentAlignmentInString( patched_document, toTOML(patched_document.items, format), @@ -366,6 +416,28 @@ function reorder(changes: Change[]): Change[] { if (isAdd(next_change)) { const bIdx = last(next_change.path); if (typeof bIdx === 'number' && arraysEqual(aPrefix, next_change.path.slice(0, -1))) { + // A source-coordinate remove remains valid against the original array and + // therefore has to run before a same-array Add, even when the diff emitted the + // remove after that Add (seed 2591153). Post-shift removes deliberately stay on + // the far side of the Add (seed 1137525). + if (change.coordinate === 'source') { + let sourceRemove = -1; + for (let k = j + 1; k < changes.length; k++) { + const candidate = changes[k]; + if (isRemove(candidate) + && candidate.coordinate === 'source' + && arraysEqual(aPrefix, candidate.path.slice(0, -1))) { + sourceRemove = k; + break; + } + } + if (sourceRemove !== -1) { + const sourceChange = changes.splice(sourceRemove, 1)[0]; + changes.splice(i, 0, sourceChange); + i = -1; + break; + } + } break; } j++; @@ -702,7 +774,7 @@ function preserveFormatting(existing: Value, replacement: Value): void { * const result = applyChanges(originalDoc, updatedDoc, changes, format); * ``` */ -function applyChanges(original: Document, updated: Document, changes: Change[], format: TomlFormat, temporal: boolean = false, commentEligibleNodes: WeakSet = new WeakSet(), rawUpdated: any = undefined): Document { +function applyChanges(original: Document, updated: Document, changes: Change[], format: TomlFormat, temporal: boolean = false, commentEligibleNodes: WeakSet = new WeakSet(), rawUpdated: any = undefined, useMultilineTransactions = false): Document { // Track AOT keys whose entries were all removed so we can insert empty arrays. Keyed by // the dotted name for de-duplication, but carrying the path segments — a nested key like // [[a.b]] has to be re-materialised as `a.b = []`, not as a root key named `a.b`. @@ -1025,6 +1097,119 @@ function applyChanges(original: Document, updated: Document, changes: Change[], // never call insert()/remove(), which would re-dirty offsets nothing downstream flushes). const objectMoves: Move[] = []; + // Several writes into one multiline inline container can overlap through the container's + // stale end position even when each write is flushed individually. Treat only those + // multi-change, comment-free containers transactionally; a single edit still uses the + // formatting-preserving fine-grained path and comments retain their ownership handling. + const multilineChangePaths = new Map(); + const multilineChanges = new Map(); + const multilineHasUnresolvedChange = new Set(); + const multilineAncestors = new WeakMap(); + const indexMultilineAncestors = (node: TreeNode, ancestors: TreeNode[] = []) => { + const nextAncestors = (isInlineTable(node) || isInlineArray(node)) && + node.loc.end.line > node.loc.start.line + ? [...ancestors, node] + : ancestors; + multilineAncestors.set(node, nextAncestors); + if (isKeyValue(node)) { + indexMultilineAncestors(node.value, nextAncestors); + } else if (isInlineItem(node)) { + indexMultilineAncestors(node.item, nextAncestors); + } else if (hasItems(node)) { + for (const item of node.items as TreeNode[]) indexMultilineAncestors(item, nextAncestors); + } + }; + const commentCache = new WeakMap(); + const multilineStringCountCache = new WeakMap(); + function containsComment(node: TreeNode): boolean { + const cached = commentCache.get(node); + if (cached !== undefined) return cached; + let result: boolean; + if (isComment(node)) result = true; + else if (isKeyValue(node)) result = containsComment(node.value); + else if (isInlineItem(node)) result = containsComment(node.item); + else result = hasItems(node) && (node.items as TreeNode[]).some(containsComment); + commentCache.set(node, result); + return result; + } + function multilineStringCount(node: TreeNode): number { + const cached = multilineStringCountCache.get(node); + if (cached !== undefined) return cached; + let result: number; + if (isString(node)) result = node.loc.end.line > node.loc.start.line ? 1 : 0; + else if (isKeyValue(node)) result = multilineStringCount(node.value); + else if (isInlineItem(node)) result = multilineStringCount(node.item); + else result = hasItems(node) + ? (node.items as TreeNode[]).reduce((count, item) => count + multilineStringCount(item), 0) + : 0; + multilineStringCountCache.set(node, result); + return result; + } + // Only the transactional pass consumes any of this, and building it walks the + // whole document plus every change, so skip it entirely on the fine-grained + // pass. The maps stay empty and transactionalPaths below comes out empty. + if (useMultilineTransactions) { + indexMultilineAncestors(original); + for (const change of changes) { + let target = tryFindByPath(original, change.path); + const unresolved = !target; + if (!target) { + // An Add earlier in the same array can make a later Remove path refer to + // post-mutation coordinates. Resolve its nearest existing ancestor so + // both changes still contribute to the same transaction (seed 175924). + for (let length = change.path.length - 1; length >= 0 && !target; length--) { + target = tryFindByPath(original, change.path.slice(0, length)); + } + } + if (!target) continue; + const ancestors = multilineAncestors.get(target) ?? []; + const targetIsMultiline = (isInlineTable(target) || isInlineArray(target)) && + target.loc.end.line > target.loc.start.line; + + for (const container of targetIsMultiline ? [...ancestors, target] : ancestors) { + if (containsComment(container) || multilineStringCount(container) < 1) continue; + const containerChanges = multilineChanges.get(container); + if (containerChanges) containerChanges.push(change); + else multilineChanges.set(container, [change]); + if (unresolved) multilineHasUnresolvedChange.add(container); + const path = absolutePathOf(container); + if (path !== undefined) multilineChangePaths.set(container, path); + } + } + } + const transactionalPaths = [...multilineChanges] + .filter(([container, containerChanges]) => { + if (!multilineChangePaths.has(container)) return false; + const count = containerChanges.length; + const hasMoveOrAdd = containerChanges.some(change => isMove(change) || isAdd(change)); + const strings = multilineStringCount(container); + const hasEnoughStrings = strings >= 2 || + (strings >= 1 && (multilineHasUnresolvedChange.has(container) || count === 1)); + return hasEnoughStrings && + (count >= 3 || (count === 2 && (!hasMoveOrAdd || multilineHasUnresolvedChange.has(container))) || + (count === 1 && containerChanges.some(isRemove))); + }) + .map(([container]) => multilineChangePaths.get(container)!) + .filter((path, index, paths) => !paths.some((other, otherIndex) => + otherIndex !== index && path.length > other.length && + arraysEqual(path.slice(0, other.length), other) + )); + if (useMultilineTransactions && typeof process !== 'undefined' && process.env.TOML_PATCH_DEBUG_TRANSACTION) { + console.warn([...multilineChanges].map(([container, containerChanges]) => ({ + type: container.type, + path: multilineChangePaths.get(container), + count: containerChanges.length, + strings: multilineStringCount(container), + changes: containerChanges.map(change => change.type) + }))); + } + if (transactionalPaths.length > 0) { + changes = changes.filter(change => !transactionalPaths.some(path => + change.path.length >= path.length && arraysEqual(change.path.slice(0, path.length), path) + )); + changes.push(...transactionalPaths.map(path => ({ type: ChangeType.Edit as const, path }))); + } + // Potential Changes: // // Add: Add key-value to object, add item to array diff --git a/src/toml-document.ts b/src/toml-document.ts index 486cd319..0ce48cc7 100644 --- a/src/toml-document.ts +++ b/src/toml-document.ts @@ -2,11 +2,13 @@ import parseTOML, { continueParsingTOML } from './parse-toml'; import toJS from './to-js'; import { TomlFormat } from './toml-format'; import { Block } from './cst'; -import { patchCst } from './patch'; +import { patchCst, hasTransactionCandidate } from './patch'; +import { hasTemporal, hasMultilineStringDelimiter, patchResultMatches } from './patch-validate'; import { detectNewline, resolveTomlFormat } from './toml-format'; import { truncateCst } from './truncate'; import type { ParseOptions, IntegersAsBigInt } from './parse-options'; import { decodeUtf8Bytes, hasLeadingBom, stripLeadingBom, UTF8_BOM } from './decode-utf8'; +import { isObject } from './utils'; /** * TomlDocument encapsulates a TOML CST and provides methods to interact with it. @@ -74,6 +76,12 @@ export class TomlDocument { /** * Applies a patch to the current CST using a modified JS object. * Updates the internal CST. Use toTomlString getter to retrieve the updated TOML string. + * + * The result is verified internally: the produced TOML is re-parsed and checked + * against `updatedObject`, retrying with a coarser writer if it does not match. + * A patch that neither attempt can satisfy commits the fine-grained result, with + * the CST re-derived from it so the document and its TOML string stay in step. + * * @param updatedObject - The modified JS object to patch with * @param format - Optional formatting options */ @@ -81,14 +89,70 @@ export class TomlDocument { const fmt = resolveTomlFormat(format, this._format); - const { tomlString, document } = patchCst( + // patchCst() mutates the nodes it is handed, so this._cst is already spent + // once the call below returns, whether or not the result is usable. Keeping + // the pre-patch source lets every later path re-derive a clean CST from it. + const sourceBefore = this._currentTomlString; + // Decided before patchCst() runs, because it mutates these nodes. The cheap + // string scan short-circuits before the CST walk. + const needsVerification = hasMultilineStringDelimiter(sourceBefore) && hasTransactionCandidate(this._cst); + + // Only worth deriving the pre-patch values when a stripped date could + // actually be present; temporal mode hands out Temporal objects untouched. + if (!this._temporal && hasPlainDate(updatedObject)) { + const originalObject = toJS(this._cst, sourceBefore, { + integersAsBigInt: this._integersAsBigInt, + temporal: false + }); + updatedObject = restoreDateRepresentations(updatedObject, originalObject); + } + + const first = patchCst( this._cst, updatedObject, fmt ); - this._cst = document.items; - this._format = fmt; - this._currentTomlString = tomlString; + + const commit = (tomlString: string, cst: Block[]) => { + this._cst = cst; + this._format = fmt; + this._currentTomlString = tomlString; + }; + + if (!needsVerification) { + commit(first.tomlString, first.document.items); + return; + } + + // The comparison has to read values back the way this document produces + // them, or a correct patch looks like a mismatch: integersAsBigInt is the + // document's own setting, and temporal follows the updated object exactly + // as the internal diff does. + const comparison = { + temporal: hasTemporal(updatedObject), + integersAsBigInt: this._integersAsBigInt + }; + + if (patchResultMatches(updatedObject, first.tomlString, comparison)) { + commit(first.tomlString, first.document.items); + return; + } + + const retried = patchCst( + Array.from(parseTOML(sourceBefore)), + updatedObject, + fmt, + true + ); + if (patchResultMatches(updatedObject, retried.tomlString, comparison)) { + commit(retried.tomlString, retried.document.items); + return; + } + + // Neither attempt round-trips. Commit the fine-grained result, which is what + // earlier versions produced, re-deriving the tree from it so the document's + // CST and its TOML string stay in agreement. + commit(first.tomlString, Array.from(parseTOML(first.tomlString))); } /** @@ -183,6 +247,76 @@ export class TomlDocument { } } +/** + * True when `value` holds a plain `Date` anywhere, i.e. one of the objects + * toJsObject() produced by stripping a TOML date class. Used to skip the work + * in restoreDateRepresentations() for the common case of a document with no + * dates in it. Cycle-safe. + */ +function hasPlainDate(value: any, seen: WeakSet = new WeakSet()): boolean { + if (value instanceof Date) return value.constructor === Date; + if (!value || typeof value !== 'object') return false; + if (seen.has(value)) return false; + seen.add(value); + if (Array.isArray(value)) return value.some(item => hasPlainDate(item, seen)); + return Object.keys(value).some(key => hasPlainDate(value[key], seen)); +} + +/** + * Re-attaches the TOML date representation that toJsObject() strips. + * + * toJsObject() deliberately hands out plain `Date` objects so that callers get + * the standard Date contract, notably a standard toISOString(). But the writer + * derives a value's TOML text from toISOString(), and the TOML date classes + * override it to return the original spelling. A plain Date handed back in + * therefore writes as a full offset date-time, so read-modify-write turned + * `1986-03-02` into `1986-03-02T00:00:00.000Z` whenever the surrounding node was + * rewritten rather than preserved from the CST. + * + * Where the caller left a date alone (still a plain Date, same instant), the + * original typed instance goes back in so the original spelling is written. A + * date whose instant actually changed is left as-is: only the caller knows + * whether a new instant is still meant to be a local date, and they can say so + * by passing LocalDate / LocalTime / LocalDateTime / OffsetDateTime, which are + * exported for that purpose. Instances of those classes are never touched. + * + * The caller's object is not mutated; a node is copied only when something + * beneath it changed, so an untouched tree is returned by reference. + */ +function restoreDateRepresentations(updated: any, original: any, seen: WeakSet = new WeakSet()): any { + if (updated instanceof Date) { + const erased = updated.constructor === Date + && original instanceof Date + && original.constructor !== Date + && original.getTime() === updated.getTime(); + return erased ? original : updated; + } + if (!updated || typeof updated !== 'object' || seen.has(updated)) return updated; + seen.add(updated); + + if (Array.isArray(updated)) { + if (!Array.isArray(original)) return updated; + let changed = false; + const restored = updated.map((item, index) => { + const next = restoreDateRepresentations(item, original[index], seen); + if (next !== item) changed = true; + return next; + }); + return changed ? restored : updated; + } + + if (!isObject(original)) return updated; + let changed = false; + const restored: Record = {}; + for (const key of Object.keys(updated)) { + const value = updated[key]; + const next = restoreDateRepresentations(value, original[key], seen); + restored[key] = next; + if (next !== value) changed = true; + } + return changed ? restored : updated; +} + /** * Recursively converts custom date classes to regular JavaScript Date objects. * This ensures that the toJsObject property returns standard Date objects diff --git a/src/toml-format.ts b/src/toml-format.ts index 9d362db0..6cdb39dc 100644 --- a/src/toml-format.ts +++ b/src/toml-format.ts @@ -243,6 +243,40 @@ export function detectTabsForIndentation(str: string): boolean { * @param format - The format object to validate * @returns The validated format object with only supported properties and correct types */ +/** + * Spellings accepted for `newLine`, mapped to the line ending they denote. + * A Map rather than an object literal so that a key like `constructor` cannot + * resolve through Object.prototype. + */ +const NEW_LINE_ALIASES = new Map([ + ['\n', '\n'], + ['\r\n', '\r\n'], + ['\\n', '\n'], + ['\\r\\n', '\r\n'], + ['LF', '\n'], + ['CRLF', '\r\n'], + ['UNIX', '\n'], + ['DOS', '\r\n'] +]); + +/** + * Resolves an accepted `newLine` spelling to the literal it denotes. + * + * TOML permits only LF and CRLF, so anything else would silently produce a + * document that does not parse (a bare '\\r', or the literal text 'LF' written + * between every pair of lines). Rejecting it here means every entry point that + * resolves a format gets the check, rather than just `patch()`. + * + * @throws TypeError when the value denotes neither LF nor CRLF + */ +export function normalizeNewLine(value: string): string { + const resolved = NEW_LINE_ALIASES.get(value) ?? NEW_LINE_ALIASES.get(value.toUpperCase()); + if (resolved === undefined) { + throw new TypeError('Invalid newLine value: expected LF or CRLF'); + } + return resolved; +} + export function validateFormatObject(format: any): any { if (!format || typeof format !== 'object') { return {}; @@ -292,6 +326,12 @@ export function validateFormatObject(format: any): any { throw new TypeError(`Invalid types for format properties: ${invalid.join(', ')}`); } + // Runs after the type checks so a non-string newLine is still reported as a + // type error rather than an unsupported value. + if ('newLine' in validatedFormat) { + validatedFormat.newLine = normalizeNewLine(validatedFormat.newLine); + } + return validatedFormat; } @@ -462,7 +502,7 @@ export class TomlFormat { updateOrder?: boolean ) { // Use provided values or fall back to defaults - this.newLine = newLine ?? DEFAULT_NEWLINE; + this.newLine = newLine == null ? DEFAULT_NEWLINE : normalizeNewLine(newLine); this.trailingNewline = trailingNewline ?? DEFAULT_TRAILING_NEWLINE; this.trailingComma = trailingComma ?? DEFAULT_TRAILING_COMMA; this.bracketSpacing = bracketSpacing ?? DEFAULT_BRACKET_SPACING;