diff --git a/CHANGELOG.md b/CHANGELOG.md index 8998198..825825c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **tests**: A fast-check property suite (`tests/property/`) now enforces round-trip laws across all 11 format parsers — translated values survive reconstruct/extract intact, re-applying the same translations never changes the file, and an identity sync is a fixed point — plus preservation laws for the placeholder and ICU utilities. Runs are seeded-random with 200 cases per law by default (`FC_NUM_RUNS` overrides; `FC_SEED`/`FC_PATH` replay a recorded counterexample). The suite found the U+2028 TOML corruption and the `.properties` leading-space loss fixed in this release; example tests document decisions, properties enforce laws. - **tests**: Tests that assert only inside a `catch` block now declare their assertion count, so they fail instead of passing with zero assertions when the command under test unexpectedly succeeds — the failure they exist to detect was the one they could not see. Sites that already assert on the success path are unchanged. `npm test` also refuses to run against a **stale** `dist/`, not just a missing one: the suites execute the built CLI, so a stale build reported results that did not describe the current source. A new suite checks every documented `deepl …` invocation in the README and docs against the CLI's real surface, so a command or flag that drifts out of existence fails CI rather than waiting for a reader to find it. - **docs**: The README leads with the npm install path and marks Homebrew as pending until the tap exists. Install instructions target `@deepl/cli` (10 occurrences across `docs/SYNC.md`, four example scripts, `examples/README.md`, and the git-hook template in `src/services/git-hooks.ts`). The README installation section documents three install paths — npm (`npm install -g @deepl/cli`), from source, and Homebrew (`brew install deepl/tap/deepl`, marked pending until the tap ships) — with an explicit Node.js 24 prerequisite, replacing the `better-sqlite3` native-compilation caveat (Xcode CLT / python3-make-gcc), which no longer applies. The `TROUBLESHOOTING.md` `NODE_MODULE_VERSION` / `npm rebuild better-sqlite3` entry is replaced by accurate guidance for the one remaining cache-degradation cause (running on Node < 24). Six stale `DeepLcom` GitHub URLs now point at the `DeepL` org directly. `CONTRIBUTING.md` states the Node 24 development prerequisite, and `SECURITY.md`'s supported-versions table reflects that only 2.x is a published, supported line. - **tests**: Removed the global manual mocks for `p-limit` and `fast-glob` (`tests/__mocks__/`). Because a manual mock for a node module is auto-applied to every suite, and `resetMocks: true` strips its implementation, `pLimit(n)` resolved to `undefined` and `fg(...)` resolved to `undefined` in all 236 suites — so no test exercised a real concurrency limit or a real glob walk, and two concurrency defects reached a release candidate undetected. The suites that need these mocked declare them explicitly with their own implementations, so nothing depended on the global versions. Added `tests/unit/concurrency-limiting.test.ts`, which asserts that `p-limit` rejects a non-positive concurrency, that peak overlap never exceeds the limit, and that `fast-glob` returns real paths — it fails if either global mock is reintroduced. @@ -52,6 +53,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **formats**: TOML reconstruction escapes U+2028/U+2029 (Unicode line/paragraph separators) in double-quoted values, and literal-string values gaining one fall back to double quotes. Written raw, these characters broke the entry-line scan on the *next* sync (JavaScript's `.` excludes line terminators), which re-appended the key as a duplicate and made the third sync fail to parse the file at all — first sync fine, second silently corrupting, third crashing. Found by the property-based round-trip suite. +- **formats**: `.properties` reconstruction escapes leading spaces in values (`\ `), which the value parser otherwise strips on the next read — a translation beginning with a space silently lost it on every subsequent sync. Leading tabs, trailing spaces, and newlines were already escaped correctly. Found by the property-based round-trip suite. - **sync**: `--auto-commit` now recognises its own translation output rather than only the files a given run wrote, which fixes two related problems. A translation left on disk by an earlier refused run was classed as an unrelated modification, so auto-commit refused forever and the user had to commit it by hand; it is now committed once the genuinely unrelated changes are dealt with. And in `--watch` mode, where the same path runs once per trigger, a trigger that translated nothing skipped the checks entirely and reported success while a commit was still owed. Staging is also driven by what is actually dirty, so a rewrite that produced identical bytes no longer attempts an empty commit. Ownership is derived from the lockfile's tracked source files, with each file matched to its own bucket so one bucket's `target_path_pattern` cannot claim another's output. - **sync**: Re-running `deepl sync --auto-commit` after a refusal no longer reports success without committing. A refused run still writes its translations to disk, so the retry found nothing to translate; the preflight was skipped in that case and the command exited 0 with no commit made, the translation still uncommitted and the tree still dirty. The repo-state checks now run whenever `--auto-commit` is requested, so the refusal is reported identically on every attempt. Staging and committing are still skipped when there is nothing to commit. Two tests covered this path but could not see the defect: their assertions ran only inside a `catch` the retry never entered, which also hid an assertion that could never have matched (it looked for "detached HEAD" against a message reading "HEAD is detached"). - **write**: `deepl write` now runs from a published install. `diff` is imported at the top of the write command but was declared only under `devDependencies`, which consumers do not install, so every command that loaded the module failed with `Cannot find package 'diff'` before producing output. `--help` did not surface it because the module loads lazily. diff --git a/package-lock.json b/package-lock.json index cab72a5..5130390 100644 --- a/package-lock.json +++ b/package-lock.json @@ -40,6 +40,7 @@ "eslint": "^10.8.0", "eslint-config-prettier": "^10.1.8", "eslint-plugin-jest": "^29.16.0", + "fast-check": "^4.9.0", "globals": "^17.8.0", "jest": "^30.4.2", "memfs": "^4.64.0", @@ -4986,6 +4987,46 @@ "node": "^18.14.0 || ^20.0.0 || ^22.0.0 || >=24.0.0" } }, + "node_modules/fast-check": { + "version": "4.9.0", + "resolved": "https://registry.npmjs.org/fast-check/-/fast-check-4.9.0.tgz", + "integrity": "sha512-7ms6T7SybUev/PQITciI0yLM2pOSFy5zpG8Ty7tQofcVaQUvrMXp6CBwqF6fThLCLOrfBtuHAtwq6Yu4XPCllg==", + "dev": true, + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/dubzzz" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fast-check" + } + ], + "license": "MIT", + "dependencies": { + "pure-rand": "^8.0.0" + }, + "engines": { + "node": ">=12.17.0" + } + }, + "node_modules/fast-check/node_modules/pure-rand": { + "version": "8.4.2", + "resolved": "https://registry.npmjs.org/pure-rand/-/pure-rand-8.4.2.tgz", + "integrity": "sha512-vvuOGgcuPJAirlHvuQw1TrOiw7ptaIXXmIbNuiNOY6lNGJJH49PQ1Kj4nd783nPdQhQdicgOjVI2yI/9BD6/Ng==", + "dev": true, + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/dubzzz" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fast-check" + } + ], + "license": "MIT" + }, "node_modules/fast-deep-equal": { "version": "3.1.3", "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", diff --git a/package.json b/package.json index 0532040..00144f7 100644 --- a/package.json +++ b/package.json @@ -92,6 +92,7 @@ "eslint": "^10.8.0", "eslint-config-prettier": "^10.1.8", "eslint-plugin-jest": "^29.16.0", + "fast-check": "^4.9.0", "globals": "^17.8.0", "jest": "^30.4.2", "memfs": "^4.64.0", diff --git a/src/formats/properties.ts b/src/formats/properties.ts index fc9c1e1..6cb4e34 100644 --- a/src/formats/properties.ts +++ b/src/formats/properties.ts @@ -155,8 +155,12 @@ export class PropertiesFormatParser implements FormatParser { } private escapeValue(s: string): string { - let result = ''; - for (const ch of s) { + // Leading spaces must be escaped: the value parser strips unescaped + // whitespace after the separator, so a raw leading space is lost on + // the next read. (Leading tabs are covered by the \t escape below.) + const leading = /^ +/.exec(s); + let result = leading ? '\\ '.repeat(leading[0].length) : ''; + for (const ch of s.slice(leading ? leading[0].length : 0)) { switch (ch) { case '\n': result += '\\n'; break; case '\t': result += '\\t'; break; diff --git a/src/formats/toml.ts b/src/formats/toml.ts index 053cb28..08474a3 100644 --- a/src/formats/toml.ts +++ b/src/formats/toml.ts @@ -241,8 +241,10 @@ export class TomlFormatParser implements FormatParser { function encodeTomlString(value: string, useDoubleQuote: boolean): string { if (!useDoubleQuote) { // Literal strings (single-quoted) are raw — no escapes, cannot contain `'` - // or a newline. If the translation has either, fall back to double-quoted. - if (value.includes("'") || /[\n\r]/.test(value)) { + // or a newline. U+2028/U+2029 also force the double-quoted fallback: left + // raw they break ENTRY_LINE_RE on the next pass (JS `.` excludes line + // separators), so the key is re-appended as a duplicate. + if (value.includes("'") || /[\n\r\u2028\u2029]/.test(value)) { return encodeTomlString(value, true); } return `'${value}'`; @@ -250,11 +252,14 @@ function encodeTomlString(value: string, useDoubleQuote: boolean): string { // Double-quoted: escape backslash first, then `"`, then common control chars. // Order matters so later escapes don't double-escape the backslashes we add. + // U+2028/U+2029 must never be written raw (see the literal-string comment). const escaped = value .replace(/\\/g, '\\\\') .replace(/"/g, '\\"') .replace(/\n/g, '\\n') .replace(/\r/g, '\\r') - .replace(/\t/g, '\\t'); + .replace(/\t/g, '\\t') + .replace(/\u2028/g, '\\u2028') + .replace(/\u2029/g, '\\u2029'); return `"${escaped}"`; } diff --git a/tests/property/arbitraries.ts b/tests/property/arbitraries.ts new file mode 100644 index 0000000..b9fdcc3 --- /dev/null +++ b/tests/property/arbitraries.ts @@ -0,0 +1,86 @@ +/** + * Shared fast-check arbitraries for the property suite. + * + * Translation values are grapheme-based (no lone surrogates) with a pool of + * adversarial constants mixed in: format syntax, entities, quotes, RTL, + * astral characters, combining marks, placeholder patterns. + * + * Control characters other than \n and \t are excluded: they are invalid in + * XML 1.0 and questionable input for every format, so they would only add + * invalid-input noise to the run. + */ +import fc from 'fast-check'; + +export const NASTY_STRINGS: readonly string[] = [ + 'Hello 😀 world', // astral / surrogate pair + 'café', // combining acute (NFD-style) + 'café', // precomposed (NFC) + 'שלום עולם', // RTL Hebrew + 'ثم شرب القهوة', // RTL Arabic + '日本語のテキスト', + 'Türkçe İstanbul ı', + 'Terms & Conditions', + '5 < 10 && 10 > 5', + 'bold it', + ']]>', + '"double" and \'single\' quotes', + 'back\\slash \\n literal', + '{name} has {{count}} items', + '${var} and %1$s and %s and %@', + 'line one\nline two', + 'tab\there', + ' leading and trailing ', + "it''s '{'quoted'}' ICU", + 'key=value # hash ; semi : colon', + 'msgid "injected"', + '', + '[section] toml = "like"', + '- yaml: like', + '=0 one other', +]; + +// eslint-disable-next-line no-control-regex -- excluding control chars is the point +const CONTROL_CHARS_RE = /[\u0000-\u0008\u000B-\u001F\u007F-\u009F]/; + +/** A single translation value: never empty, no control chars beyond \n \t. */ +export const translationArb: fc.Arbitrary = fc + .oneof( + { + weight: 3, + arbitrary: fc.string({ unit: 'grapheme', minLength: 1, maxLength: 60 }), + }, + { weight: 2, arbitrary: fc.constantFrom(...NASTY_STRINGS) }, + { + weight: 1, + arbitrary: fc + .tuple( + fc.constantFrom(...NASTY_STRINGS), + fc.string({ unit: 'grapheme', maxLength: 20 }) + ) + .map(([nasty, tail]) => `${nasty}${tail}`), + } + ) + .filter((s) => s.length > 0 && !CONTROL_CHARS_RE.test(s)); + +/** Exactly `n` translation values. */ +export function translationsArb(n: number): fc.Arbitrary { + return fc.array(translationArb, { minLength: n, maxLength: n }); +} + +export const FC_NUM_RUNS = Number(process.env['FC_NUM_RUNS'] ?? 200); + +/** + * Common fc.assert parameters. Replay a recorded failure with: + * FC_SEED= FC_PATH= npx jest tests/property -t '' + */ +export function fcParams(): fc.Parameters { + const params: fc.Parameters = { numRuns: FC_NUM_RUNS }; + if (process.env['FC_SEED']) { + params.seed = Number(process.env['FC_SEED']); + } + if (process.env['FC_PATH']) { + params.path = process.env['FC_PATH']; + params.endOnFailure = true; + } + return params; +} diff --git a/tests/property/batch-splitting.property.test.ts b/tests/property/batch-splitting.property.test.ts new file mode 100644 index 0000000..75b74a2 --- /dev/null +++ b/tests/property/batch-splitting.property.test.ts @@ -0,0 +1,114 @@ +/** + * Property tests for TranslationService.translateBatch chunking and + * index mapping (src/services/translation.ts). + * + * Laws, with a fake client and the cache disabled: + * B1 no client call carries more than TRANSLATE_BATCH_SIZE texts + * B2 the texts sent across all calls are exactly the unique non-empty + * inputs, each sent once (dedup) + * B3 every non-empty input position gets its own text's translation, + * duplicates included (index mapping) + * B4 the number of client calls is ceil(unique / TRANSLATE_BATCH_SIZE) + */ +import fc from 'fast-check'; +import { + TranslationService, + TRANSLATE_BATCH_SIZE, +} from '../../src/services/translation'; +import type { DeepLClient } from '../../src/api/deepl-client'; +import type { ConfigService } from '../../src/storage/config'; +import { fcParams } from './arbitraries'; + +interface RecordedCall { + texts: string[]; +} + +function makeFakes(): { + client: DeepLClient; + config: ConfigService; + calls: RecordedCall[]; +} { + const calls: RecordedCall[] = []; + const client = { + translateBatch: (texts: string[]) => { + calls.push({ texts: [...texts] }); + return Promise.resolve(texts.map((t) => ({ text: `X:${t}` }))); + }, + } as unknown as DeepLClient; + const config = { + get: () => ({ defaults: {} }), + getValue: () => false, // cache.enabled -> false + } as unknown as ConfigService; + return { client, config, calls }; +} + +/** + * Input positions index into a small pool so duplicates are common — the + * index-mapping law is only interesting when the same text appears at + * several positions. + */ +const textsArb = fc + .tuple( + fc.array(fc.string({ unit: 'grapheme', minLength: 1, maxLength: 20 }), { + minLength: 1, + maxLength: 130, + }), + fc.array(fc.nat(129), { minLength: 0, maxLength: 200 }) + ) + .map(([pool, picks]) => { + // Suffix with the pool index so pool entries are pairwise distinct. + const distinct = pool.map((t, i) => `${t}#${i}`); + return picks.map((p) => distinct[p % distinct.length]!); + }); + +describe('translateBatch chunking and index mapping', () => { + it('B1-B4: chunk size, dedup, index mapping, call count', () => { + return fc.assert( + fc.asyncProperty(textsArb, async (texts) => { + const { client, config, calls } = makeFakes(); + const service = new TranslationService(client, config); + + const results = await service.translateBatch(texts, { + targetLang: 'de', + }); + + // B1 + for (const call of calls) { + expect(call.texts.length).toBeLessThanOrEqual(TRANSLATE_BATCH_SIZE); + } + + // B2 + const sent = calls.flatMap((c) => c.texts); + const unique = [...new Set(texts.filter((t) => t))]; + expect(sent.length).toBe(unique.length); + expect(new Set(sent)).toEqual(new Set(unique)); + + // B3 + expect(results.length).toBe(texts.length); + for (let i = 0; i < texts.length; i++) { + if (texts[i]) { + expect(results[i]?.text).toBe(`X:${texts[i]!}`); + } + } + + // B4 + const expectedCalls = + unique.length === 0 + ? 0 + : Math.ceil(unique.length / TRANSLATE_BATCH_SIZE); + expect(calls.length).toBe(expectedCalls); + }), + fcParams() + ); + }); + + it('empty input makes no client calls and returns an empty array', async () => { + const { client, config, calls } = makeFakes(); + const service = new TranslationService(client, config); + + const results = await service.translateBatch([], { targetLang: 'de' }); + + expect(results).toEqual([]); + expect(calls).toEqual([]); + }); +}); diff --git a/tests/property/cache-key-normalization.test.ts b/tests/property/cache-key-normalization.test.ts new file mode 100644 index 0000000..e707f6d --- /dev/null +++ b/tests/property/cache-key-normalization.test.ts @@ -0,0 +1,33 @@ +/** + * Directed demonstration (not a property). + * + * TranslationService.generateCacheKey hashes the raw text without Unicode + * normalization, so NFC and NFD encodings of the same visible string produce + * two cache entries — and therefore two billable API calls. + * + * generateCacheKey reads nothing from `this`, so it is invoked via the + * prototype without constructing the service. + */ +import { TranslationService } from '../../src/services/translation'; + +type KeyFn = (text: string, options: { targetLang: string }) => string; + +const generateCacheKey = ( + TranslationService.prototype as unknown as { generateCacheKey: KeyFn } +).generateCacheKey; + +describe('translation cache key normalization', () => { + it('documents that NFC and NFD of the same visible string produce distinct keys', () => { + const nfc = 'café'.normalize('NFC'); + const nfd = 'café'.normalize('NFD'); + expect(nfc).not.toBe(nfd); // different code points + expect(nfc.normalize('NFC')).toBe(nfd.normalize('NFC')); // same visible string + + const keyNfc = generateCacheKey.call(null, nfc, { targetLang: 'de' }); + const keyNfd = generateCacheKey.call(null, nfd, { targetLang: 'de' }); + + // Current behavior: two entries for one visible string. Whether this is + // a bug or intentional exact-input keying is a policy decision. + expect(keyNfc).not.toBe(keyNfd); + }); +}); diff --git a/tests/property/formats-fixedpoint.property.test.ts b/tests/property/formats-fixedpoint.property.test.ts new file mode 100644 index 0000000..df15050 --- /dev/null +++ b/tests/property/formats-fixedpoint.property.test.ts @@ -0,0 +1,276 @@ +/** + * Round-trip laws over all 11 format parsers, driven by fast-check. + * + * Symmetric formats (extract reads the same slot reconstruct writes): + * L1 value round-trip: extract(reconstruct(skeleton, t)).values === t + * L2 content idempotence: reconstruct(out, same entries) === out + * L3 extract fixed point: extract after a second identity cycle is stable + * + * Asymmetric formats (PO, XLIFF — extract reads msgid/, reconstruct + * writes msgstr/): + * L2 content idempotence: reconstruct(out, t) === out + * L4 source preservation: extract(out).values === extract(skeleton).values + */ +import fc from 'fast-check'; +import { createDefaultRegistrySync } from '../../src/formats/index'; +import type { FormatParser, TranslatedEntry } from '../../src/formats/format'; +import { fcParams, translationsArb } from './arbitraries'; + +interface FormatCase { + configKey: string; + skeleton: string; + symmetric: boolean; + /** multi-locale formats: locale extract reads from / reconstruct writes to */ + sourceLocale?: string; + targetLocale?: string; +} + +const CASES: FormatCase[] = [ + { + configKey: 'json', + symmetric: true, + skeleton: `{ + "greeting": "Hello", + "nav": { + "home": "Home", + "title": "Site: \\"quoted\\"" + }, + "count": 42, + "enabled": true +} +`, + }, + { + configKey: 'yaml', + symmetric: true, + skeleton: `# top comment +greeting: Hello +nav: + home: Home + title: "Quoted: value" +`, + }, + { + configKey: 'po', + symmetric: false, + skeleton: `msgid "" +msgstr "Content-Type: text/plain; charset=UTF-8\\n" + +#. Developer note +#: src/app.ts:10 +msgid "Hello" +msgstr "" + +msgctxt "menu" +msgid "Open" +msgstr "" + +msgid "Terms & Conditions" +msgstr "" +`, + }, + { + configKey: 'android_xml', + symmetric: true, + skeleton: ` + + Hello + Terms & Conditions + 5 < 10 + +`, + }, + { + configKey: 'ios_strings', + symmetric: true, + skeleton: `/* Greeting shown on launch */ +"greeting" = "Hello"; +"farewell" = "Goodbye"; +"quoted" = "Say \\"hi\\""; +`, + }, + { + configKey: 'arb', + symmetric: true, + skeleton: `{ + "@@locale": "en", + "greeting": "Hello", + "@greeting": { + "description": "A greeting" + }, + "farewell": "Goodbye" +} +`, + }, + { + configKey: 'xliff', + symmetric: false, + skeleton: ` + + + + + Hello + A greeting + + + Terms & Conditions + + + + +`, + }, + { + configKey: 'toml', + symmetric: true, + skeleton: `greeting = "Hello" + +[nav] +home = "Home" +title = "Quoted \\"value\\"" +`, + }, + { + configKey: 'properties', + symmetric: true, + skeleton: `# Top comment +greeting=Hello +nav.home=Home +nav.title=With spaces +`, + }, + { + configKey: 'xcstrings', + symmetric: true, + sourceLocale: 'en', + targetLocale: 'de', + skeleton: `${JSON.stringify( + { + sourceLanguage: 'en', + version: '1.0', + strings: { + greeting: { + comment: 'Welcome screen title', + localizations: { + en: { stringUnit: { state: 'translated', value: 'Hello' } }, + }, + }, + farewell: { + localizations: { + en: { stringUnit: { state: 'translated', value: 'Goodbye' } }, + }, + }, + }, + }, + null, + 2 + )}\n`, + }, + { + configKey: 'laravel_php', + symmetric: true, + skeleton: ` 'Hello', + 'nav' => [ + 'home' => 'Home', + ], +]; +`, + }, +]; + +const registry = createDefaultRegistrySync(); + +function valueMap( + parser: FormatParser, + content: string, + locale?: string +): Map { + return new Map(parser.extract(content, locale).map((e) => [e.key, e.value])); +} + +describe('format parser round-trip properties', () => { + it('covers every registered format', () => { + const covered = new Set(CASES.map((c) => c.configKey)); + expect([...covered].sort()).toEqual([...registry.getFormatKeys()].sort()); + }); + + describe.each(CASES.map((c) => [c.configKey, c] as const))( + '%s', + (_key, fmt) => { + const parser = registry.getParserByFormatKey(fmt.configKey)!; + const baseEntries = parser.extract(fmt.skeleton, fmt.sourceLocale); + + it('skeleton extracts at least two entries', () => { + expect(baseEntries.length).toBeGreaterThanOrEqual(2); + }); + + if (fmt.symmetric) { + it('L1+L2+L3: generated translations survive reconstruct/extract and reach a fixed point', () => { + fc.assert( + fc.property(translationsArb(baseEntries.length), (translations) => { + const entries: TranslatedEntry[] = baseEntries.map((e, i) => ({ + ...e, + translation: translations[i]!, + })); + + // L1: one write, values must survive intact + const out = parser.reconstruct( + fmt.skeleton, + entries, + fmt.targetLocale + ); + const extractLocale = fmt.targetLocale ?? fmt.sourceLocale; + const got = valueMap(parser, out, extractLocale); + for (let i = 0; i < baseEntries.length; i++) { + const key = baseEntries[i]!.key; + expect(got.get(key)).toBe(translations[i]!); + } + + // L2: re-applying the same translations must not change the file + const out2 = parser.reconstruct(out, entries, fmt.targetLocale); + expect(out2).toBe(out); + + // L3: identity cycle on the written file is a fixed point + const identityEntries: TranslatedEntry[] = parser + .extract(out, extractLocale) + .map((e) => ({ ...e, translation: e.value })); + const out3 = parser.reconstruct( + out, + identityEntries, + fmt.targetLocale + ); + expect(valueMap(parser, out3, extractLocale)).toEqual(got); + }), + fcParams() + ); + }); + } else { + it('L2+L4: reconstruct is idempotent and never mutates source values', () => { + const sourceValues = valueMap(parser, fmt.skeleton, fmt.sourceLocale); + fc.assert( + fc.property(translationsArb(baseEntries.length), (translations) => { + const entries: TranslatedEntry[] = baseEntries.map((e, i) => ({ + ...e, + translation: translations[i]!, + })); + + const out = parser.reconstruct(fmt.skeleton, entries); + + // L4: extract still returns the untouched source strings + expect(valueMap(parser, out)).toEqual(sourceValues); + + // L2: applying the same translations again must be a no-op + const out2 = parser.reconstruct(out, entries); + expect(out2).toBe(out); + }), + fcParams() + ); + }); + } + } + ); +}); diff --git a/tests/property/icu.property.test.ts b/tests/property/icu.property.test.ts new file mode 100644 index 0000000..03c40cc --- /dev/null +++ b/tests/property/icu.property.test.ts @@ -0,0 +1,152 @@ +/** + * Property tests for ICU MessageFormat preservation (src/utils/icu-preservation.ts). + * + * reassemble canonicalizes whitespace by design, so the laws are + * extract-level, not byte-level: + * I1 non-ICU strings pass through byte-identical + * I2 parse → reassemble(identity) → parse preserves segment texts + * I3 the canonicalized form is a fixed point of another identity cycle + * I4 reassemble throws on a wrong translation count + */ +import fc from 'fast-check'; +import { parseIcu } from '../../src/utils/icu-preservation'; +import { fcParams } from './arbitraries'; + +const ICU_DETECT_RE = /\{\s*[\w]+\s*,\s*(?:plural|select|selectordinal)\s*,/; + +/** + * Leaf prose: no braces (would change nesting), no ICU-significant lone + * apostrophe. Per ICU quoting, `'` immediately before `{`, `}`, or `#` + * opens a quoted span — a leaf ending in `'` sits right before the branch's + * closing `}` and swallows it, making the whole message legitimately + * malformed. That is generator noise, not a parser defect (verified: the + * parser falls back safely on such input). + */ +const leafArb = fc + .string({ unit: 'grapheme', maxLength: 25 }) + .filter( + (s) => + !s.includes('{') && + !s.includes('}') && + !/'(?=#|$)/.test(s) && + !ICU_DETECT_RE.test(s) + ); + +const varNameArb = fc.constantFrom('count', 'gender', 'n', 'item_type', 'x2'); + +const pluralSelectorsArb = fc + .uniqueArray( + fc.constantFrom('=0', '=1', 'zero', 'one', 'two', 'few', 'many'), + { + minLength: 0, + maxLength: 3, + } + ) + .map((sels) => [...sels, 'other']); + +const selectSelectorsArb = fc + .uniqueArray(fc.constantFrom('male', 'female', 'red', 'blue'), { + minLength: 0, + maxLength: 2, + }) + .map((sels) => [...sels, 'other']); + +interface IcuNode { + varName: string; + keyword: 'plural' | 'select' | 'selectordinal'; + branches: Array<{ selector: string; content: string | IcuNode }>; +} + +function renderNode(node: IcuNode): string { + const branches = node.branches + .map((b) => { + const inner = + typeof b.content === 'string' ? b.content : renderNode(b.content); + return `${b.selector} {${inner}}`; + }) + .join(' '); + return `{${node.varName}, ${node.keyword}, ${branches}}`; +} + +const icuNodeArb: fc.Arbitrary = fc.letrec<{ node: IcuNode }>( + (tie) => ({ + node: fc + .record({ + varName: varNameArb, + keyword: fc.constantFrom('plural', 'select', 'selectordinal'), + selectors: fc.oneof(pluralSelectorsArb, selectSelectorsArb), + leaves: fc.array(leafArb, { minLength: 8, maxLength: 8 }), + nestAt: fc.option(fc.nat(3), { nil: undefined }), + nested: fc.option( + tie('node').map((n) => n), + { nil: undefined, depthSize: 'small' } + ), + }) + .map(({ varName, keyword, selectors, leaves, nestAt, nested }) => { + const branches: IcuNode['branches'] = selectors.map((selector, i) => ({ + selector, + content: leaves[i] ?? '', + })); + // Nested ICU is only supported when the branch content IS a full block. + if (nested && nestAt !== undefined && nestAt < branches.length) { + branches[nestAt]!.content = nested; + } + return { varName, keyword, branches }; + }), + }) +).node; + +const icuMessageArb = fc + .tuple(leafArb, icuNodeArb, leafArb) + .map(([prefix, node, suffix]) => `${prefix}${renderNode(node)}${suffix}`); + +const nonIcuArb = fc + .string({ unit: 'grapheme', maxLength: 60 }) + .filter((s) => !ICU_DETECT_RE.test(s)); + +describe('ICU preservation properties', () => { + it('I1: non-ICU strings pass through reassemble byte-identical', () => { + fc.assert( + fc.property(nonIcuArb, (text) => { + const r = parseIcu(text); + expect(r.isIcu).toBe(false); + expect(r.reassemble([])).toBe(text); + }), + fcParams() + ); + }); + + it('I2+I3: identity reassembly preserves segment texts and reaches a fixed point', () => { + fc.assert( + fc.property(icuMessageArb, (message) => { + const r1 = parseIcu(message); + expect(r1.isIcu).toBe(true); + + const texts1 = r1.segments.map((s) => s.text); + const rebuilt = r1.reassemble(texts1); + + const r2 = parseIcu(rebuilt); + expect(r2.isIcu).toBe(true); + expect(r2.segments.map((s) => s.text)).toEqual(texts1); + + // I3: canonical form is stable under another identity cycle + const rebuilt2 = r2.reassemble(r2.segments.map((s) => s.text)); + expect(rebuilt2).toBe(rebuilt); + }), + fcParams() + ); + }); + + it('I4: reassemble throws on wrong translation count', () => { + fc.assert( + fc.property(icuMessageArb, (message) => { + const r = parseIcu(message); + expect(r.isIcu).toBe(true); + expect(() => + r.reassemble([...r.segments.map((s) => s.text), 'extra']) + ).toThrow(/expected \d+ translations/); + }), + fcParams() + ); + }); +}); diff --git a/tests/property/message-preprocess.property.test.ts b/tests/property/message-preprocess.property.test.ts new file mode 100644 index 0000000..3180f57 --- /dev/null +++ b/tests/property/message-preprocess.property.test.ts @@ -0,0 +1,188 @@ +/** + * Property tests for the sync message-preprocess pipeline + * (src/sync/sync-message-preprocess.ts). + * + * The pipeline's stated invariant is positional: every stage preserves array + * length and index assignments, so pluralSlots.textIndex and + * icuMappings.textIndex remain valid indices into the results array. + * + * P1 expandPlurals keeps the original texts as an unchanged prefix + * P2 every plural slot points past the prefix, at its own plural value + * P3 detectIcu preserves length and rewrites exactly the ICU positions + * P4 writebackPlurals writes each result to its slot's plural form and + * leaves slots without a result untouched + */ +import fc from 'fast-check'; +import { + expandPlurals, + detectIcu, + writebackPlurals, +} from '../../src/sync/sync-message-preprocess'; +import type { SyncDiff } from '../../src/sync/types'; +import type { TranslationResult } from '../../src/api/translation-client'; +import { fcParams } from './arbitraries'; + +const valueArb = fc.string({ unit: 'grapheme', minLength: 1, maxLength: 20 }); + +const QUANTITIES = ['zero', 'one', 'two', 'few', 'many', 'other'] as const; + +const diffArb: fc.Arbitrary = fc + .record({ + key: fc.string({ unit: 'grapheme-ascii', minLength: 1, maxLength: 10 }), + value: valueArb, + plurals: fc.option( + fc + .uniqueArray(fc.constantFrom(...QUANTITIES), { + minLength: 1, + maxLength: 4, + }) + .chain((quantities) => + fc.tuple( + fc.constant(quantities), + fc.array(valueArb, { + minLength: quantities.length, + maxLength: quantities.length, + }) + ) + ) + .map(([quantities, values]) => + quantities.map((quantity, i) => ({ quantity, value: values[i]! })) + ), + { nil: undefined } + ), + msgidPlural: fc.option(valueArb, { nil: undefined }), + }) + .map(({ key, value, plurals, msgidPlural }) => { + const diff: SyncDiff = { key, status: 'new', value }; + if (plurals || msgidPlural) { + diff.metadata = {}; + if (plurals) diff.metadata['plurals'] = plurals; + if (msgidPlural) diff.metadata['msgid_plural'] = msgidPlural; + } + return diff; + }); + +const diffsArb = fc.array(diffArb, { minLength: 0, maxLength: 12 }); + +function baseTexts(diffs: SyncDiff[]): string[] { + return diffs.map((d) => d.value ?? ''); +} + +describe('sync message-preprocess pipeline positions', () => { + it('P1+P2: expandPlurals appends plural values without disturbing the prefix', () => { + fc.assert( + fc.property(diffsArb, (diffs) => { + const texts = baseTexts(diffs); + const { extendedTexts, pluralSlots } = expandPlurals(texts, diffs); + + // P1 + expect(extendedTexts.slice(0, texts.length)).toEqual(texts); + + // P2 + for (const slot of pluralSlots) { + expect(slot.textIndex).toBeGreaterThanOrEqual(texts.length); + expect(slot.textIndex).toBeLessThan(extendedTexts.length); + const diff = diffs[slot.diffIndex]!; + if (slot.format === 'android') { + const plurals = diff.metadata?.['plurals'] as Array<{ + quantity: string; + value: string; + }>; + const item = plurals.find((p) => p.quantity === slot.slotKey)!; + expect(extendedTexts[slot.textIndex]).toBe(item.value); + } else { + expect(extendedTexts[slot.textIndex]).toBe( + diff.metadata?.['msgid_plural'] + ); + } + } + + // Every appended text is claimed by exactly one slot. + const claimed = pluralSlots + .map((s) => s.textIndex) + .sort((a, b) => a - b); + expect(new Set(claimed).size).toBe(claimed.length); + expect(claimed.length).toBe(extendedTexts.length - texts.length); + }), + fcParams() + ); + }); + + it('P3: detectIcu preserves length and rewrites exactly the ICU positions', () => { + const icuText = 'You have {count, plural, one {# item} other {# items}}.'; + fc.assert( + fc.property( + fc.array(fc.oneof(valueArb, fc.constant(icuText)), { maxLength: 15 }), + (texts) => { + const { extendedTexts, icuMappings } = detectIcu(texts); + + expect(extendedTexts.length).toBe(texts.length); + const icuIndices = new Set(icuMappings.map((m) => m.textIndex)); + for (let i = 0; i < texts.length; i++) { + if (icuIndices.has(i)) { + expect(extendedTexts[i]).toBe(`__ICU_PLACEHOLDER_${i}__`); + } else { + expect(extendedTexts[i]).toBe(texts[i]); + } + } + } + ), + fcParams() + ); + }); + + it('P4: writebackPlurals writes each available result to its slot and skips the rest', () => { + fc.assert( + fc.property( + diffsArb, + fc.infiniteStream(fc.boolean()), + (diffs, haveResult) => { + const texts = baseTexts(diffs); + const { extendedTexts, pluralSlots } = expandPlurals(texts, diffs); + + const results: (TranslationResult | null)[] = extendedTexts.map( + () => null + ); + const iterator = haveResult[Symbol.iterator](); + const expectTranslated = new Set(); + for (const slot of pluralSlots) { + if (iterator.next().value) { + results[slot.textIndex] = { + text: `T:${extendedTexts[slot.textIndex]!}`, + }; + expectTranslated.add(slot.textIndex); + } + } + + writebackPlurals(results, pluralSlots, diffs); + + for (const slot of pluralSlots) { + const diff = diffs[slot.diffIndex]!; + const original = extendedTexts[slot.textIndex]!; + if (slot.format === 'android') { + const plurals = diff.metadata?.['plurals'] as Array<{ + quantity: string; + value: string; + }>; + const item = plurals.find((p) => p.quantity === slot.slotKey)!; + expect(item.value).toBe( + expectTranslated.has(slot.textIndex) + ? `T:${original}` + : original + ); + } else if (expectTranslated.has(slot.textIndex)) { + const forms = diff.metadata?.['plural_forms'] as Record< + string, + string + >; + expect(forms['msgstr[1]']).toBe(`T:${original}`); + } else { + expect(diff.metadata?.['plural_forms']).toBeUndefined(); + } + } + } + ), + fcParams() + ); + }); +}); diff --git a/tests/property/text-preservation.property.test.ts b/tests/property/text-preservation.property.test.ts new file mode 100644 index 0000000..3a8ea0f --- /dev/null +++ b/tests/property/text-preservation.property.test.ts @@ -0,0 +1,95 @@ +/** + * Property tests for the preservation utilities (src/utils/text-preservation.ts). + * + * Core law: for any text, preserve → identity-translate → restore is the + * identity function, and the multiset of preserved tokens is unchanged. + * + * Texts containing literal placeholder tokens (__VAR_n__ / __CODE_n__) are + * excluded from the main law and pinned by a directed test instead: + * restorePlaceholders cannot distinguish a literal token in the input from + * one the preserver inserted, so such texts are rewritten on restore. + */ +import fc from 'fast-check'; +import { + preserveCodeBlocks, + preserveVariables, + restorePlaceholders, +} from '../../src/utils/text-preservation'; +import { FC_NUM_RUNS, fcParams } from './arbitraries'; + +const PLACEHOLDER_RE = /__(?:VAR|CODE)_\d+__/; + +const fragmentArb = fc.oneof( + { weight: 4, arbitrary: fc.string({ unit: 'grapheme', maxLength: 30 }) }, + { + weight: 3, + arbitrary: fc.constantFrom( + '{name}', + '{{count}}', + '${var}', + '{0}', + '{名前}', + '%1$s', + '%2$d', + '%s', + '%d', + '%@', + '`inline code`', + '```\nfenced block\n```', + '`a && b`', + '{not_closed', + 'closed_not}', + '$not_a_var', + '%x', + '{name with space}', + '{{nested {inner}}}' + ), + } +); + +const textArb = fc + .array(fragmentArb, { minLength: 0, maxLength: 12 }) + .map((parts) => parts.join(' ')) + .filter((s) => !PLACEHOLDER_RE.test(s)); + +function roundTrip(text: string): string { + const map = new Map(); + const processed = preserveVariables(preserveCodeBlocks(text, map), map); + return restorePlaceholders(processed, map); +} + +describe('text preservation properties', () => { + it('preserve → identity-translate → restore is the identity', () => { + fc.assert( + fc.property(textArb, (text) => { + expect(roundTrip(text)).toBe(text); + }), + { ...fcParams(), numRuns: FC_NUM_RUNS * 2 } + ); + }); + + it('processed text contains no un-preserved simple variables', () => { + fc.assert( + fc.property(textArb, (text) => { + const map = new Map(); + const processed = preserveVariables(preserveCodeBlocks(text, map), map); + // After preservation the MT engine must not see bare placeholders + // that the patterns claim to cover. + expect(processed).not.toMatch(/\$\{[\p{L}\p{N}_]+\}/u); + expect(processed).not.toMatch(/%\d+\$[sdfu@]/); + }), + fcParams() + ); + }); + + describe('directed: literal placeholder tokens in input', () => { + it('documents the collision: a literal __VAR_0__ in the source is rewritten on restore', () => { + const text = 'literal __VAR_0__ and {name}'; + const result = roundTrip(text); + // Intended behavior would be result === text. Recording the actual + // (corrupting) behavior so the failure mode is pinned with a repro. + expect(result).toBe('literal {name} and {name}'); + expect(result).not.toBe(text); + }); + }); +}); diff --git a/tests/property/translation-validator.property.test.ts b/tests/property/translation-validator.property.test.ts new file mode 100644 index 0000000..19b931a --- /dev/null +++ b/tests/property/translation-validator.property.test.ts @@ -0,0 +1,111 @@ +/** + * Property tests for the translation validator + * (src/sync/translation-validator.ts). + * + * The validator inspects machine-translation output, so it must be total: + * V1 never throws, on any (source, translation) pair + * V2 a translation identical in structure to its source is never an error + * (identity can warn — e.g. "untranslated" — but a text cannot + * structurally mismatch itself) + * V3 dropping a placeholder from the translation is always flagged as a + * placeholders error + * V4 a placeholder added to the translation is always flagged + */ +import fc from 'fast-check'; +import { validateTranslation } from '../../src/sync/translation-validator'; +import { fcParams, translationArb } from './arbitraries'; + +const PLACEHOLDERS = [ + '{name}', + '{{count}}', + '${var}', + '%1$s', + '%s', + '%@', + '{0}', +]; + +const proseArb = fc + .string({ unit: 'grapheme', minLength: 1, maxLength: 15 }) + // Keep prose free of placeholder/ICU syntax so the generated placeholders + // are the only ones in play. + .filter((s) => !/[{}%$]/.test(s)); + +/** Prose interleaved with a known set of placeholders. */ +const sourceWithPlaceholdersArb = fc + .tuple( + fc.array(fc.constantFrom(...PLACEHOLDERS), { minLength: 1, maxLength: 4 }), + fc.array(proseArb, { minLength: 2, maxLength: 5 }) + ) + .map(([placeholders, prose]) => { + const parts: string[] = [prose[0]!]; + placeholders.forEach((ph, i) => { + parts.push(ph, prose[(i + 1) % prose.length]!); + }); + return { text: parts.join(' '), placeholders }; + }); + +describe('translation validator properties', () => { + it('V1: never throws on arbitrary input pairs', () => { + fc.assert( + fc.property(translationArb, translationArb, (source, translation) => { + expect(() => + validateTranslation('k', source, translation) + ).not.toThrow(); + }), + fcParams() + ); + }); + + it('V2: identity is never a structural error', () => { + fc.assert( + fc.property(translationArb, (text) => { + const result = validateTranslation('k', text, text); + expect(result.severity).not.toBe('error'); + }), + fcParams() + ); + }); + + it('V3: a dropped placeholder is always an error', () => { + fc.assert( + fc.property( + sourceWithPlaceholdersArb, + fc.nat(), + ({ text, placeholders }, pick) => { + const dropped = placeholders[pick % placeholders.length]!; + const translation = text.replace(dropped, ''); + + const result = validateTranslation('k', text, translation); + + expect(result.severity).toBe('error'); + expect( + result.issues.some( + (i) => i.check === 'placeholders' && i.severity === 'error' + ) + ).toBe(true); + } + ), + fcParams() + ); + }); + + it('V4: an added placeholder is always flagged', () => { + fc.assert( + fc.property( + sourceWithPlaceholdersArb, + fc.constantFrom(...PLACEHOLDERS), + ({ text }, extra) => { + const translation = `${text} ${extra}`; + + const result = validateTranslation('k', text, translation); + + expect(result.issues.some((i) => i.check === 'placeholders')).toBe( + true + ); + } + ), + fcParams() + ); + }); +}); diff --git a/tests/unit/formats-roundtrip.test.ts b/tests/unit/formats-roundtrip.test.ts index 78a5822..de5338e 100644 --- a/tests/unit/formats-roundtrip.test.ts +++ b/tests/unit/formats-roundtrip.test.ts @@ -7,6 +7,7 @@ import { AndroidXmlFormatParser } from '../../src/formats/android-xml'; import { PropertiesFormatParser } from '../../src/formats/properties'; +import { TomlFormatParser } from '../../src/formats/toml'; describe('parser round-trip stability', () => { describe('Android XML entity escaping', () => { @@ -97,5 +98,61 @@ describe('parser round-trip stability', () => { expect(parser.extract(withUmlaut)[0]?.value).toBe('Grüße, Welt'); }); + + it('should round-trip leading spaces by escaping them', () => { + const parser = new PropertiesFormatParser(); + + for (const translation of [' padded', ' two leading', ' ', ' x ']) { + const out = parser.reconstruct( + source, + parser.extract(source).map((e) => ({ ...e, translation })), + ); + + // An unescaped leading space is trimmed by the value parser (per the + // .properties format), silently losing it. + expect(parser.extract(out)[0]?.value).toBe(translation); + } + }); + }); + + describe('TOML line-separator characters', () => { + const source = 'greeting = "Hello"\nfarewell = "Bye"\n'; + + it('should keep files parseable across repeated syncs when a translation contains U+2028/U+2029', () => { + for (const separator of ['\u2028', '\u2029']) { + const parser = new TomlFormatParser(); + const entries = parser + .extract(source) + .map((e) => ({ ...e, translation: `x${separator}y` })); + + const out = parser.reconstruct(source, entries); + expect(parser.extract(out).map((e) => e.value)).toEqual([ + `x${separator}y`, + `x${separator}y`, + ]); + + // A raw line separator in the written value breaks the entry-line + // scan on the next pass (JS `.` excludes U+2028/U+2029), which + // re-appends the key as a duplicate and makes the file unparseable. + const out2 = parser.reconstruct(out, entries); + expect(out2).toBe(out); + expect(() => parser.extract(out2)).not.toThrow(); + } + }); + + it('should fall back to a double-quoted string when a literal-string value gains a line separator', () => { + const parser = new TomlFormatParser(); + const literalSource = "greeting = 'Hello'\n"; + const entries = parser + .extract(literalSource) + .map((e) => ({ ...e, translation: 'x\u2028y' })); + + const out = parser.reconstruct(literalSource, entries); + expect(parser.extract(out)[0]?.value).toBe('x\u2028y'); + + const out2 = parser.reconstruct(out, entries); + expect(out2).toBe(out); + expect(() => parser.extract(out2)).not.toThrow(); + }); }); });