A FunctionElement rename carries into the interface dictionary's catalog, in both editors

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",

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<Bin
         // catalog, which names the entry as the file still does. Null for all but a rename of
         // a catalogued architectural entry.
         const catalogRename = catalogRenameOf(msg.columnId, msg.newValue, node, entry);
+        // And a service interface's function, which the catalog keeps a copy of under the
+        // interface's definition. Never non-null together with catalogRename.
+        const functionRename = catalogFunctionRenameOf(msg.columnId, msg.newValue, node, entry);
         // The undo half of the patch has to be snapshotted here: this is the only moment
         // the pre-edit state exists. (Through attempt, so an entry that will not
         // serialize costs this edit its fast undo and not the edit itself.)
@@ -1147,10 +1156,13 @@ export class BinarySlddEditorProvider implements vscode.CustomEditorProvider<Bin
         // The catalog is a zip member this document only passes through, so carrying the
         // rename into it is a member swap rather than a text edit — computed here, after the
         // validation gate, so a rejected edit does not pay for reading the part. Null unless
-        // a definition actually carries the name.
+        // the catalog spells what was renamed: a definition carrying the entry's name, or the
+        // function listed under its interface.
         const scPart = catalogRename
           ? scXmlRenamePatch(document.zipMeta, catalogRename.oldName, catalogRename.newName)
-          : null;
+          : functionRename
+            ? scXmlFunctionRenamePatch(document.zipMeta, functionRename)
+            : null;
         document.pushEdit('Edit ' + msg.columnId, before, after, patch, scPart ?? undefined);
         document.repaintOps([{ kind: 'replace', entryRowId, entry }]);
         if (msg.columnId === 'Name') webview.postMessage({ type: 'selectRows', rowIds: [node.id] });

src/host/SlddTextEditorProvider.ts

@@ -79,7 +79,7 @@ import { postOrReport } from './postPayload.js';
 import { wireNavigateSelect, drainNavigateSelect } from './navigate.js';
 import { answerMatrixRequest } from './matrixRequest.js';
 import { parsesAsJson } from './slddFormat.js';
-import { catalogRenameOf, scJsonRenameEdits } from './scRename.js';
+import { catalogFunctionRenameOf, catalogRenameOf, scJsonFunctionRenameEdits, scJsonRenameEdits } from './scRename.js';
 import { buildSectionRowId } from '../common/sectionRowId.js';
 import { basename } from '../common/pathUtil.js';
 import type { TableToHostMessage } from '../common/protocol.js';
@@ -965,6 +965,11 @@ export class SlddTextEditorProvider implements vscode.CustomTextEditorProvider {
         // before-the-mutation reason: the catalog names the entry as the document still
         // does. Null for all but a rename of a catalogued architectural entry.
         const catalogRename = catalogRenameOf(msg.columnId, msg.newValue, node, entry);
+        // And a service interface's function, which the catalog keeps a copy of: read here for
+        // the same reason, since the catalog spells the function as the document still does.
+        // Never non-null together with catalogRename (that one is the entry, this one a row
+        // under it).
+        const functionRename = catalogFunctionRenameOf(msg.columnId, msg.newValue, node, entry);
 
         // Mutated through entryOps so the session's node index follows a rename: an id is a
         // PATH, so renaming an entry rekeys it and everything under it. The wide re-parse used
@@ -1016,9 +1021,13 @@ export class SlddTextEditorProvider implements vscode.CustomTextEditorProvider {
         // saying two different things: the entry is no longer classified (a struct type
         // re-reads as a plain data interface) and the catalog defines an interface no entry
         // backs. Empty for every rename no definition carries, which is nearly all of them.
+        // A function of a service interface is spelled in the catalog too (its `p_Name`, and
+        // the `p_FunctionPrototype` that declares it), so its rename carries the same way.
         const scEdits = catalogRename
           ? scJsonRenameEdits(currentText, catalogRename.oldName, catalogRename.newName)
-          : [];
+          : functionRename
+            ? scJsonFunctionRenameEdits(currentText, functionRename)
+            : [];
 
         // Byte-scoped range replace: only the edited entry's span changes, so
         // sibling entries stay byte-identical. Native undo groups this edit.

src/host/scRename.ts

@@ -32,14 +32,27 @@
 // writes a bus of value types) would otherwise drag the catalog off the entry that
 // really carries that name.
 //
+// A service interface's FUNCTION is the one row below an entry that the catalog also spells:
+// it keeps its own copy of each function under the interface's definition (`p_Name`, and
+// `p_FunctionPrototype`, which declares the function by name). Core renames the data part's
+// Prototype with the Name and carries the rename into the catalog in its own save, which
+// neither provider calls; so `catalogFunctionRenameOf` is the second gate, and the two format
+// helpers below it are core's own function edits (`scFunctionRenameEdits`, what that save
+// applies) over the same two edit surfaces. The two gates never both fire: one needs the row
+// to BE the entry, the other needs it to be a function directly under it.
+//
 // Kept VS-Code-free, like the rest of the edit vocabulary, so it is testable without a
 // window.
 import {
   SC_PART_XML,
   applyScEdits,
+  scFunctionRenameEdits,
   scRenameEdits,
+  scanScJsonFunctions,
   scanScJsonText,
   scanScXml,
+  scanScXmlFunctions,
+  type CatalogFunctionRenames,
   type ScTextEdit,
 } from 'data-explorer-core';
 
@@ -70,6 +83,50 @@ export function catalogRenameOf(
   return { oldName: node.name, newName: newValue };
 }
 
+/**
+ * A rename of a service interface's function, which the catalog keeps its own copy of
+ * (`p_Name` / `p_FunctionPrototype` under the interface's definition): the interface as the
+ * file spells it, and the function's name in the file and next.
+ */
+export interface CatalogFunctionRename {
+  interfaceName: string;
+  oldName: string;
+  newName: string;
+}
+
+/**
+ * The function rename this cell edit carries into the catalog, or null if it carries none:
+ * a Name edit of a `Simulink.FunctionElement` that is a direct child of the top-level entry,
+ * which is the one function rename core records for its own save
+ * (`FunctionElementNode._functionRenamed`).
+ *
+ * MUST be called before the mutation, like `catalogRenameOf`: `node.name` is the name the
+ * file still spells. The interface's name needs no such care. Both providers carry an entry
+ * rename into the document the moment it is made, so the file spells the interface as
+ * `entry.name`.
+ *
+ * Not a lookup in the catalog either, for the same reason as `catalogRenameOf`: a function
+ * the catalog does not list is answered by the helpers finding no sites.
+ */
+export function catalogFunctionRenameOf(
+  columnId: string,
+  newValue: string,
+  node: { name: string; className?: string; parent?: unknown },
+  entry: { name: string },
+): CatalogFunctionRename | null {
+  if (columnId !== 'Name' || node.parent !== entry || node.className !== 'Simulink.FunctionElement') {
+    return null;
+  }
+  if (!node.name || node.name === newValue) {
+    return null;
+  }
+  return { interfaceName: entry.name, oldName: node.name, newName: newValue };
+}
+
+/** The one rename, in the shape core's edits take: interface, then function, as the file spells them. */
+const asFunctionRenames = (rename: CatalogFunctionRename): CatalogFunctionRenames =>
+  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<string, Uint8Array>,
+  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)) };
+}

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<vscode.Uri> {
-  const src = vscode.Uri.joinPath(ws(), '..', '..', '..', 'test', 'fixtures', 'arch_binary.sldd');
+async function workingCopy(name: string, fixture = 'arch_binary.sldd'): Promise<vscode.Uri> {
+  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]), [
+        ['<p_FunctionPrototype>y = f(u)</p_FunctionPrototype>', '<p_FunctionPrototype>y = compute(u)</p_FunctionPrototype>'],
+        ['<p_Name>f</p_Name>\n        </p_FunctionElements>', '<p_Name>compute</p_Name>\n        </p_FunctionElements>'],
+      ]),
+      '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<vscode.CustomDocument>[] = [];
+    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