From 8984e5d3e92f9d97f573ea6c8fa1ab8e9a77d9b8 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 16 Oct 2026 11:50:19 -0400 Subject: [PATCH 1/3] Carry a service function's rename into the System Composer catalog Renaming a function of a service interface now renames System Composer's copy of it too (p_Name and p_FunctionPrototype in the interface dictionary), in text and binary dictionaries, as one undo step (ruling A5). Core's own save already does this (data-explorer-core #109), but neither editor saves through core, and catalogRenameOf only passes a rename of the entry itself, so the catalog kept the old name. - scRename.ts: a second gate, catalogFunctionRenameOf (a Name edit of a Simulink.FunctionElement directly under the top-level entry, read before the mutation), and two format helpers over core's scanScJsonFunctions / scanScXmlFunctions and scFunctionRenameEdits. - Each provider takes one more branch in its scEdits / scPart expression. The text editor puts the catalog ranges in the same WorkspaceEdit. The binary editor's member swap rides pushEdit, whose undo restores the member's original bytes. - A rename the Prototype cannot follow is refused by core before any catalog edit is computed, and shows as a validationError like every other refused edit. Tests: scRenameCarry.test.ts runs each scenario (one rename, interface renamed before or after, chain, rename back, swap) through the provider's composition and through core's own serializeSource, in both formats, and requires the same catalog. archRenameCarry.test.ts adds a save test and an undo test on a new fixture, arch_service_binary.sldd. This needs the barrel exports on data-explorer-core's export-function-rename-catalog-carry branch. The pin moves to the core release that carries them in a separate change. --- src/host/BinarySlddEditorProvider.ts | 18 +- src/host/SlddTextEditorProvider.ts | 13 +- src/host/scRename.ts | 91 +++ .../suite/archRenameCarry.test.ts | 113 +++- test/fixtures/arch_service_binary.sldd | Bin 0 -> 1426 bytes test/fixtures/make-fixtures.mjs | 71 ++- test/scRenameCarry.test.ts | 517 +++++++++++++++++- 7 files changed, 788 insertions(+), 35 deletions(-) create mode 100644 test/fixtures/arch_service_binary.sldd diff --git a/src/host/BinarySlddEditorProvider.ts b/src/host/BinarySlddEditorProvider.ts index 85ce8bf..3bcba5f 100644 --- a/src/host/BinarySlddEditorProvider.ts +++ b/src/host/BinarySlddEditorProvider.ts @@ -63,7 +63,13 @@ import { deleteEntriesByNameXml, type StructuralResult, } from './xmlStructuralEdit.js'; -import { catalogRenameOf, scXmlRenamePatch, type ScPartPatch } from './scRename.js'; +import { + catalogFunctionRenameOf, + catalogRenameOf, + scXmlFunctionRenamePatch, + scXmlRenamePatch, + type ScPartPatch, +} from './scRename.js'; import { applyEntryOps, entryRecord, @@ -1105,6 +1111,9 @@ export class BinarySlddEditorProvider implements vscode.CustomEditorProvider + new Map([[rename.interfaceName, new Map([[rename.oldName, rename.newName]])]]); + /** * The range replacements that carry a rename into an uncompressed-text dictionary's * catalog, as offsets into the document text the caller is about to edit. @@ -85,6 +142,18 @@ export function scJsonRenameEdits(text: string, oldName: string, newName: string return scRenameEdits(scanScJsonText(text), oldName, newName); } +/** + * The range replacements that carry a function rename into an uncompressed-text + * dictionary's catalog, as offsets into the document text the caller is about to edit: + * the function's `p_Name`, and its `p_FunctionPrototype` where that declares it by name. + * + * Core's own edits, the ones its save applies, run on the text as it stands. Empty when the + * catalog does not list the function under that interface, or when there is no catalog part. + */ +export function scJsonFunctionRenameEdits(text: string, rename: CatalogFunctionRename): ScTextEdit[] { + return scFunctionRenameEdits(text, scanScJsonFunctions(text), asFunctionRenames(rename)); +} + /** A pass-through zip member, before and after a rename was carried into it. */ export interface ScPartPatch { member: string; @@ -117,3 +186,25 @@ export function scXmlRenamePatch( } return { member: SC_PART_XML, before, after: new TextEncoder().encode(applyScEdits(xml, edits)) }; } + +/** + * The catalog member swap that carries a function rename into a compressed-binary + * dictionary, or null when there is nothing to carry. It has `scXmlRenamePatch`'s shape, so + * it rides `pushEdit` and its undo the same way, and `before` is again the member's original + * bytes. + */ +export function scXmlFunctionRenamePatch( + zipMeta: Record, + rename: CatalogFunctionRename, +): ScPartPatch | null { + const before = zipMeta[SC_PART_XML]; + if (!before) { + return null; + } + const xml = new TextDecoder().decode(before); + const edits = scFunctionRenameEdits(xml, scanScXmlFunctions(xml), asFunctionRenames(rename)); + if (edits.length === 0) { + return null; + } + return { member: SC_PART_XML, before, after: new TextEncoder().encode(applyScEdits(xml, edits)) }; +} diff --git a/test-integration/suite/archRenameCarry.test.ts b/test-integration/suite/archRenameCarry.test.ts index b0a0d67..1c9311e 100644 --- a/test-integration/suite/archRenameCarry.test.ts +++ b/test-integration/suite/archRenameCarry.test.ts @@ -33,7 +33,12 @@ import { findEntryObjectSpan, } from 'data-explorer-core'; import { BinarySlddEditorProvider } from '../../src/host/BinarySlddEditorProvider'; -import { catalogRenameOf, scXmlRenamePatch } from '../../src/host/scRename'; +import { + catalogFunctionRenameOf, + catalogRenameOf, + scXmlFunctionRenamePatch, + scXmlRenamePatch, +} from '../../src/host/scRename'; import { mutateEntry } from '../../src/host/entryOps'; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -57,14 +62,15 @@ function token() { const settle = (ms: number) => new Promise((r) => setTimeout(r, ms)); /** - * A working copy of the vitest suite's architectural fixture. + * A working copy of one of the vitest suite's architectural fixtures (`arch_binary.sldd` + * unless named). * * Sourced from `test/fixtures` rather than duplicated here, so the dictionary this asks * about is the same one the unit tests ask about; and written OUTSIDE the workspace folder, * whose exact file set two other suites assert. */ -async function workingCopy(name: string): Promise { - const src = vscode.Uri.joinPath(ws(), '..', '..', '..', 'test', 'fixtures', 'arch_binary.sldd'); +async function workingCopy(name: string, fixture = 'arch_binary.sldd'): Promise { + const src = vscode.Uri.joinPath(ws(), '..', '..', '..', 'test', 'fixtures', fixture); const dst = vscode.Uri.joinPath(ws(), '..', name); await vscode.workspace.fs.copy(src, dst, { overwrite: true }); return dst; @@ -132,6 +138,43 @@ function renameEntry(doc: any, root: any, name: string, newName: string) { return { before, after, part: part! }; } +/** + * applyEdit's rename of a service interface's FUNCTION, minus the webview message: the + * catalog keeps its own copy of the function (`p_Name`, `p_FunctionPrototype`), so this + * edit carries a member swap too, through the function gate rather than the entry one. + */ +function renameFunction(doc: any, root: any, entryName: string, name: string, newName: string) { + const entry = entryNamed(root, entryName); + assert.ok(entry, `the fixture has an entry named ${entryName}`); + const node = (entry.children as any[]).find((c: any) => c.name === name); + assert.ok(node, `${entryName} has a function named ${name}`); + const selector = entrySelectorOf(entry); + assert.strictEqual(catalogRenameOf('Name', newName, node, entry), null, 'a function is not an entry rename'); + const functionRename = catalogFunctionRenameOf('Name', newName, node, entry); + assert.ok(functionRename, 'a function of a top-level service interface is a rename the catalog can carry'); + + assert.strictEqual(mutateEntry(entry, () => node.setProperty('Name', newName)), true); + + const before = doc.chunkXml as string; + const span = findEntryObjectSpan(before, selector); + assert.ok(span, 'the entry’s XML object is locatable'); + const frag = serializeEntryToXml(entry).replace(/\n$/, ''); + const after = before.slice(0, span!.offset) + frag + before.slice(span!.offset + span!.length); + const part = scXmlFunctionRenamePatch(doc.zipMeta, functionRename!); + assert.ok(part, 'the catalog lists this function, so the rename has a part to carry'); + + doc.pushEdit('Edit Name', before, after, undefined, part); + return { before, after, part: part! }; +} + +/** `text` with each `from` replaced by its `to`, checking first that `from` occurs exactly once. */ +function replacedOnce(text: string, pairs: [string, string][]): string { + return pairs.reduce((out, [from, to]) => { + assert.strictEqual(out.split(from).length - 1, 1, `"${from}" occurs exactly once`); + return out.replace(from, () => to); + }, text); +} + suite('Architectural rename carries into the System Composer catalog', () => { let provider: BinarySlddEditorProvider; @@ -205,6 +248,68 @@ suite('Architectural rename carries into the System Composer catalog', () => { await vscode.workspace.fs.delete(uri); }); + test('a save writes a renamed service function in the chunk and in the catalog', async () => { + // The catalog's copy of a service interface's function: without the carry the saved + // chunk says `compute` and the saved catalog still says `f`, which MATLAB reads as two + // different functions. + const uri = await workingCopy('arch_rename_function_save.sldd', 'arch_service_binary.sldd'); + const original = await partsOnDisk(uri); + + const { doc } = await open(provider, uri); + const { part } = renameFunction(doc, DataModel.getDataSource((doc as any).srcId), 'ServiceInterface', 'f', 'compute'); + assert.deepStrictEqual((doc as any).zipMeta[SC_PART_XML], part.after, 'the edit swapped the catalog member on the document'); + await provider.saveCustomDocument(doc, token()); + + const saved = await partsOnDisk(uri); + assert.ok(saved.chunkXml.includes('>compute<'), 'the saved chunk spells the new name'); + assert.ok(saved.chunkXml.includes('y = compute(u)'), 'and the Prototype that declares it'); + // Exactly the function's two sites: the data interface named f and g's argument g stay. + assert.strictEqual( + new TextDecoder().decode(saved.zipMeta[SC_PART_XML]), + replacedOnce(new TextDecoder().decode(original.zipMeta[SC_PART_XML]), [ + ['y = f(u)', 'y = compute(u)'], + ['f\n ', 'compute\n '], + ]), + 'the saved catalog renames the function at its p_Name and p_FunctionPrototype, and nowhere else', + ); + assert.strictEqual(kindsOnDisk(saved).ServiceInterface, 'Service Interface', 'and still models the entry'); + + doc.dispose(); + await vscode.workspace.fs.delete(uri); + }); + + test('undo of a service function rename restores the catalog member byte for byte, and a save writes that back', async () => { + const uri = await workingCopy('arch_rename_function_undo.sldd', 'arch_service_binary.sldd'); + const original = await partsOnDisk(uri); + + const { doc } = await open(provider, uri); + const edits: vscode.CustomDocumentEditEvent[] = []; + const sub = provider.onDidChangeCustomDocument((e) => edits.push(e)); + const { before, part } = renameFunction(doc, DataModel.getDataSource((doc as any).srcId), 'ServiceInterface', 'f', 'compute'); + assert.strictEqual(edits.length, 1, 'the rename is ONE edit on VS Code’s undo stack'); + + edits[0].undo(); + assert.strictEqual((doc as any).chunkXml, before, 'undo restored the chunk'); + assert.deepStrictEqual( + (doc as any).zipMeta[SC_PART_XML], + original.zipMeta[SC_PART_XML], + 'and the catalog member, byte for byte', + ); + + edits[0].redo(); + assert.deepStrictEqual((doc as any).zipMeta[SC_PART_XML], part.after, 'redo swapped it back'); + + edits[0].undo(); + await provider.saveCustomDocument(doc, token()); + const saved = await partsOnDisk(uri); + assert.strictEqual(saved.chunkXml, original.chunkXml, 'the saved chunk is the undone one'); + assert.deepStrictEqual(saved.zipMeta[SC_PART_XML], original.zipMeta[SC_PART_XML], 'and so is the saved catalog'); + + sub.dispose(); + doc.dispose(); + await vscode.workspace.fs.delete(uri); + }); + test('a revert brings the catalog back from disk too, not just the chunk', async () => { // The half of a revert that is easy to miss: it re-reads the file, and a revert that // restored only `data/chunk0.xml` would leave the patched catalog in place — a file whose diff --git a/test/fixtures/arch_service_binary.sldd b/test/fixtures/arch_service_binary.sldd new file mode 100644 index 0000000000000000000000000000000000000000..d4a7aed7191525778b8c2602a09a50328b463179 GIT binary patch literal 1426 zcmWIWW@Zs#U|`^2SQ8Tv`(?e^4>=|Vh6$Vu4E#XRl*E!m{p5_&ylex#irk#HVX@h_ z4Mb|suQ#ZAb!^Uw_)MNo?`U&@h}{$V4@o4S_|&`j?6#(q_9yrK&$UNaXKp(5;!}V< zN6y^pb?0^-U-0g#R_9h#j*}I)oZW<#d4oy$mvGY?>&J;U3zAm0P z;mt-J$6L1E?gGpcFE}d;OPe{~+PdJgoEL{`u5=}@ukBs^gZ>LG^H>zoCHl6p#E<3EF@v@n&ATsh#lb+?Y`gqzqZ^IJSD$IBGD^p$xol! zo5xQ4$&kssbv`(1!uxOYOXaxceazXr;(W&1uUl@q{#z(esS~!D>p|4IU@LK(r(XBu zBtCZpmd5ahZ&W&E-W0dcdFQv~Et{fN9lUn_db&1;f9DD9hDY&pHQeV(#`fx;Z>|u& zu<6M(^=^qUoz=&^7&=!!d79|`WST$gs-31Qwod->%xum1D{cFYpMI76?`Fq(KlI$< z?iVTN7T3=yeqH}$kJh6awm-+rCX`Ea1r#hZn^ST7UTf6t(-U?@ZJqZnEbg&+07_B{ zoK?%Xh>?MzmV<#oADEPiGjmIGGV`+aiz|yuQgf5@a|`l|Q;YO7^GZ^S(h`$XT{4qP zGV}8iiz-1$?d-I`{3Zi|AHQpt3mhdPJTVLH2>2{S(aQAgD#0p&EMa| z)HKV|^w{=o<=6kOf7+M5et+7+jVy}HayQe&OV8}P{HnKIR{2;;qh?K~!>wiOL;hq% zPWo}cASvRfww{=f;5nUVr8+acSbfeX={cU+pisAq@lHRsL~&6e^M+Oh3qQLV4Qc{@X9cRkPZxB9pbw{V+wiE!`> zOC+>9`}tqv>vFhqXOV+KbW6dXXXS-L@42sb#QoiHYr%8T!>I{H zQ*{?t_~g*7mwBHIq_3TA z-@J`4EagJ)rS5x1do!HZ?GmZ#f9ZPr2Z!E1|2sbg@BI9_>Y=p#+4>UK3+z=}yDUC@ z{J*B-&fcHJc1z_`vfHbY?wyp*{ePrv&3*A{bqN-0<58pgTvUOH5-^%|fLIoYb5lza z!KFuTd2)VHYCutbL26M+W@<4gayPHsu=%8~&Ph)nF0ZqmzB+zexs8krjQqD;bQj%v zas38nh5&CyCJ_eQ1ryK)B!E^-p=(A@iU^(Fz_O0Sgo$oAy2lZQTd@!bh5&C?Hjr{= OAbbj>rvo)GFaQ7^VP7Qx literal 0 HcmV?d00001 diff --git a/test/fixtures/make-fixtures.mjs b/test/fixtures/make-fixtures.mjs index 9ed2739..e2f910d 100644 --- a/test/fixtures/make-fixtures.mjs +++ b/test/fixtures/make-fixtures.mjs @@ -572,6 +572,75 @@ writeFileSync( }), ); +// --- arch_service_binary.sldd: a SERVICE INTERFACE in the compressed-binary format --- +// +// The fixture a FUNCTION rename's carry needs, which `arch_binary.sldd` cannot be: it has no +// service interface, and other suites count its entries. The catalog keeps its own copy of +// each function of a service interface (`p_Name` and `p_FunctionPrototype` under the +// interface's definition), so renaming a function has to patch that member too. +// +// The same shape as core's functionElementRename.test.ts, which is the corpus's one binary +// dictionary with a service interface (example/dd1.sldd), trimmed: `ServiceInterface` holds +// `f: y = f(u)` and `g: [a,b] = g(g)`. It carries two traps, because the host patches text: +// a DATA interface that is also named `f`, and `g`'s first argument, which is named `g`. A +// rename of either function that took either of those would corrupt something else. +const svcArg = (argName) => + `

Fixed

${argName}

double

real

1.0

`; +const svcFn = (fnName, prototype, args) => + `

${prototype}

${fnName}

0

${args.map(svcArg).join('')}

`; +const svcChunkXml = + `\n` + + `\n` + + ` \n` + + `

ServiceInterface

\n` + + `

1c399950-c59b-404a-91e9-0cd02c0527ce

\n` + + `

${ARCH_NS}

\n` + + `

20260306T201108.832214

\n` + + `

user

\n` + + `

1

\n` + + `

${svcFn('f', 'y = f(u)', ['u', 'y'])}${svcFn('g', '[a,b] = g(g)', ['g', 'a', 'b'])}

\n` + + ` \n` + + `\n`; +const svcFnXml = (fnName, prototype, args) => + `\n ` + + args + .map( + (a) => + `\n ` + + `\n ${a}` + + `\n `, + ) + .join('') + + `\n ${prototype}` + + `\n 1` + + `\n ${fnName}` + + `\n `; +const svcScXml = + `\n` + + `\n` + + ` \n` + + ` svc\n` + + ` \n` + + ` \n` + + ` f\n` + + ` \n` + + ` ` + + svcFnXml('f', 'y = f(u)', ['u', 'y']) + + svcFnXml('g', '[a,b] = g(g)', ['g', 'a', 'b']) + + `\n ServiceInterface\n` + + ` \n` + + ` \n` + + ` \n` + + `\n`; +writeFileSync( + here('arch_service_binary.sldd'), + zipSync({ + 'data/chunk0.xml': strToU8(svcChunkXml), + 'simulink/systemcomposer/interfaceDictionary.xml': strToU8(svcScXml), + 'metadata/mwcoreProperties.xml': strToU8(`R2027a`), + }), +); + // --- chain_*: a model whose dictionary chain runs THROUGH A COMPRESSED dictionary --- // // The set that pins the cheap tier's one real hazard. Scoping a Usage answer to "the models @@ -680,6 +749,6 @@ writeFileSync( console.log( 'wrote model_with_refs.slx, model_with_refs.mdl, legacy_ctrl.mdl, compressed.sldd, ' + 'object_array_binary.sldd, nd_numeric.mat, params.sldd, shared_gain.slx, shadow_ws.slx, ' + - 'sid_blocks.slx, arch_binary.sldd, chain_top.sldd, chain_leaf.sldd, chain_model.slx, ' + + 'sid_blocks.slx, arch_binary.sldd, arch_service_binary.sldd, chain_top.sldd, chain_leaf.sldd, chain_model.slx, ' + 'shadow_first.sldd, shadow_second.sldd, shadow_pair.slx', ); diff --git a/test/scRenameCarry.test.ts b/test/scRenameCarry.test.ts index 8c2f879..f85cd97 100644 --- a/test/scRenameCarry.test.ts +++ b/test/scRenameCarry.test.ts @@ -41,6 +41,8 @@ import { DataModel, SlddNode, SC_PART_XML, + createSession, + ingest, serializeEntryToXml, entrySelectorOf, findEntryObjectSpan, @@ -50,7 +52,14 @@ import { findEntrySpan, detectIndent } from '../src/host/entrySplice.js'; import { reserializeEntry, findOwningEntry } from '../src/host/structuralEdit.js'; import { mutateEntry } from '../src/host/entryOps.js'; import { readSlddParts } from '../src/host/slddContent.js'; -import { catalogRenameOf, scJsonRenameEdits, scXmlRenamePatch } from '../src/host/scRename.js'; +import { + catalogFunctionRenameOf, + catalogRenameOf, + scJsonFunctionRenameEdits, + scJsonRenameEdits, + scXmlFunctionRenamePatch, + scXmlRenamePatch, +} from '../src/host/scRename.js'; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -72,23 +81,32 @@ const kindsOf = (sldd: SlddNode): Record => { const openUris: string[] = []; -/** SlddTextEditorProvider's applyEdit, minus the parts that need a webview. */ -function openJson(uri: string) { +/** + * SlddTextEditorProvider's applyEdit, minus the parts that need a webview. + * + * `edit` rewrites the fixture's text before anything reads it, for a case the checked-in + * file does not have (a Prototype that does not declare its own function). + */ +function openJson(uri: string, edit: (text: string) => string = (text) => text) { if (!openUris.includes(uri)) openUris.push(uri); DataModel.removeDataSource(uri); invalidate(uri); - const doc = { text: readFileSync(fixture('./fixtures/arch.sldd'), 'utf8') }; + const doc = { text: edit(readFileSync(fixture('./fixtures/arch.sldd'), 'utf8')) }; const live = () => getModel(uri, 'arch.sldd', doc.text); const entryNamed = (name: string) => (live() as any).children.flatMap((s: any) => s.children).find((e: any) => e.name === name); + /** A function of a service interface entry, as its row resolves. */ + const functionNamed = (entryName: string, name: string) => + entryNamed(entryName).children.find((c: any) => c.name === name); /** * A table rename, as the provider composes it: read the document text, mutate the * tree it is holding, then submit ONE edit holding the entry's new span text and - * every catalog site that names it. + * every catalog site that names it. A refused rename returns the refusal and writes + * nothing, which is where the provider's validationError branch returns. */ - const rename = (rowId: string, newValue: string) => { + const attemptRename = (rowId: string, newValue: string) => { const currentText = doc.text; live(); const node = findNode(uri, rowId); @@ -97,17 +115,31 @@ function openJson(uri: string) { const selector = entrySelectorOf(entry); // Read BEFORE the mutation: the catalog is keyed by the name the file still spells. const carry = catalogRenameOf('Name', newValue, node, entry); + const functionCarry = catalogFunctionRenameOf('Name', newValue, node, entry); - expect(mutateEntry(entry, () => (node as any).setProperty('Name', newValue))).toBe(true); + const result = mutateEntry(entry, () => (node as any).setProperty('Name', newValue)); + if (result !== true) { + return { result, edits: [], carry, functionCarry }; + } const span = findEntrySpan(currentText, selector); expect(span, `the entry text for "${selector.name}" is locatable`).not.toBeNull(); const edits = [ { start: span!.offset, end: span!.offset + span!.length, text: reserializeEntry(entry, detectIndent(currentText)) }, - ...(carry ? scJsonRenameEdits(currentText, carry.oldName, carry.newName) : []), + ...(carry + ? scJsonRenameEdits(currentText, carry.oldName, carry.newName) + : functionCarry + ? scJsonFunctionRenameEdits(currentText, functionCarry) + : []), ]; doc.text = applyDocEdits(currentText, edits); - return { edits, carry }; + return { result, edits, carry, functionCarry }; + }; + + const rename = (rowId: string, newValue: string) => { + const attempt = attemptRename(rowId, newValue); + expect(attempt.result, `the rename of row ${rowId} to ${newValue} is accepted`).toBe(true); + return attempt; }; /** post(): the wide repaint every multi-change event falls back to. */ @@ -116,7 +148,7 @@ function openJson(uri: string) { return getModel(uri, 'arch.sldd', doc.text) as unknown as SlddNode; }; - return { doc, live, entryNamed, rename, reread }; + return { doc, live, entryNamed, functionNamed, attemptRename, rename, reread }; } /** @@ -153,9 +185,9 @@ afterAll(() => { // --------------------------------------------------------------------------- /** BinarySlddEditorProvider's document, at the size this question needs. */ -function openBinary() { +function openBinary(fixtureName = 'arch_binary.sldd') { const srcId = 'screname:' + Math.random(); - const zip = unzipSync(new Uint8Array(readFileSync(fixture('./fixtures/arch_binary.sldd')))); + const zip = unzipSync(new Uint8Array(readFileSync(fixture('./fixtures/' + fixtureName)))); const zipMeta: Record = {}; for (const [member, data] of Object.entries(zip)) if (member !== 'data/chunk0.xml') zipMeta[member] = data; const doc = { chunkXml: new TextDecoder().decode(zip['data/chunk0.xml']), zipMeta }; @@ -163,7 +195,7 @@ function openBinary() { const build = () => { DataModel.removeDataSource(srcId); return DataModel.addDataSource(srcId, readSlddParts(doc.chunkXml, doc.zipMeta), { - path: 'arch_binary.sldd', + path: fixtureName, }) as unknown as SlddNode; }; const live = () => ((DataModel as any).getDataSource(srcId) ?? build()) as SlddNode; @@ -174,20 +206,36 @@ function openBinary() { * A table rename, as the provider composes it: splice the entry's `` in * chunkXml and swap the catalog member, then hand both to pushEdit so undo can put * them back together. + * + * `child` names a row under the entry (a service interface's function) to rename instead + * of the entry itself. A refused rename returns the refusal and patches nothing, which is + * where the provider's validationError branch returns. */ - const rename = (name: string, newValue: string) => { + const attemptRename = (name: string, newValue: string, child?: string) => { const entry = entryNamed(name); + const node = child === undefined ? entry : entry.children.find((c: any) => c.name === child); + expect(node, `the entry ${name} has a row named ${child}`).toBeTruthy(); const selector = entrySelectorOf(entry); - const carry = catalogRenameOf('Name', newValue, entry, entry); - const part = carry ? scXmlRenamePatch(doc.zipMeta, carry.oldName, carry.newName) : null; + // Both read BEFORE the mutation: the catalog is keyed by the names the file still spells. + const carry = catalogRenameOf('Name', newValue, node, entry); + const functionCarry = catalogFunctionRenameOf('Name', newValue, node, entry); - expect(DataModel.mutateSubtree(entry, () => entry.setProperty('Name', newValue))).toBe(true); + const result = mutateEntry(entry, () => node.setProperty('Name', newValue)); + if (result !== true) { + return { result, part: null, undo: () => {}, carry, functionCarry }; + } const before = doc.chunkXml; const span = findEntryObjectSpan(before, selector); expect(span, `the entry XML for "${name}" is locatable`).not.toBeNull(); const frag = serializeEntryToXml(entry).replace(/\n$/, ''); const after = before.slice(0, span!.offset) + frag + before.slice(span!.offset + span!.length); + // After the validation gate, as in the provider: a refused edit reads no part. + const part = carry + ? scXmlRenamePatch(doc.zipMeta, carry.oldName, carry.newName) + : functionCarry + ? scXmlFunctionRenamePatch(doc.zipMeta, functionCarry) + : null; // pushEdit's forward half, and the undo it hands VS Code. doc.chunkXml = after; @@ -196,27 +244,36 @@ function openBinary() { doc.chunkXml = before; if (part) doc.zipMeta[part.member] = part.before; }; - return { part, undo }; + return { result, part, undo, carry, functionCarry }; + }; + + const rename = (name: string, newValue: string, child?: string) => { + const attempt = attemptRename(name, newValue, child); + expect(attempt.result, `the rename of ${child ?? name} to ${newValue} is accepted`).toBe(true); + return attempt; }; /** repaintAll(): the wide re-read of chunk + pass-through parts. */ const reread = () => build(); - /** writeTo(): the re-zip a save performs, read back as a file would be. */ - const roundTrip = () => { + /** writeTo()'s re-zip, unzipped again: the members a save puts in the file. */ + const saved = () => { const zipped = zipSync({ ...doc.zipMeta, 'data/chunk0.xml': new TextEncoder().encode(doc.chunkXml) }); const files = unzipSync(zipped); const meta: Record = {}; for (const [member, data] of Object.entries(files)) if (member !== 'data/chunk0.xml') meta[member] = data; - return SlddNode.parse( - readSlddParts(new TextDecoder().decode(files['data/chunk0.xml']), meta), - 'arch_binary.sldd', - ); + return { chunkXml: new TextDecoder().decode(files['data/chunk0.xml']), zipMeta: meta }; + }; + + /** writeTo(): the re-zip a save performs, read back as a file would be. */ + const roundTrip = () => { + const files = saved(); + return SlddNode.parse(readSlddParts(files.chunkXml, files.zipMeta), fixtureName); }; const dispose = () => DataModel.removeDataSource(srcId); - return { doc, live, entryNamed, rename, reread, roundTrip, dispose }; + return { doc, live, entryNamed, attemptRename, rename, reread, saved, roundTrip, dispose }; } // --------------------------------------------------------------------------- @@ -393,3 +450,413 @@ describe('carrying a rename in a compressed-binary dictionary', () => { expect(scXmlRenamePatch(zipMeta, 'Kp', 'Ki')).toBeNull(); }); }); + +// --------------------------------------------------------------------------- +// A service interface's FUNCTION, which the catalog keeps a copy of too. +// --------------------------------------------------------------------------- +// +// A `Simulink.FunctionElement` is a row under its ServiceBus entry, so the entry gate above +// never sees it. But the catalog spells each function of a service interface itself, under +// that interface's definition: its `p_Name`, and its `p_FunctionPrototype`, which declares it +// BY NAME (`y = f(u,v)`). Core renames the data part's Prototype with the Name (and refuses a +// rename the Prototype cannot follow), and its own save carries the rename into the catalog. +// Neither editor saves through core, so each has to carry it the way it carries an entry +// rename: extra ranges in the same WorkspaceEdit, or a member swap on the same undo step. +// +// The standard is core's save, not a description of it: every scenario below is run twice, +// once through the provider's composition and once through a fresh core session's +// `serializeSource`, and the two catalogs must be the same text. + +const CATALOG_KEY = '"__MW_TEXT_PART__/simulink/systemcomposer/interfaceDictionary"'; + +/** A textual dictionary's catalog part: from its key to the end of the text. */ +function catalogPartOf(text: string): string { + const at = text.indexOf(CATALOG_KEY); + expect(at, 'the dictionary has a catalog part').toBeGreaterThan(0); + return text.slice(at); +} + +/** Everything before the catalog part: the entries. */ +const dataPartOf = (text: string): string => text.slice(0, text.indexOf(CATALOG_KEY)); + +/** `text` with each `from` replaced by its `to`, checking first that `from` occurs exactly once. */ +function replacedOnce(text: string, pairs: [from: string, to: string][]): string { + return pairs.reduce((out, [from, to]) => { + expect(out.split(from).length - 1, `"${from}" occurs exactly once`).toBe(1); + return out.replace(from, () => to); + }, text); +} + +/** One rename: of the entry `entry`, or of its function `fn`, to `to`. */ +interface Step { + entry: string; + fn?: string; + to: string; +} + +/** Core's own save of `content` after `steps` ran on a fresh session's tree. */ +function coreSaved(content: Uint8Array, filename: string, steps: Step[]) { + const session = createSession(); + const src: any = ingest(session, content, { filename }); + steps.forEach(({ entry, fn, to }) => { + const e = src.children.flatMap((s: any) => s.children).find((x: any) => x.name === entry); + const node = fn === undefined ? e : e.children.find((c: any) => c.name === fn); + expect(node.setProperty('Name', to), `core accepts ${fn ?? entry} -> ${to}`).toBe(true); + }); + const out = session.serializeSource(src.name)!; + session.removeDataSource(src.name); + return out; +} + +/** The scenarios a record of renames has to get right, and an editor that edits as it goes. */ +const STEPS: [label: string, steps: Step[]][] = [ + ['one rename', [{ entry: 'ServiceInterface', fn: 'f', to: 'compute' }]], + [ + 'the interface renamed first, then its function', + [ + { entry: 'ServiceInterface', to: 'Svc' }, + { entry: 'Svc', fn: 'f', to: 'compute' }, + ], + ], + [ + 'the function renamed first, then its interface', + [ + { entry: 'ServiceInterface', fn: 'f', to: 'compute' }, + { entry: 'ServiceInterface', to: 'Svc' }, + ], + ], + [ + 'a chain', + [ + { entry: 'ServiceInterface', fn: 'f', to: 'compute' }, + { entry: 'ServiceInterface', fn: 'compute', to: 'step' }, + ], + ], + [ + 'a rename back', + [ + { entry: 'ServiceInterface', fn: 'f', to: 'compute' }, + { entry: 'ServiceInterface', fn: 'compute', to: 'f' }, + ], + ], +]; + +describe('the function renames that reach the catalog', () => { + const entry = { name: 'ServiceInterface', className: 'Simulink.ServiceBus' }; + const fn = { name: 'f', className: 'Simulink.FunctionElement', parent: entry }; + + it('carries a function of a top-level service interface, keyed by the interface', () => { + expect(catalogFunctionRenameOf('Name', 'compute', fn, entry)).toEqual({ + interfaceName: 'ServiceInterface', + oldName: 'f', + newName: 'compute', + }); + }); + + it('carries the rows a real service interface reads as', () => { + // The fakes above only stand for the tree if a real function row has the entry as its + // parent and the class the gate asks for, which is what this pins. + const d = openJson('test://sc-fn-gate.sldd'); + const real = d.functionNamed('ServiceInterface', 'f'); + expect(catalogFunctionRenameOf('Name', 'compute', real, d.entryNamed('ServiceInterface'))).toEqual({ + interfaceName: 'ServiceInterface', + oldName: 'f', + newName: 'compute', + }); + }); + + it('does not carry an edit to any other column', () => { + expect(catalogFunctionRenameOf('Description', 'a note', fn, entry)).toBeNull(); + expect(catalogFunctionRenameOf('Value', 'y = g(u)', fn, entry)).toBeNull(); + }); + + it('does not carry a rename that renames nothing', () => { + expect(catalogFunctionRenameOf('Name', 'f', fn, entry)).toBeNull(); + }); + + it('does not carry an element of a data bus', () => { + const bus = { name: 'DataInterface', className: 'Simulink.Bus' }; + expect(catalogFunctionRenameOf('Name', 'Speed', { name: 'ValueType', className: 'Simulink.BusElement', parent: bus }, bus)).toBeNull(); + }); + + it('does not carry a row deeper than the function, such as one of its arguments', () => { + const arg = { name: 'u', className: 'Simulink.BusElement', parent: fn }; + expect(catalogFunctionRenameOf('Name', 'x', arg, entry)).toBeNull(); + }); + + it('does not carry the entry itself, which is the entry gate’s case, so the two never both fire', () => { + expect(catalogFunctionRenameOf('Name', 'Svc', entry, entry)).toBeNull(); + expect(catalogRenameOf('Name', 'Svc', entry, entry)).not.toBeNull(); + expect(catalogRenameOf('Name', 'compute', fn, entry)).toBeNull(); + }); +}); + +describe('carrying a function rename in an uncompressed-text dictionary', () => { + it('writes the function’s two catalog sites, and nothing else, in the same edit', () => { + const d = openJson('test://sc-fn-json-sites.sldd'); + const before = d.doc.text; + + const { edits, carry, functionCarry } = d.rename(d.functionNamed('ServiceInterface', 'f').id, 'compute'); + + expect(carry).toBeNull(); + expect(functionCarry).toEqual({ interfaceName: 'ServiceInterface', oldName: 'f', newName: 'compute' }); + // The entry span, the function's `p_Name`, and its `p_FunctionPrototype`. + expect(edits).toHaveLength(3); + // The entries are what the entry span alone writes: the Name and its Prototype. + expect(dataPartOf(d.doc.text)).toBe(dataPartOf(applyDocEdits(before, [edits[0]]))); + expect(dataPartOf(d.doc.text)).toContain('"Name": "compute"'); + expect(dataPartOf(d.doc.text)).toContain('"Prototype": "y = compute(u,v)"'); + // And the catalog differs from the one read at exactly the function's own two sites. + expect(catalogPartOf(d.doc.text)).toBe( + replacedOnce(catalogPartOf(before), [ + ['"p_FunctionPrototype": "y = f(u,v)"', '"p_FunctionPrototype": "y = compute(u,v)"'], + ['"p_Name": "f"', '"p_Name": "compute"'], + ]), + ); + }); + + it.each(STEPS)('writes the catalog core’s own save writes: %s', (_label, steps) => { + const d = openJson('test://sc-fn-json-core.sldd'); + const original = d.doc.text; + + steps.forEach(({ entry, fn, to }) => { + const row = fn === undefined ? d.entryNamed(entry) : d.functionNamed(entry, fn); + const { edits } = d.rename(row.id, to); + // A multi-range edit takes the wide repaint, which re-reads the document. + if (edits.length > 1) d.reread(); + }); + + const core = coreSaved(new TextEncoder().encode(original), 'arch.sldd', steps); + expect(core.kind).toBe('text'); + expect(catalogPartOf(d.doc.text)).toBe(catalogPartOf((core as { text: string }).text)); + }); + + it('writes the catalog core’s own save writes for a swap, which the sibling check makes three renames', () => { + const d = openJson('test://sc-fn-json-swap.sldd'); + const original = d.doc.text; + const steps: Step[] = [ + { entry: 'ServiceInterface', fn: 'f', to: 'tmp' }, + { entry: 'ServiceInterface', fn: 'f1', to: 'f' }, + { entry: 'ServiceInterface', fn: 'tmp', to: 'f1' }, + ]; + steps.forEach(({ entry, fn, to }) => { + d.rename(d.functionNamed(entry, fn!).id, to); + d.reread(); + }); + + const core = coreSaved(new TextEncoder().encode(original), 'arch.sldd', steps) as { text: string }; + expect(catalogPartOf(d.doc.text)).toBe(catalogPartOf(core.text)); + expect(catalogPartOf(d.doc.text)).toContain('"p_FunctionPrototype": "y = f(u,v,w,y)"'); + expect(catalogPartOf(d.doc.text)).toContain('"p_FunctionPrototype": "y = f1(u,v)"'); + }); + + it('still reads as a service interface that lists the renamed function', () => { + const d = openJson('test://sc-fn-json-reread.sldd'); + const catalogBefore = d.live().systemComposer; + + d.rename(d.functionNamed('ServiceInterface', 'f').id, 'compute'); + + const reread = d.reread() as any; + expect(kindsOf(reread).ServiceInterface).toBe('Service Interface'); + const names = d.entryNamed('ServiceInterface').children.map((c: any) => c.name); + expect(names).toContain('compute'); + expect(names).not.toContain('f'); + expect(reread.systemComposer.interfaces).toEqual(catalogBefore.interfaces); + }); + + it('writes nothing extra for a function the catalog does not list', () => { + // `f2` is in the entry and not in the catalog: the gate passes it, and the catalog has + // no site for it, so the edit is the entry span alone and the catalog is as read. + const d = openJson('test://sc-fn-json-unlisted.sldd'); + const before = d.doc.text; + + const { edits, functionCarry } = d.rename(d.functionNamed('ServiceInterface', 'f2').id, 'h'); + + expect(functionCarry).toEqual({ interfaceName: 'ServiceInterface', oldName: 'f2', newName: 'h' }); + expect(edits).toHaveLength(1); + expect(catalogPartOf(d.doc.text)).toBe(catalogPartOf(before)); + }); + + it('refuses a rename the Prototype cannot follow, with core’s reason, and writes nothing', () => { + // A Prototype that does not declare the function by its own name: core cannot rename the + // two together, and MATLAB requires them to match, so the rename is refused. That refusal + // is the `{ error, reason }` every other refused edit returns, which the provider shows. + const d = openJson('test://sc-fn-json-prototype.sldd', (text) => + replacedOnce(text, [['"Prototype": "y = f(u,v)"', '"Prototype": "y = step(u,v)"']]), + ); + const before = d.doc.text; + + const { result, edits } = d.attemptRename(d.functionNamed('ServiceInterface', 'f').id, 'compute'); + + expect(result).toMatchObject({ error: true }); + expect((result as { reason: string }).reason).toContain("'f' cannot be renamed: its Prototype 'y = step(u,v)'"); + expect(edits).toEqual([]); + expect(d.doc.text).toBe(before); + expect(d.functionNamed('ServiceInterface', 'f').Prototype).toBe('y = step(u,v)'); + }); + + it('refuses a sibling’s name, and writes nothing', () => { + const d = openJson('test://sc-fn-json-sibling.sldd'); + const before = d.doc.text; + + const { result, edits } = d.attemptRename(d.functionNamed('ServiceInterface', 'f').id, 'f1'); + + expect(result).toMatchObject({ error: true }); + expect(edits).toEqual([]); + expect(d.doc.text).toBe(before); + }); +}); + +describe('carrying a function rename in a compressed-binary dictionary', () => { + const SVC = 'arch_service_binary.sldd'; + const decode = (bytes: Uint8Array) => new TextDecoder().decode(bytes); + const coreSavedMember = (steps: Step[]) => { + const out = coreSaved(new Uint8Array(readFileSync(fixture('./fixtures/' + SVC))), SVC, steps); + expect(out.kind).toBe('binary'); + return decode(unzipSync((out as { bytes: Uint8Array }).bytes)[SC_PART_XML]); + }; + + it('patches the function’s two catalog sites, and nothing else', () => { + const d = openBinary(SVC); + try { + const original = d.doc.zipMeta[SC_PART_XML]; + + const { part, functionCarry } = d.rename('ServiceInterface', 'compute', 'f'); + + expect(functionCarry).toEqual({ interfaceName: 'ServiceInterface', oldName: 'f', newName: 'compute' }); + expect(part?.member).toBe(SC_PART_XML); + // `before` is the member's original bytes, so undo restores the file as read. + expect(part!.before).toBe(original); + expect(decode(part!.after)).toBe( + replacedOnce(decode(original), [ + ['y = f(u)', 'y = compute(u)'], + ['f\n ', 'compute\n '], + ]), + ); + expect(d.doc.chunkXml).toContain('>compute<'); + expect(d.doc.chunkXml).toContain('y = compute(u)'); + } finally { + d.dispose(); + } + }); + + it('leaves the data interface named f, and g’s argument named g, alone', () => { + const d = openBinary(SVC); + try { + const original = decode(d.doc.zipMeta[SC_PART_XML]); + + const { part } = d.rename('ServiceInterface', 'h', 'g'); + + expect(decode(part!.after)).toBe( + replacedOnce(original, [ + ['[a,b] = g(g)', '[a,b] = h(g)'], + ['g\n ', 'h\n '], + ]), + ); + } finally { + d.dispose(); + } + }); + + it.each(STEPS)('writes the member core’s own save writes: %s', (_label, steps) => { + const d = openBinary(SVC); + try { + steps.forEach(({ entry, fn, to }) => { + const { part } = d.rename(entry, to, fn); + // A part forces the wide repaint, which re-reads chunk and parts together. + if (part) d.reread(); + }); + expect(decode(d.saved().zipMeta[SC_PART_XML])).toBe(coreSavedMember(steps)); + } finally { + d.dispose(); + } + }); + + it('survives the re-zip a save performs', () => { + const d = openBinary(SVC); + try { + const { part } = d.rename('ServiceInterface', 'compute', 'f'); + + const files = d.saved(); + expect(files.zipMeta[SC_PART_XML]).toEqual(part!.after); + expect(files.chunkXml).toContain('>compute<'); + const reread = d.roundTrip() as any; + expect(kindsOf(reread).ServiceInterface).toBe('Service Interface'); + } finally { + d.dispose(); + } + }); + + it('puts both surfaces back on undo, the member byte for byte', () => { + const d = openBinary(SVC); + try { + const originalPart = d.doc.zipMeta[SC_PART_XML]; + const originalChunk = d.doc.chunkXml; + + const { part, undo } = d.rename('ServiceInterface', 'compute', 'f'); + undo(); + + expect(d.doc.zipMeta[SC_PART_XML]).toBe(part!.before); + expect(d.doc.zipMeta[SC_PART_XML]).toBe(originalPart); + expect(d.doc.chunkXml).toBe(originalChunk); + } finally { + d.dispose(); + } + }); + + it('refuses a rename the Prototype cannot follow, with core’s reason, and patches nothing', () => { + const d = openBinary(SVC); + try { + d.doc.chunkXml = replacedOnce(d.doc.chunkXml, [['y = f(u)', 'y = step(u)']]); + const originalPart = d.doc.zipMeta[SC_PART_XML]; + const originalChunk = d.doc.chunkXml; + + const { result, part } = d.attemptRename('ServiceInterface', 'compute', 'f'); + + expect(result).toMatchObject({ error: true }); + expect((result as { reason: string }).reason).toContain("'f' cannot be renamed: its Prototype 'y = step(u)'"); + expect(part).toBeNull(); + expect(d.doc.zipMeta[SC_PART_XML]).toBe(originalPart); + expect(d.doc.chunkXml).toBe(originalChunk); + } finally { + d.dispose(); + } + }); + + it('refuses a sibling’s name, and patches nothing', () => { + const d = openBinary(SVC); + try { + const originalPart = d.doc.zipMeta[SC_PART_XML]; + + const { result, part } = d.attemptRename('ServiceInterface', 'g', 'f'); + + expect(result).toMatchObject({ error: true }); + expect(part).toBeNull(); + expect(d.doc.zipMeta[SC_PART_XML]).toBe(originalPart); + } finally { + d.dispose(); + } + }); + + it('does not patch for a function the catalog does not list', () => { + const d = openBinary(SVC); + try { + // A name no function has; the data interface `f`, which has no functions; and an + // interface the catalog does not define at all. + expect(scXmlFunctionRenamePatch(d.doc.zipMeta, { interfaceName: 'ServiceInterface', oldName: 'h', newName: 'k' })).toBeNull(); + expect(scXmlFunctionRenamePatch(d.doc.zipMeta, { interfaceName: 'f', oldName: 'f', newName: 'k' })).toBeNull(); + expect(scXmlFunctionRenamePatch(d.doc.zipMeta, { interfaceName: 'Other', oldName: 'f', newName: 'k' })).toBeNull(); + } finally { + d.dispose(); + } + }); + + it('does not patch a dictionary that has no catalog part at all', () => { + const zip = unzipSync(new Uint8Array(readFileSync(fixture('./fixtures/compressed.sldd')))); + const zipMeta: Record = {}; + for (const [member, data] of Object.entries(zip)) if (member !== 'data/chunk0.xml') zipMeta[member] = data; + + expect(zipMeta[SC_PART_XML]).toBeUndefined(); + expect(scXmlFunctionRenamePatch(zipMeta, { interfaceName: 'ServiceInterface', oldName: 'f', newName: 'k' })).toBeNull(); + }); +}); From cb72803b42b6f788328565af8dbdc8ad0a80e634 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 16 Oct 2026 12:13:16 -0400 Subject: [PATCH 2/3] Pin core v1.39.0, which exports the function-rename catalog carry data-explorer-core v1.39.0 (mathworks/data-explorer-core#119, released in #120 there) exports scFunctionRenameEdits, scanScJsonFunctions, scanScXmlFunctions and the ScFunction and CatalogFunctionRenames types from its barrel, which the previous commit calls to carry a FunctionElement rename into the System Composer catalog. The lockfile changes only core's entries: the root dependency spec and node_modules/data-explorer-core, now 1.39.0 resolved at 8e334082682eae7901cc07016f4fb90b3cdc7c73. --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6779464..b4a9ce5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "BSD-3-Clause", "dependencies": { "@lit/context": "^1.1.6", - "data-explorer-core": "github:mathworks/data-explorer-core#v1.38.1", + "data-explorer-core": "github:mathworks/data-explorer-core#v1.39.0", "fast-xml-parser": "^5.8.0", "fflate": "^0.8.3", "highlight.js": "^11.11.1", @@ -3184,8 +3184,8 @@ } }, "node_modules/data-explorer-core": { - "version": "1.38.1", - "resolved": "git+ssh://git@github.com/mathworks/data-explorer-core.git#b8c3f2ba7977c812fc03256f5ac4deaf8c4eac68", + "version": "1.39.0", + "resolved": "git+ssh://git@github.com/mathworks/data-explorer-core.git#8e334082682eae7901cc07016f4fb90b3cdc7c73", "license": "BSD-3-Clause", "dependencies": { "fast-xml-parser": "^5.8.0", diff --git a/package.json b/package.json index 35b89fd..ce4c393 100644 --- a/package.json +++ b/package.json @@ -236,7 +236,7 @@ }, "dependencies": { "@lit/context": "^1.1.6", - "data-explorer-core": "github:mathworks/data-explorer-core#v1.38.1", + "data-explorer-core": "github:mathworks/data-explorer-core#v1.39.0", "fast-xml-parser": "^5.8.0", "fflate": "^0.8.3", "highlight.js": "^11.11.1", From ffe323b7cc6eba1de9e23026cd53e45545decd54 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 16 Oct 2026 12:45:14 -0400 Subject: [PATCH 3/3] Drive the function-rename carry through both real editors scRenameCarry.test.ts and archRenameCarry.test.ts compose the edit from the scRename helpers themselves, so neither reaches the providers' edit handler: with SlddTextEditorProvider.ts and BinarySlddEditorProvider.ts put back to main, all 44 unit and 5 integration tests there stay green. This file sends the table's own `edit` message through each real provider and a fake panel, and grades what the provider wrote against core's serializeSource of the same renames: one rename (p_Name and p_FunctionPrototype move, and nothing else), the interface renamed first, a chain, a function the catalog does not list, a Prototype that cannot follow (refused with core's own reason, nothing written), and undo as one step (text: the one change event's inverse gives back the original bytes; binary: the member read from the file, byte for byte). With only SlddTextEditorProvider.ts reverted to main, 4 of the 6 text cases fail and the 6 binary cases pass; with only BinarySlddEditorProvider.ts reverted, 4 of the 6 binary cases fail and the 6 text cases pass. The 2 that pass in each are the controls (the unlisted function and the refusal), which write no catalog edit on main either. --- .../functionRenameThroughEditors.test.ts | 497 ++++++++++++++++++ 1 file changed, 497 insertions(+) create mode 100644 test-integration/suite/functionRenameThroughEditors.test.ts diff --git a/test-integration/suite/functionRenameThroughEditors.test.ts b/test-integration/suite/functionRenameThroughEditors.test.ts new file mode 100644 index 0000000..a83941d --- /dev/null +++ b/test-integration/suite/functionRenameThroughEditors.test.ts @@ -0,0 +1,497 @@ +// Copyright 2026 The MathWorks, Inc. +// Renaming a service interface's function through the REAL editors, the way the table asks +// for it: one `edit` message, through each provider's own message handler. +// +// A `Simulink.FunctionElement` is spelled twice in an architectural dictionary: in the data +// part, and in the System Composer catalog, which keeps its own copy of each function under +// the interface (`p_Name`, and the `p_FunctionPrototype` that declares the function by name). +// A rename that moved only the first would reopen as two functions MATLAB cannot reconcile. +// Each provider carries it in one branch of its edit handler: extra ranges in the same +// WorkspaceEdit in the text editor, and a member swap on the same undo step in the binary one. +// +// test/scRenameCarry.test.ts pins the helpers that branch calls, and archRenameCarry.test.ts +// pins what a save and an undo do with the member swap. Both compose the edit themselves, as +// the handler composes it, so neither one reaches the handler: with either provider put back +// to the one before the carry, all of their tests stay green. This file is the half they +// cannot be. It sends the message the table sends, through a fake panel, and grades what the +// provider wrote against core's own save of the same renames (`createSession` / `ingest` / +// `serializeSource`), which carries a function rename by itself. +// +// Measured, by putting one provider back to origin/main at a time and running this file: the +// four cases per editor that carry a rename fail, and the two that must write no catalog +// edit (a function the catalog does not list, a rename core refuses) pass, as controls. +// +// The text editor's undo is applied as the inverse of the one change event the rename fired, +// as ONE WorkspaceEdit, because the `undo` command does not take effect in this test host +// (see cutPasteUndo.test.ts). One event holding every range is what makes the rename a single +// step on VS Code's stack; the inverse shows those ranges are the whole rename. +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { unzipSync, zipSync } from 'fflate'; +import { SC_PART_XML, createSession, ingest } from 'data-explorer-core'; +import { SlddTextEditorProvider } from '../../src/host/SlddTextEditorProvider'; +import { BinarySlddEditorProvider } from '../../src/host/BinarySlddEditorProvider'; +import { childrenOf, ctx, fakePanel, isEntryRow, nameOf, settle, token, waitFor, ws } from './tools/hostHarness'; + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +const enc = (text: string) => new TextEncoder().encode(text); +const dec = (bytes: Uint8Array) => new TextDecoder().decode(bytes); + +/** The text and binary fixtures the unit suite asks the same questions of. */ +const TEXT_FIXTURE = 'arch.sldd'; +const BINARY_FIXTURE = 'arch_service_binary.sldd'; +const CHUNK_XML = 'data/chunk0.xml'; + +/** Sourced from `test/fixtures`, so the dictionary this asks about is the unit suite's. */ +const fixtureUri = (name: string) => vscode.Uri.joinPath(ws(), '..', '..', '..', 'test', 'fixtures', name); + +/** Written OUTSIDE the workspace folder, whose exact file set two other suites assert. */ +const workingUri = (name: string) => vscode.Uri.joinPath(ws(), '..', name); + +const CATALOG_KEY = '"__MW_TEXT_PART__/simulink/systemcomposer/interfaceDictionary"'; + +/** A textual dictionary's catalog part: from its key to the end of the text. */ +function catalogPartOf(text: string): string { + const at = text.indexOf(CATALOG_KEY); + assert.ok(at > 0, 'the dictionary has a catalog part'); + return text.slice(at); +} + +/** `text` with each `from` replaced by its `to`, checking first that `from` occurs exactly once. */ +function replacedOnce(text: string, pairs: [from: string, to: string][]): string { + return pairs.reduce((out, [from, to]) => { + assert.strictEqual(out.split(from).length - 1, 1, `"${from}" occurs exactly once`); + return out.replace(from, () => to); + }, text); +} + +/** One rename: of the entry `entry`, or of its function `fn`, to `to`. */ +interface Step { + entry: string; + fn?: string; + to: string; +} + +const ONE_RENAME: Step[] = [{ entry: 'ServiceInterface', fn: 'f', to: 'compute' }]; + +/** The sequences a rename has to get right in an editor that writes as it goes. */ +const SEQUENCES: [label: string, steps: Step[]][] = [ + [ + 'the interface renamed first, then its function', + [ + { entry: 'ServiceInterface', to: 'Svc' }, + { entry: 'Svc', fn: 'f', to: 'compute' }, + ], + ], + [ + 'a chain', + [ + { entry: 'ServiceInterface', fn: 'f', to: 'compute' }, + { entry: 'ServiceInterface', fn: 'compute', to: 'step' }, + ], + ], +]; + +/** The node `step` names in a tree of sections, entries and their rows. */ +function nodeOf(root: any, step: Step): any { + const entry = (root.children as any[]).flatMap((s: any) => s.children as any[]).find((e: any) => e.name === step.entry); + assert.ok(entry, `core's tree has an entry named ${step.entry}`); + if (step.fn === undefined) return entry; + const fn = (entry.children as any[]).find((c: any) => c.name === step.fn); + assert.ok(fn, `${step.entry} has a function named ${step.fn}`); + return fn; +} + +/** Core's own save of `content` after `steps` ran on a fresh session's tree. */ +function coreSaved(content: Uint8Array, filename: string, steps: Step[]): any { + const session = createSession(); + const src: any = ingest(session, content, { filename }); + steps.forEach((step) => + assert.strictEqual( + nodeOf(src, step).setProperty('Name', step.to), + true, + `core accepts ${step.fn ?? step.entry} -> ${step.to}`, + ), + ); + const out = session.serializeSource(src.name); + session.removeDataSource(src.name); + assert.ok(out, 'core can write this dictionary'); + return out; +} + +/** What core says when it refuses `step` on `content`. */ +function coreRefusal(content: Uint8Array, filename: string, step: Step): string { + const session = createSession(); + const src: any = ingest(session, content, { filename }); + const result = nodeOf(src, step).setProperty('Name', step.to); + session.removeDataSource(src.name); + assert.ok(result && typeof result === 'object' && result.error, 'core refuses this rename too'); + return String(result.reason); +} + +/** The row the table holds for `step`, from the newest paint that carries it. */ +function rowOf(posts: any[], step: Step): any { + for (let i = posts.length - 1; i >= 0; i--) { + const msg = posts[i]; + if (msg.type !== 'setRows' && msg.type !== 'updateEntryRows') continue; + const rows = msg.rows as any[]; + const entry = rows.find((r) => isEntryRow(r) && nameOf(r) === step.entry); + if (!entry) continue; + if (step.fn === undefined) return entry; + const fn = childrenOf(rows, entry.ID).find((r) => nameOf(r) === step.fn); + if (fn) return fn; + } + assert.fail(`the table was sent a row for ${step.fn ?? step.entry} under ${step.entry}`); +} + +/** The `edit` message the table posts for a committed rename of `row`. */ +const renameMessage = (row: any, to: string) => ({ + type: 'edit', + rowId: row.ID, + columnId: 'Name', + oldValue: nameOf(row), + newValue: to, +}); + +const refusalsIn = (posts: any[]) => posts.filter((m) => m.type === 'validationError'); +const problemsIn = (posts: any[]) => posts.filter((m) => m.type === 'error' || m.type === 'validationError'); + +// --- the text editor --------------------------------------------------------------------- + +/** A working copy of the text fixture (rewritten by `edit` first), open in the real provider. */ +async function openText(copyName: string, edit: (text: string) => string = (text) => text) { + const original = edit(dec(await vscode.workspace.fs.readFile(fixtureUri(TEXT_FIXTURE)))); + const uri = workingUri(copyName); + await vscode.workspace.fs.writeFile(uri, enc(original)); + const doc = await vscode.workspace.openTextDocument(uri); + const changes: vscode.TextDocumentChangeEvent[] = []; + const sub = vscode.workspace.onDidChangeTextDocument((e) => { + if (e.document.uri.toString() === uri.toString() && e.contentChanges.length > 0) changes.push(e); + }); + const view = fakePanel(); + await new SlddTextEditorProvider(ctx()).resolveCustomTextEditor(doc, view.panel, token()); + view.send({ type: 'ready' }); + await waitFor('the first paint', () => view.posts.some((m) => m.type === 'setRows')); + + /** Send one rename and wait for it to be written or refused; the posts it caused. */ + const rename = async (step: Step): Promise => { + const row = rowOf(view.posts, step); + const mark = view.posts.length; + const version = doc.version; + view.send(renameMessage(row, step.to)); + await waitFor( + `the rename to ${step.to} to be written or refused`, + () => doc.version > version || problemsIn(view.posts.slice(mark)).length > 0, + ); + // A rename that carried the catalog is several ranges, which takes the wide repaint; the + // next step reads its row from that. A tail, not the work. + await settle(300); + return view.posts.slice(mark); + }; + + const close = async () => { + view.close(); + sub.dispose(); + await vscode.workspace.fs.delete(uri); + }; + return { uri, doc, original, changes, rename, close }; +} + +suite('A service function rename through the text editor carries into the catalog', () => { + suiteSetup(async () => { + await vscode.extensions.getExtension('mathworks.simulink-data-explorer')?.activate(); + }); + + test('one rename writes the function’s p_Name and p_FunctionPrototype, in the same write as the entry', async () => { + const t = await openText('fn_rename_text_one.sldd'); + try { + const posts = await t.rename(ONE_RENAME[0]); + assert.deepStrictEqual(problemsIn(posts), [], 'the rename was accepted'); + + assert.strictEqual(t.changes.length, 1, 'one write'); + assert.strictEqual(t.changes[0].contentChanges.length, 3, 'holding the entry span, p_Name and p_FunctionPrototype'); + const text = t.doc.getText(); + assert.ok(text.includes('"Prototype": "y = compute(u,v)"'), 'the entry’s Prototype follows the Name'); + assert.strictEqual( + catalogPartOf(text), + replacedOnce(catalogPartOf(t.original), [ + ['"p_FunctionPrototype": "y = f(u,v)"', '"p_FunctionPrototype": "y = compute(u,v)"'], + ['"p_Name": "f"', '"p_Name": "compute"'], + ]), + 'the catalog moves at the function’s two sites and nowhere else', + ); + + const core = coreSaved(enc(t.original), TEXT_FIXTURE, ONE_RENAME); + assert.strictEqual(core.kind, 'text'); + assert.ok(await t.doc.save(), 'the document saved'); + const onDisk = dec(await vscode.workspace.fs.readFile(t.uri)); + assert.strictEqual( + catalogPartOf(onDisk), + catalogPartOf(core.text), + 'the saved catalog is the one core’s own save writes', + ); + } finally { + await t.close(); + } + }); + + for (const [label, steps] of SEQUENCES) { + test(`writes the catalog core’s own save writes: ${label}`, async () => { + const t = await openText(`fn_rename_text_${label.replace(/\W+/g, '_')}.sldd`); + try { + for (const step of steps) { + assert.deepStrictEqual(problemsIn(await t.rename(step)), [], `${step.fn ?? step.entry} -> ${step.to} was accepted`); + } + assert.strictEqual(t.changes.length, steps.length, 'one write per rename'); + const core = coreSaved(enc(t.original), TEXT_FIXTURE, steps); + assert.strictEqual( + catalogPartOf(t.doc.getText()), + catalogPartOf(core.text), + 'the catalog is the one core’s own save writes', + ); + } finally { + await t.close(); + } + }); + } + + test('a function the catalog does not list leaves the catalog untouched', async () => { + // `f2` is in the entry and not in the catalog, so the rename is the entry span alone. + const step: Step = { entry: 'ServiceInterface', fn: 'f2', to: 'h' }; + const t = await openText('fn_rename_text_unlisted.sldd'); + try { + assert.deepStrictEqual(problemsIn(await t.rename(step)), [], 'the rename was accepted'); + assert.strictEqual(t.changes.length, 1, 'one write'); + assert.strictEqual(t.changes[0].contentChanges.length, 1, 'of the entry span alone'); + const text = t.doc.getText(); + assert.ok(text.includes('"Prototype": "y = h(u,v)"'), 'the entry was renamed'); + assert.strictEqual(catalogPartOf(text), catalogPartOf(t.original), 'and the catalog is as read'); + assert.strictEqual( + catalogPartOf(text), + catalogPartOf(coreSaved(enc(t.original), TEXT_FIXTURE, [step]).text), + 'as core’s own save leaves it', + ); + } finally { + await t.close(); + } + }); + + test('a rename the Prototype cannot follow is refused with core’s reason, and writes nothing', async () => { + const t = await openText('fn_rename_text_refused.sldd', (text) => + replacedOnce(text, [['"Prototype": "y = f(u,v)"', '"Prototype": "y = step(u,v)"']]), + ); + try { + const posts = await t.rename(ONE_RENAME[0]); + const refusals = refusalsIn(posts); + assert.strictEqual(refusals.length, 1, `one validationError (${JSON.stringify(posts.map((m) => m.type))})`); + const reason = coreRefusal(enc(t.original), TEXT_FIXTURE, ONE_RENAME[0]); + assert.ok(reason.startsWith("'f' cannot be renamed: its Prototype 'y = step(u,v)'"), reason); + assert.strictEqual(refusals[0].reason, reason, 'carrying core’s own reason'); + assert.strictEqual(t.changes.length, 0, 'no write'); + assert.strictEqual(t.doc.getText(), t.original); + assert.strictEqual(t.doc.isDirty, false); + } finally { + await t.close(); + } + }); + + test('undo is one step: the inverse of the one write brings back the original bytes', async () => { + const t = await openText('fn_rename_text_undo.sldd'); + try { + assert.deepStrictEqual(problemsIn(await t.rename(ONE_RENAME[0])), [], 'the rename was accepted'); + const core = coreSaved(enc(t.original), TEXT_FIXTURE, ONE_RENAME); + assert.strictEqual(catalogPartOf(t.doc.getText()), catalogPartOf(core.text), 'the write carried the catalog'); + assert.strictEqual(t.changes.length, 1, 'in one write, so one step on the undo stack'); + + // Each change's range is in the text before the write; in the text after it, a range + // starts where it did plus what the changes before it grew or shrank by. + const written = [...t.changes[0].contentChanges].sort((a, b) => a.rangeOffset - b.rangeOffset); + const inverse = new vscode.WorkspaceEdit(); + let shift = 0; + for (const c of written) { + const start = c.rangeOffset + shift; + inverse.replace( + t.uri, + new vscode.Range(t.doc.positionAt(start), t.doc.positionAt(start + c.text.length)), + t.original.slice(c.rangeOffset, c.rangeOffset + c.rangeLength), + ); + shift += c.text.length - c.rangeLength; + } + assert.ok(await vscode.workspace.applyEdit(inverse), 'the undo applied'); + await waitFor('the undo to land', () => t.changes.length === 2); + assert.strictEqual(t.doc.getText(), t.original, 'the document is the bytes it opened with'); + } finally { + await t.close(); + } + }); +}); + +// --- the binary editor ------------------------------------------------------------------- + +/** A working copy of the binary fixture (its members rewritten by `edit` first), open in the real provider. */ +async function openBinary(copyName: string, edit?: (zip: Record) => void) { + const zip = unzipSync(await vscode.workspace.fs.readFile(fixtureUri(BINARY_FIXTURE))); + edit?.(zip); + const bytes = zipSync(zip); + const uri = workingUri(copyName); + await vscode.workspace.fs.writeFile(uri, bytes); + const provider = new BinarySlddEditorProvider(ctx()); + const edits: vscode.CustomDocumentEditEvent[] = []; + const sub = provider.onDidChangeCustomDocument((e) => edits.push(e)); + const doc: any = await provider.openCustomDocument(uri, {} as vscode.CustomDocumentOpenContext, token()); + const view = fakePanel(); + await provider.resolveCustomEditor(doc, view.panel, token()); + // No waits in here: this provider answers `ready` and `edit` synchronously. + view.send({ type: 'ready' }); + assert.ok(view.posts.some((m) => m.type === 'setRows'), 'the first paint sent the whole table'); + + /** Send one rename; the posts it caused. */ + const rename = (step: Step): any[] => { + const row = rowOf(view.posts, step); + const mark = view.posts.length; + view.send(renameMessage(row, step.to)); + return view.posts.slice(mark); + }; + + /** Save, and read back the members the file now holds. */ + const save = async (): Promise> => { + await provider.saveCustomDocument(doc, token()); + return unzipSync(await vscode.workspace.fs.readFile(uri)); + }; + + const close = async () => { + view.close(); + sub.dispose(); + doc.dispose(); + await vscode.workspace.fs.delete(uri); + }; + return { doc, bytes, original: zip, edits, rename, save, close }; +} + +/** The catalog member core's own save writes after `steps`. */ +const coreSavedMember = (bytes: Uint8Array, steps: Step[]): string => { + const out = coreSaved(bytes, BINARY_FIXTURE, steps); + assert.strictEqual(out.kind, 'binary'); + return dec(unzipSync(out.bytes)[SC_PART_XML]); +}; + +suite('A service function rename through the binary editor carries into the catalog', () => { + suiteSetup(async () => { + await vscode.extensions.getExtension('mathworks.simulink-data-explorer')?.activate(); + }); + + test('one rename swaps the member for one with the function’s p_Name and p_FunctionPrototype, on one undo step', async () => { + const b = await openBinary('fn_rename_binary_one.sldd'); + try { + assert.deepStrictEqual(problemsIn(b.rename(ONE_RENAME[0])), [], 'the rename was accepted'); + assert.strictEqual(b.edits.length, 1, 'one edit'); + assert.ok(b.doc.chunkXml.includes('y = compute(u)'), 'the entry’s Prototype follows the Name'); + assert.strictEqual( + dec(b.doc.zipMeta[SC_PART_XML]), + replacedOnce(dec(b.original[SC_PART_XML]), [ + ['y = f(u)', 'y = compute(u)'], + ['f\n ', 'compute\n '], + ]), + 'the catalog moves at the function’s two sites: the data interface named f stays', + ); + + const saved = await b.save(); + assert.ok(dec(saved[CHUNK_XML]).includes('y = compute(u)'), 'the saved chunk holds the rename'); + assert.strictEqual( + dec(saved[SC_PART_XML]), + coreSavedMember(b.bytes, ONE_RENAME), + 'the saved catalog is the one core’s own save writes', + ); + } finally { + await b.close(); + } + }); + + for (const [label, steps] of SEQUENCES) { + test(`writes the catalog core’s own save writes: ${label}`, async () => { + const b = await openBinary(`fn_rename_binary_${label.replace(/\W+/g, '_')}.sldd`); + try { + for (const step of steps) { + assert.deepStrictEqual(problemsIn(b.rename(step)), [], `${step.fn ?? step.entry} -> ${step.to} was accepted`); + } + assert.strictEqual(b.edits.length, steps.length, 'one edit per rename'); + const saved = await b.save(); + assert.strictEqual( + dec(saved[SC_PART_XML]), + coreSavedMember(b.bytes, steps), + 'the saved catalog is the one core’s own save writes', + ); + } finally { + await b.close(); + } + }); + } + + test('a function the catalog does not list leaves the member untouched', async () => { + // The catalog stops listing `g`: drop its function element, so `g` is in the chunk alone. + const b = await openBinary('fn_rename_binary_unlisted.sldd', (zip) => { + const xml = dec(zip[SC_PART_XML]); + const listed = xml.match(/\n *]*uuid="fe-g">[\s\S]*?<\/p_FunctionElements>/g) ?? []; + assert.strictEqual(listed.length, 1, 'the fixture’s catalog lists g once'); + zip[SC_PART_XML] = enc(xml.replace(listed[0], '')); + }); + const step: Step = { entry: 'ServiceInterface', fn: 'g', to: 'h' }; + try { + const member = b.doc.zipMeta[SC_PART_XML]; + assert.deepStrictEqual(problemsIn(b.rename(step)), [], 'the rename was accepted'); + assert.strictEqual(b.edits.length, 1, 'one edit'); + assert.ok(b.doc.chunkXml.includes('[a,b] = h(g)'), 'the entry was renamed'); + assert.strictEqual(b.doc.zipMeta[SC_PART_XML], member, 'and the member is the one read, unswapped'); + const saved = await b.save(); + assert.deepStrictEqual(saved[SC_PART_XML], b.original[SC_PART_XML], 'so the save writes it as read'); + assert.strictEqual(dec(saved[SC_PART_XML]), coreSavedMember(b.bytes, [step]), 'as core’s own save leaves it'); + } finally { + await b.close(); + } + }); + + test('a rename the Prototype cannot follow is refused with core’s reason, and writes nothing', async () => { + const b = await openBinary('fn_rename_binary_refused.sldd', (zip) => { + zip[CHUNK_XML] = enc(replacedOnce(dec(zip[CHUNK_XML]), [['>y = f(u)<', '>y = step(u)<']])); + }); + try { + const chunk = b.doc.chunkXml; + const member = b.doc.zipMeta[SC_PART_XML]; + const posts = b.rename(ONE_RENAME[0]); + const refusals = refusalsIn(posts); + assert.strictEqual(refusals.length, 1, `one validationError (${JSON.stringify(posts.map((m) => m.type))})`); + const reason = coreRefusal(b.bytes, BINARY_FIXTURE, ONE_RENAME[0]); + assert.ok(reason.startsWith("'f' cannot be renamed: its Prototype 'y = step(u)'"), reason); + assert.strictEqual(refusals[0].reason, reason, 'carrying core’s own reason'); + assert.strictEqual(b.edits.length, 0, 'no edit'); + assert.strictEqual(b.doc.chunkXml, chunk, 'the chunk is as read'); + assert.strictEqual(b.doc.zipMeta[SC_PART_XML], member, 'and so is the member'); + } finally { + await b.close(); + } + }); + + test('undo is one step, and puts the member back byte for byte', async () => { + const b = await openBinary('fn_rename_binary_undo.sldd'); + try { + const chunk = b.doc.chunkXml; + const member = b.doc.zipMeta[SC_PART_XML]; + assert.deepStrictEqual(problemsIn(b.rename(ONE_RENAME[0])), [], 'the rename was accepted'); + assert.strictEqual(b.edits.length, 1, 'one edit, so one step on the undo stack'); + assert.strictEqual( + dec(b.doc.zipMeta[SC_PART_XML]), + coreSavedMember(b.bytes, ONE_RENAME), + 'the edit swapped in core’s catalog', + ); + + b.edits[0].undo(); + assert.strictEqual(b.doc.chunkXml, chunk, 'undo restored the chunk'); + assert.strictEqual(b.doc.zipMeta[SC_PART_XML], member, 'and the member read from the file'); + const saved = await b.save(); + assert.deepStrictEqual(saved[SC_PART_XML], b.original[SC_PART_XML], 'which a save writes byte for byte'); + } finally { + await b.close(); + } + }); +});