diff --git a/.changeset/collapse-step-escaper-copies.md b/.changeset/collapse-step-escaper-copies.md new file mode 100644 index 000000000..43f6b9149 --- /dev/null +++ b/.changeset/collapse-step-escaper-copies.md @@ -0,0 +1,10 @@ +--- +'@ifc-lite/data': minor +'@ifc-lite/export': patch +--- + +The STEP string escaper (`escapeStepString`) existed twice in TypeScript: once, private, in `@ifc-lite/data`'s `step-serializers.ts`, and again, exported, in `@ifc-lite/export`'s `step-serialization.ts`. The two bodies were identical — same backslash/quote doubling, same `\X2\`/`\X4\` non-ASCII directive thresholds, same one-space-per-control-character rule. `@ifc-lite/export` already depends on `@ifc-lite/data` with no cycle, so there was no reason for the second copy. + +`@ifc-lite/data` now exports `escapeStepString` as the single implementation; `@ifc-lite/export`'s `step-serialization.ts` re-exports it instead of keeping its own copy, so every existing call site is unaffected. The Rust implementation, `ifc_lite_export::step_text::escape`, stays separate — sharing it with TypeScript would need a wasm adapter, which is a bigger change than this one — and continues to be pinned by a hand-kept vector test rather than shared code. + +A prior fix (#3284) added a test in each of the two TypeScript files asserting that both matched the Rust half's output on the same inputs; with only one TypeScript implementation left, that duplication is gone and the coverage lives once, in `@ifc-lite/data`'s `step-serializers.test.ts`. diff --git a/packages/data/src/index.ts b/packages/data/src/index.ts index 833218b96..8b2e09c04 100644 --- a/packages/data/src/index.ts +++ b/packages/data/src/index.ts @@ -56,6 +56,7 @@ export { generateHeader, generateStepFileWithRegistry, parseStepValue, + escapeStepString, } from './step-serializers.js'; export type { StepValue, diff --git a/packages/data/src/step-serializers.test.ts b/packages/data/src/step-serializers.test.ts index a3f610189..3598ef131 100644 --- a/packages/data/src/step-serializers.test.ts +++ b/packages/data/src/step-serializers.test.ts @@ -5,6 +5,7 @@ import { describe, it, expect } from 'vitest'; import { enumVal, + escapeStepString, formatStepReal, generateHeader, generateStepFileWithRegistry, @@ -233,13 +234,16 @@ describe('toStepLineWithRegistry / generateStepFileWithRegistry', () => { /** - * The `@ifc-lite/data` serializer's escaper is the second TS copy of the rule - * pinned in `@ifc-lite/export`'s `step-serialization.test.ts` and in - * `ifc_lite_export::step_text::escape`: ONE SPACE PER CONTROL CHARACTER, not - * one per run (#3284 item 2). `escapeStepString` is module-private here, so a - * run reaches it through the two public funnels that use it — `serializeValue` - * for an attribute value and `generateHeader` for a header field. The expected - * strings are the Rust half's observed output for the same inputs. + * `escapeStepString` is now the ONE TypeScript implementation (#3300): + * `@ifc-lite/export`'s `step-serialization.ts` re-exports this symbol rather + * than keeping its own copy, so this suite is the only place TS-side coverage + * needs to live. `ifc_lite_export::step_text::escape` stays a separate, + * hand-kept Rust implementation — a wasm adapter to share it is out of scope + * here — so the rule below (ONE SPACE PER CONTROL CHARACTER, not one per run, + * #3284 item 2) is pinned against the Rust half's observed output rather than + * shared code. Exercised through the two public funnels that use it — + * `serializeValue` for an attribute value and `generateHeader` for a header + * field — plus `escapeStepString` itself directly, now that it is exported. */ describe('control-character runs are one space each (#3284, parity with the Rust escape)', () => { const RUST_VECTORS: ReadonlyArray = [ @@ -275,3 +279,35 @@ describe('control-character runs are one space each (#3284, parity with the Rust } }); }); + + +/** + * `escapeStepString` exported directly (#3300): the two TS copies of the + * encode half collapsed into this one, re-exported by `@ifc-lite/export`. + * Awkward inputs a caller could plausibly pass through -- a bare backslash, + * an apostrophe, a BMP non-ASCII character, text that already looks like a + * `\X2\` directive, and the empty string. + */ +describe('escapeStepString direct (#3300)', () => { + it('doubles a lone backslash', () => { + expect(escapeStepString('a\\b')).toBe('a\\\\b'); + }); + + it('doubles a lone apostrophe', () => { + expect(escapeStepString("a'b")).toBe("a''b"); + }); + + it('encodes a non-ASCII character as an \\X2\\ directive', () => { + expect(escapeStepString('\u00C4')).toBe('\\X2\\00C4\\X0\\'); + }); + + it('doubles the backslashes of text that already looks like a directive', () => { + // The input is literal text, not an actual directive: every backslash in + // it must be doubled like any other, or a reader would decode it as one. + expect(escapeStepString('a\\X2\\00FC\\X0\\b')).toBe('a\\\\X2\\\\00FC\\\\X0\\\\b'); + }); + + it('returns the empty string unchanged', () => { + expect(escapeStepString('')).toBe(''); + }); +}); diff --git a/packages/data/src/step-serializers.ts b/packages/data/src/step-serializers.ts index 36b394f9d..7cde56a27 100644 --- a/packages/data/src/step-serializers.ts +++ b/packages/data/src/step-serializers.ts @@ -188,10 +188,10 @@ export function serializeValue(value: StepValue): string { * `\X2\HHHH\X0\`, `\X4\HHHHHHHH\X0\`), never a raw byte, and buildingSMART * says the same for IFC2X3/IFC4/IFC4X3. A reader decoding as ISO-8859-1 (what * the base standard and most consumers assume) turns raw UTF-8 into mojibake or - * a broken parse: IfcOpenShell#699/#1016, files rejected by Solibri. Matches - * `@ifc-lite/export`'s `escapeStepString`. + * a broken parse: IfcOpenShell#699/#1016, files rejected by Solibri. The one TS + * implementation (#3300); export re-exports it, a vector test pins Rust parity. */ -function escapeStepString(str: string): string { +export function escapeStepString(str: string): string { const escaped = str .replace(/\\/g, '\\\\') // Backslash .replace(/'/g, "''") // Single quote diff --git a/packages/export/src/step-serialization.test.ts b/packages/export/src/step-serialization.test.ts index 0b5848184..19288cd9a 100644 --- a/packages/export/src/step-serialization.test.ts +++ b/packages/export/src/step-serialization.test.ts @@ -196,69 +196,18 @@ describe('toStepRealScaled', () => { }); }); -describe('escapeStepString non-ASCII encoding (ISO 10303-21 6.3.3.4)', () => { - // ISO 10303-21 restricts a string literal's plain-text bytes to the "basic - // graphic" range 32-126; anything outside it is a control directive - // (\X\HH, \X2\HHHH\X0\, \X4\HHHHHHHH\X0\), never a raw byte. buildingSMART's - // own IFC string-encoding guidance states the same for IFC2X3/IFC4/IFC4X3: - // "characters ... represented by decimal value 32 to 126 ... any other - // character ... has to be encoded" (e.g. German 'Ä' as '\X2\00C4\X0\'). - // A reader that treats the file bytes as ISO-8859-1 (the byte encoding real - // consumers - and the base standard - assume) turns a raw UTF-8 multi-byte - // sequence into mojibake or an outright parse break; this is a reported, - // reproduced defect in real IFC tooling (IfcOpenShell#699, files rejected - // by Solibri) for exactly this shape of writer bug. - it('encodes a BMP character as \\X2\\HHHH\\X0\\, not raw UTF-8', () => { - expect(escapeStepString('Trümpler')).toBe('Tr\\X2\\00FC\\X0\\mpler'); - }); - - it('encodes a non-BMP character (emoji) as \\X4\\HHHHHHHH\\X0\\', () => { - expect(escapeStepString('😀')).toBe('\\X4\\0001F600\\X0\\'); - }); - - it('leaves printable ASCII untouched', () => { - expect(escapeStepString('plain text 123')).toBe('plain text 123'); - }); -}); - - /** - * A run of control characters becomes ONE SPACE PER CHARACTER (#3284 item 2). - * - * The expectations below are not invented: they are the observed output of the - * Rust half, `ifc_lite_export::step_text::escape`, over the same six inputs — - * the doc comment on each escaper claims it "matches" the other, and until - * this fix the TS `/[\x00-\x1F\x7F]+/g` collapsed `"a\t\t\tb"` to `'a b'` - * while Rust wrote `'a b'`. ISO 10303-21 6.3.3.4 mandates neither (it only - * bars the control byte from the literal), so the tie is broken by the parity - * claim and by information loss: collapsing discards the run's length. + * `escapeStepString` no longer has an implementation here (#3300): this + * package re-exports `@ifc-lite/data`'s. Full coverage -- non-ASCII `\X2\`/ + * `\X4\` directives, one-space-per-control-char (#3284), and the vector + * comparison against `ifc_lite_export::step_text::escape` -- lives once, in + * `packages/data/src/step-serializers.test.ts`. This is a wiring check that + * the re-export resolves to a working function, not a second copy of that + * suite. */ -describe('escapeStepString control-character runs (#3284, parity with the Rust escape)', () => { - // label, input, and the output the Rust half printed for that input. - const RUST_VECTORS: ReadonlyArray = [ - ['tab run', 'a\t\t\tb', 'a b'], - ['crlf', 'a\r\nb', 'a b'], - ['mixed C0 + DEL', 'a\u0000\u000B\u001F\u007Fb', 'a b'], - ['single control char', 'a\tb', 'a b'], - ['quote doubling around a run', "O'Brien\t\tx", "O''Brien x"], - // Negative control: no control characters at all, byte-identical output. - ['no control chars', 'plain text 123', 'plain text 123'], - ]; - - it.each(RUST_VECTORS)('%s escapes exactly as the Rust half does', (_label, input, expected) => { - expect(escapeStepString(input)).toBe(expected); - }); - - it('preserves the length of every control run and emits no control byte', () => { - // Both directions of the rule in one place: the output must have the same - // length as the input (one space per control character), and must contain - // no control character (a run left intact would also keep its length, so - // neither half alone is sufficient). - for (const n of [1, 2, 3, 8]) { - const escaped = escapeStepString(`a${'\n'.repeat(n)}b`); - expect(escaped).toBe(`a${' '.repeat(n)}b`); - // eslint-disable-next-line no-control-regex - expect(escaped).not.toMatch(/[\u0000-\u001F\u007F]/); - } +describe('escapeStepString (re-exported from @ifc-lite/data, #3300)', () => { + it('escapes quotes, backslashes and non-ASCII the way @ifc-lite/data does', () => { + expect(escapeStepString("O'Brien\\x")).toBe("O''Brien\\\\x"); + expect(escapeStepString('\u00C4')).toBe('\\X2\\00C4\\X0\\'); }); }); diff --git a/packages/export/src/step-serialization.ts b/packages/export/src/step-serialization.ts index bf4315681..47a0117c0 100644 --- a/packages/export/src/step-serialization.ts +++ b/packages/export/src/step-serialization.ts @@ -22,7 +22,17 @@ */ import { serializeValue, SCHEMA_REGISTRY, type IfcAttributeValue } from '@ifc-lite/parser'; -import { QuantityType, formatStepReal } from '@ifc-lite/data'; +import { QuantityType, formatStepReal, escapeStepString } from '@ifc-lite/data'; + +/** + * Re-exported so every existing `import { escapeStepString } from + * './step-serialization.js'` call site keeps working. The implementation + * lives once, in `@ifc-lite/data` (#3300) — this package used to keep its + * own byte-identical copy, which is exactly the duplication + * `packages/codegen/test/serialization-generator.test.ts` already forbids + * for the schema-agnostic serializer bundles. + */ +export { escapeStepString }; /** EXPRESS base primitives a defined type ultimately resolves to. */ const EXPRESS_PRIMITIVES = new Set(['BOOLEAN', 'LOGICAL', 'INTEGER', 'REAL', 'NUMBER', 'STRING', 'BINARY']); @@ -115,65 +125,6 @@ export function serializeTypedMarker(type: string, value: string | number | bool return `${token}(${serializeInnerByBase(value, resolveExpressBase(type))})`; } -/** - * Escape a string for STEP format: backslash and single-quote doubling, plus - * non-ASCII directive encoding. - * - * Control characters (CR/LF and other C0 codes, plus DEL) are each replaced by - * ONE space — a run of n control characters becomes n spaces, not one — so - * every generated STEP entity stays on one physical line and round-trips - * through the line-oriented merge/convert paths without losing length. - * Collapsing a run (`/[...]+/`, which this did until #3284) disagreed with - * `ifc_lite_export::step_text::escape`, which has always mapped each control - * character to its own space: the same value written by the two halves came - * out different. ISO 10303-21 6.3.3.4 mandates neither, but preserving the - * count loses no information. - * - * ISO 10303-21 6.3.3.4 restricts a string literal's plain-text bytes to the - * "basic graphic" range 32-126; every other character is a control directive - * (`\X\HH`, `\X2\HHHH\X0\`, `\X4\HHHHHHHH\X0\`), never a raw byte — and - * buildingSMART's IFC string-encoding guidance states the same for the - * IFC2X3/IFC4/IFC4X3 schemas this exporter targets: characters outside - * decimal 32-126 "have to be encoded" (e.g. 'Ä' as `\X2\00C4\X0\`). A reader - * that treats the file's bytes as ISO-8859-1 — the byte encoding the base - * standard and most real consumers assume for these schemas — turns a raw - * UTF-8 multi-byte sequence into mojibake or a broken parse; this exact - * writer shape is a reported, reproduced defect in real IFC tooling - * (IfcOpenShell#699/#1016; files rejected by Solibri). Encoded one character - * at a time (never batching a run into one `\X2\...\X0\` block) to keep the - * encoder trivially correct; ISO 10303-21 allows either. - */ -export function escapeStepString(str: string): string { - const escaped = str - .replace(/\\/g, '\\\\') - .replace(/'/g, "''") - // eslint-disable-next-line no-control-regex - .replace(/[\x00-\x1F\x7F]/g, ' '); - return encodeNonAsciiStepDirectives(escaped); -} - -/** - * Replace every character outside the STEP "basic graphic" range (32-126) - * with its `\X2\`/`\X4\` directive. Must run AFTER backslash-doubling: the - * directive's own backslashes are literal syntax the reader expects - * undoubled, so escaping non-ASCII first would corrupt them when the - * backslash-doubling pass ran afterward. - */ -function encodeNonAsciiStepDirectives(str: string): string { - let out = ''; - for (const ch of str) { - const cp = ch.codePointAt(0)!; - if (cp >= 32 && cp <= 126) { - out += ch; - } else if (cp <= 0xFFFF) { - out += `\\X2\\${cp.toString(16).toUpperCase().padStart(4, '0')}\\X0\\`; - } else { - out += `\\X4\\${cp.toString(16).toUpperCase().padStart(8, '0')}\\X0\\`; - } - } - return out; -} - /** * Convert a number to a valid STEP REAL literal. * diff --git a/scripts/api-surface.json b/scripts/api-surface.json index 79d3519a1..ea4aa5dbf 100644 --- a/scripts/api-surface.json +++ b/scripts/api-surface.json @@ -1020,6 +1020,7 @@ "entityTableFromColumns: function", "entityTableToColumns: function", "enumVal: function", + "escapeStepString: function", "exactNameOfRow: function", "exactTypeName: function", "findAttribute: function",