fix(data,export): collapse the STEP string escaper's two TS copies into one (#3300)

.changeset/collapse-step-escaper-copies.md

@@ -0,0 +1,10 @@
+---
+'@ifc-lite/data': patch
+'@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`.

packages/data/src/index.ts

@@ -56,6 +56,7 @@ export {
   generateHeader,
   generateStepFileWithRegistry,
   parseStepValue,
+  escapeStepString,
 } from './step-serializers.js';
 export type {
   StepValue,

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<readonly [string, string, string]> = [
@@ -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('');
+  });
+});

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

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<readonly [string, string, string]> = [
-    ['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\\');
   });
 });

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.
  *

scripts/api-surface.json

@@ -1020,6 +1020,7 @@
     "entityTableFromColumns: function",
     "entityTableToColumns: function",
     "enumVal: function",
+    "escapeStepString: function",
     "exactNameOfRow: function",
     "exactTypeName: function",
     "findAttribute: function",