diff --git a/src/datamodel/node/data/Simulink.EnumType.md b/src/datamodel/node/data/Simulink.EnumType.md index 43f0e43..7c977d5 100644 --- a/src/datamodel/node/data/Simulink.EnumType.md +++ b/src/datamodel/node/data/Simulink.EnumType.md @@ -146,6 +146,60 @@ This ensures add/remove stays consistent in the serialized format. | Add child | `removeChildNode(child)` | `restoreChildNode(child, index)` | | Remove child | `restoreChildNode(child, index)` | `removeChildNode(child)` | +### Removing the enumeral that IS the DefaultValue + +**Measured in MATLAB R2027a Prerelease 27.1.0.3393633 on 2026-09-17** +(`test/parity/matlab/probe_enum_removal.m`, `probe_enum_stale_open.m`). The two +dictionary flavours behave DIFFERENTLY from each other, and both differ from us. + +| | Design Data (`Simulink.data.dictionary.EnumTypeDefinition`) | Architectural Data (`Simulink.dictionary.archdata.EnumType`) | +|---|---|---| +| remove API | `removeEnumeral(obj, INDEX)` | `removeEnumeral(obj, NAME)` | +| add API | `appendEnumeral(obj, name, value, desc)` | `addEnumeral(obj, name, value, desc)` | +| removing the default enumeral | allowed; live `DefaultValue` keeps the **dangling** name | allowed; `DefaultValue` immediately reads the **first surviving** enumeral | +| is it cleared or re-pointed? | neither — genuinely still stored (it survives removing other enumerals too) | **cleared**: remove the first one too and the default follows to the next survivor | +| removing a NON-default enumeral | default unchanged | default unchanged | +| removing the LAST enumeral | allowed, down to zero enumerals | **refused** — `interface_dictionary:api:CannotDeleteLastEnumeral` | +| assigning a name that does not exist | rejected — `Simulink:DataType:DynamicEnum_InvalidDefaultValue` | rejected — same identifier | +| on save | the invalid `DefaultValue` key is **dropped from the file** | same: **dropped from the file** | +| on reload | `DefaultValue` is `''` → effective default is the first enumeral | reads the first enumeral | + +So the two converge on the same end state — **the effective default becomes the +first surviving enumeral** — and neither ever writes a `DefaultValue` naming an +enumeral the file does not contain. Design data merely defers the tidy-up until +save; arch data does it at once. Note the asymmetry both share: the name is +validated on ASSIGNMENT but not re-validated on REMOVAL. + +#### What we do instead — an open defect + +`removeChildNode` splices the child and never looks at `DefaultValue`, so after +deleting the enumeral the default named: + +- the Value column and PI row still show the **removed** name, because + `displayValue` returns `this.DefaultValue` whenever it is non-empty; +- **no enumeral carries the "current" icon** — `EnumValueNode.icon` compares + `parent.DefaultValue === this.name`, and nothing matches any more; +- the Value dropdown offers the surviving names while displaying one that is not + among its own options (`PropEnumValue.readOptions` reads the live children); +- and `_getSerializedProperties` writes `"DefaultValue": ""` into + the file, which is the combination MATLAB never produces. + +That last one was taken to MATLAB rather than assumed. Verified for BOTH .sldd +formats, each in its own MATLAB session (opening two dictionaries that define the +same enum in one session makes MATLAB reuse the first definition and contaminates +the second result): **MATLAB opens our file without error and silently repairs +it** — `DefaultValue` reads back `''`, so the effective default becomes the first +surviving enumeral, and a resave drops the key from the file. In a clean session +it does not even warn. + +So this is a **display and round-trip fidelity defect, not file corruption**: +nothing is lost or unloadable, but our UI shows a default MATLAB will not honour, +and the value the user chose changes underneath them when the dictionary is +reopened in MATLAB. The fix that matches both flavours is one line of intent — on +removing the enumeral that `DefaultValue` names, clear `DefaultValue` (and restore +it on undo), which makes our own `displayValue` fall back to `children[0]`, exactly +what MATLAB reports. **Not yet implemented.** + ## Validation mirrored in code - `setProperty('Value', )` writes `DefaultValue = name` unconditionally @@ -187,6 +241,19 @@ This ensures add/remove stays consistent in the serialized format. ## Open questions / deferred +- **Removing the enumeral that IS the DefaultValue**: measured in MATLAB and + written up above — we keep the dangling name, MATLAB moves the default to the + first survivor. Open defect, fix identified, not implemented. + +- **Removing the LAST enumeral**: `canRemoveChild()` returns true whenever there is + at least one child, so our UI will empty an enum out completely. Design data + allows that; **Architectural Data refuses it** + (`interface_dictionary:api:CannotDeleteLastEnumeral` — "Enumerations must have at + least one enumeration member"). We do not distinguish the two flavours here, so + for a derived enum we offer a deletion MATLAB would reject. Not yet handled; + what MATLAB does when it LOADS an arch-data enum with zero enumerals has not + been measured. + - **Enumeral Name editing**: Individual enumeral names can be renamed via the tree. If an enumeral is renamed to match the current DefaultValue, no update is needed. If the DefaultValue's enumeral is renamed, the DefaultValue becomes diff --git a/test/parity/matlab/probe_enum_removal.m b/test/parity/matlab/probe_enum_removal.m new file mode 100644 index 0000000..a28e0ef --- /dev/null +++ b/test/parity/matlab/probe_enum_removal.m @@ -0,0 +1,372 @@ +% Copyright 2026 The MathWorks, Inc. +% +% What does MATLAB do to an enum's DefaultValue when the enumeral it names is +% REMOVED? Our UI lets the user delete any enumeral (EnumTypeNode.canRemoveChild +% is unconditional) and never touches DefaultValue afterwards, so we can produce +% an enum whose default names an enumeral that is no longer there. This probe +% asks MATLAB what it does in the same situation, for BOTH dictionary flavours: +% +% Design Data Simulink.data.dictionary.EnumTypeDefinition +% Architectural Data Simulink.dictionary.archdata.EnumType +% +% The two APIs are NOT parallel, which round 1 of this probe established: +% design data: appendEnumeral(obj, name, value, desc), removeEnumeral(obj, INDEX) +% arch data: addEnumeral(obj, name), removeEnumeral(obj, NAME) +% Both carry a char DefaultValue, and both validate it on assignment +% ("Default value does not match any of the enumeration names"). +% +% Every step is wrapped, because the interesting outcomes include "MATLAB +% refuses" -- an error here is data, not a failure. Run under -batch and read +% the tagged lines. +function probe_enum_removal() + fprintf('PROBE_BEGIN\n'); + fprintf('VERSION %s\n', version); + root = tempname; mkdir(root); + fprintf('TMPROOT %s\n', root); + + run_step(@() dd_live(), '1. DESIGN DATA -- remove the default enumeral, live object'); + run_step(@() dd_discriminate(), '2. DESIGN DATA -- is the default CLEARED or RE-POINTED'); + run_step(@() dd_persist(root), '3. DESIGN DATA -- does it reach the file and reload'); + run_step(@() ad_live(), '4. ARCH DATA -- remove the default enumeral, live object'); + run_step(@() ad_discriminate(), '5. ARCH DATA -- is the default CLEARED or RE-POINTED'); + run_step(@() ad_persist(root), '6. ARCH DATA -- does it reach the file and reload'); + fprintf('\nPROBE_END\n'); +end + +% A step that throws is reported and the probe continues: "MATLAB refuses" is one +% of the answers being looked for, so it must not end the run. +function run_step(fn, title) + fprintf('\n===== %s =====\n', title); + try + fn(); + catch e + fprintf('STEP_ERROR %s | %s\n', e.identifier, e.message); + end +end + +% Print a value however it is shaped, short enough for one line. +function s = brief(v) + try + if ischar(v) + s = ['''' v '''']; + elseif isstring(v) && isscalar(v) + s = ['"' char(v) '"']; + elseif isempty(v) + s = sprintf('', class(v), mat2str(size(v))); + elseif isnumeric(v) || islogical(v) + s = mat2str(v); + else + s = sprintf('<%s %s>', class(v), mat2str(size(v))); + end + catch + s = ''; + end +end + +% The enumeral names an enum currently carries, in order. Works for a struct +% array (design data) and an object array (arch data). +function s = enumeral_names(en) + try + parts = cell(1, numel(en)); + for k = 1:numel(en) + parts{k} = char(en(k).Name); + end + s = ['[' strjoin(parts, ' ') ']']; + catch e + s = sprintf('', e.message); + end +end + +function i = idx_of(en, name) + i = 0; + for k = 1:numel(en) + if strcmp(char(en(k).Name), name) + i = k; return + end + end +end + +% A design-data enum carrying exactly `names`. A fresh EnumTypeDefinition already +% has one enumeral ('enum1', value 0, DefaultValue ''), so the placeholder is +% appended past and then dropped -- otherwise every experiment below would be +% reasoning about a list whose first entry it did not choose. +function ed = dd_new(names) + ed = Simulink.data.dictionary.EnumTypeDefinition; + for k = 1:numel(names) + appendEnumeral(ed, names{k}, int32(k - 1), ''); + end + removeEnumeral(ed, idx_of(ed.Enumerals, 'enum1')); +end + +% ---------------------------------------------------------------- 1 +% Three enumerals, DefaultValue on the MIDDLE one, then remove that one. The +% middle is deliberate: removing the first would be indistinguishable from the +% implicit "defaults to the first enumeral" rule taking over. +function dd_live() + ed = dd_new({'item1', 'item2', 'item3'}); + fprintf('DD.built %s default=%s\n', enumeral_names(ed.Enumerals), brief(ed.DefaultValue)); + ed.DefaultValue = 'item2'; + fprintf('DD.set_default %s\n', brief(ed.DefaultValue)); + + % THE QUESTION. Refuse, silently clear, re-point, or leave a dangling name? + try + removeEnumeral(ed, idx_of(ed.Enumerals, 'item2')); + fprintf('DD.remove_default ALLOWED\n'); + catch e + fprintf('DD.remove_default REFUSED %s | %s\n', e.identifier, e.message); + end + fprintf('DD.after %s default=%s\n', enumeral_names(ed.Enumerals), brief(ed.DefaultValue)); + + % Assignment-time validation, for contrast: if a bogus name is rejected here, + % then whatever removal did above is a deliberate choice and not an absence + % of checking. + try + ed.DefaultValue = 'nosuch'; + fprintf('DD.set_bogus ACCEPTED -> %s\n', brief(ed.DefaultValue)); + catch e + fprintf('DD.set_bogus REJECTED %s | %s\n', e.identifier, e.message); + end + + % Removing a NON-default enumeral must leave the default alone. + ed2 = dd_new({'item1', 'item2', 'item3'}); + ed2.DefaultValue = 'item2'; + removeEnumeral(ed2, idx_of(ed2.Enumerals, 'item3')); + fprintf('DD.remove_other %s default=%s\n', enumeral_names(ed2.Enumerals), brief(ed2.DefaultValue)); + + % And emptying the enum out entirely -- our canRemoveChild allows it. + ed3 = dd_new({'only'}); + try + removeEnumeral(ed3, 1); + fprintf('DD.empty_out ALLOWED n=%d default=%s\n', numel(ed3.Enumerals), brief(ed3.DefaultValue)); + catch e + fprintf('DD.empty_out REFUSED %s | %s\n', e.identifier, e.message); + end +end + +% ---------------------------------------------------------------- 2 +% If removing the default makes the default read as the FIRST enumeral, two very +% different mechanisms fit: the stored char was CLEARED and the getter falls back +% to the first (what our displayValue does), or it was actively RE-POINTED to the +% first (a real write). They differ the moment the first enumeral is removed too, +% so that is the discriminator. +function dd_discriminate() + ed = dd_new({'a', 'b', 'c'}); + ed.DefaultValue = 'b'; + removeEnumeral(ed, idx_of(ed.Enumerals, 'b')); + fprintf('DD.disc.step1 %s default=%s\n', enumeral_names(ed.Enumerals), brief(ed.DefaultValue)); + % Now drop the first. Cleared+fallback => default follows to 'c'. + % Re-pointed to 'a' => default is now dangling at 'a', or MATLAB refuses. + try + removeEnumeral(ed, idx_of(ed.Enumerals, 'a')); + fprintf('DD.disc.step2 %s default=%s\n', enumeral_names(ed.Enumerals), brief(ed.DefaultValue)); + catch e + fprintf('DD.disc.step2 REFUSED %s | %s\n', e.identifier, e.message); + end + + % Removing the LAST one while it is the default: does the default land on the + % first enumeral, or on the removed one's neighbour? + ed2 = dd_new({'a', 'b', 'c'}); + ed2.DefaultValue = 'c'; + removeEnumeral(ed2, idx_of(ed2.Enumerals, 'c')); + fprintf('DD.disc.last %s default=%s\n', enumeral_names(ed2.Enumerals), brief(ed2.DefaultValue)); + + % Is an explicitly-set default distinguishable from an unset one at all? If + % setting the first enumeral as the default stores 'a' rather than '', then + % the empty string is a real state and "cleared" is observable. + ed3 = dd_new({'a', 'b'}); + fprintf('DD.disc.unset %s\n', brief(ed3.DefaultValue)); + ed3.DefaultValue = 'a'; + fprintf('DD.disc.set_first %s\n', brief(ed3.DefaultValue)); + ed3.DefaultValue = ''; + fprintf('DD.disc.cleared %s\n', brief(ed3.DefaultValue)); +end + +% ---------------------------------------------------------------- 3 +% Does the outcome reach a FILE, and what does MATLAB read back? This is the half +% that matters to us: our serializer writes DefaultValue and the enumeral list +% independently, so a live-object rule that tidies the default is a rule our +% writer does not have. +function dd_persist(root) + f = fullfile(root, 'dd_enum.sldd'); + dd = Simulink.data.dictionary.create(f); + sec = getSection(dd, 'Design Data'); + ed = dd_new({'item1', 'item2', 'item3'}); + ed.DefaultValue = 'item2'; + addEntry(sec, 'MyEnum', ed); + saveChanges(dd); + close(dd); + fprintf('DD.persist.baseline_saved\n'); + + % What the file itself says, before any removal -- so the key name and + % spelling are on record for our writer to match. + txt = fileread(f); + tok = regexp(txt, '"DefaultValue"\s*:\s*"[^"]*"', 'match', 'once'); + fprintf('DD.persist.file_key %s\n', tok); + + % Now remove the default enumeral through the ENTRY, the way a user would. + dd = Simulink.data.dictionary.open(f); + en = getEntry(getSection(dd, 'Design Data'), 'MyEnum'); + v = getValue(en); + removeEnumeral(v, idx_of(v.Enumerals, 'item2')); + fprintf('DD.persist.in_memory %s default=%s\n', enumeral_names(v.Enumerals), brief(v.DefaultValue)); + try + setValue(en, v); + saveChanges(dd); + fprintf('DD.persist.saved_after_remove OK\n'); + catch e + fprintf('DD.persist.saved_after_remove ERROR %s | %s\n', e.identifier, e.message); + end + close(dd); + + txt2 = fileread(f); + tok2 = regexp(txt2, '"DefaultValue"\s*:\s*"[^"]*"', 'match', 'once'); + if isempty(tok2) + fprintf('DD.persist.file_key_after \n'); + else + fprintf('DD.persist.file_key_after %s\n', tok2); + end + + try + dd = Simulink.data.dictionary.open(f); + v2 = getValue(getEntry(getSection(dd, 'Design Data'), 'MyEnum')); + fprintf('DD.persist.reloaded %s default=%s\n', enumeral_names(v2.Enumerals), brief(v2.DefaultValue)); + close(dd); + catch e + fprintf('DD.persist.reload ERROR %s | %s\n', e.identifier, e.message); + end +end + +% An arch-data enum carrying exactly `names`. Same placeholder problem as the +% design-data builder: addEnumType seeds one enumeral called 'enum1'. +% addEnumeral wants a VALUE as well as a name -- `addEnumeral(et, name)` raises +% MATLAB:minrhs. Round 1 of this probe hid that behind a try/catch fallback and +% reported nothing, which is why the 4-argument form is spelled out here. +function et = ad_new(ad, typeName, names) + et = addEnumType(ad, typeName); + for k = 1:numel(names) + addEnumeral(et, names{k}, int32(k - 1), ''); + end + removeEnumeral(et, 'enum1'); +end + +% ---------------------------------------------------------------- 4 +% The same question in Architectural Data, where the enum is a HANDLE object and +% its enumerals are objects rather than a struct array -- so removal could +% plausibly behave differently from the design-data copy semantics. +function ad_live() + ad = Simulink.dictionary.archdata.create([tempname '.sldd']); + et = ad_new(ad, 'MyEnum', {'item1', 'item2', 'item3'}); + fprintf('AD.built %s default=%s\n', enumeral_names(et.Enumerals), brief(et.DefaultValue)); + et.DefaultValue = 'item2'; + fprintf('AD.set_default %s\n', brief(et.DefaultValue)); + + try + removeEnumeral(et, 'item2'); + fprintf('AD.remove_default ALLOWED\n'); + catch e + fprintf('AD.remove_default REFUSED %s | %s\n', e.identifier, e.message); + end + fprintf('AD.after %s default=%s\n', enumeral_names(et.Enumerals), brief(et.DefaultValue)); + try + fprintf('AD.after.getEnumeralNames %s\n', strjoin(reshape(cellstr(getEnumeralNames(et)), 1, []), ' ')); + catch e + fprintf('AD.after.getEnumeralNames ERROR %s\n', e.message); + end + + try + et.DefaultValue = 'nosuch'; + fprintf('AD.set_bogus ACCEPTED -> %s\n', brief(et.DefaultValue)); + catch e + fprintf('AD.set_bogus REJECTED %s | %s\n', e.identifier, e.message); + end + + et2 = ad_new(ad, 'OtherEnum', {'item1', 'item2', 'item3'}); + et2.DefaultValue = 'item2'; + removeEnumeral(et2, 'item3'); + fprintf('AD.remove_other %s default=%s\n', enumeral_names(et2.Enumerals), brief(et2.DefaultValue)); + + et3 = ad_new(ad, 'SoleEnum', {'only'}); + try + removeEnumeral(et3, 'only'); + fprintf('AD.empty_out ALLOWED n=%d default=%s\n', numel(et3.Enumerals), brief(et3.DefaultValue)); + catch e + fprintf('AD.empty_out REFUSED %s | %s\n', e.identifier, e.message); + end + try, discardChanges(ad); catch, end +end + +% ---------------------------------------------------------------- 5 +function ad_discriminate() + ad = Simulink.dictionary.archdata.create([tempname '.sldd']); + et = ad_new(ad, 'Disc', {'a', 'b', 'c'}); + et.DefaultValue = 'b'; + removeEnumeral(et, 'b'); + fprintf('AD.disc.step1 %s default=%s\n', enumeral_names(et.Enumerals), brief(et.DefaultValue)); + try + removeEnumeral(et, 'a'); + fprintf('AD.disc.step2 %s default=%s\n', enumeral_names(et.Enumerals), brief(et.DefaultValue)); + catch e + fprintf('AD.disc.step2 REFUSED %s | %s\n', e.identifier, e.message); + end + + et2 = ad_new(ad, 'DiscLast', {'a', 'b', 'c'}); + et2.DefaultValue = 'c'; + removeEnumeral(et2, 'c'); + fprintf('AD.disc.last %s default=%s\n', enumeral_names(et2.Enumerals), brief(et2.DefaultValue)); + + et3 = ad_new(ad, 'DiscClear', {'a', 'b'}); + fprintf('AD.disc.unset %s\n', brief(et3.DefaultValue)); + et3.DefaultValue = 'b'; + fprintf('AD.disc.set_second %s\n', brief(et3.DefaultValue)); + try + et3.DefaultValue = ''; + fprintf('AD.disc.cleared %s\n', brief(et3.DefaultValue)); + catch e + fprintf('AD.disc.clear REJECTED %s | %s\n', e.identifier, e.message); + end + try, discardChanges(ad); catch, end +end + +% ---------------------------------------------------------------- 6 +function ad_persist(root) + f = fullfile(root, 'ad_enum.sldd'); + ad = Simulink.dictionary.archdata.create(f); + et = ad_new(ad, 'MyEnum', {'item1', 'item2', 'item3'}); + et.DefaultValue = 'item2'; + save(ad); + fprintf('AD.persist.baseline_saved\n'); + txt = fileread(f); + tok = regexp(txt, '"DefaultValue"\s*:\s*"[^"]*"', 'match', 'once'); + if isempty(tok) + fprintf('AD.persist.file_key \n'); + else + fprintf('AD.persist.file_key %s\n', tok); + end + + removeEnumeral(et, 'item2'); + fprintf('AD.persist.in_memory %s default=%s\n', enumeral_names(et.Enumerals), brief(et.DefaultValue)); + try + save(ad); + fprintf('AD.persist.saved_after_remove OK\n'); + catch e + fprintf('AD.persist.saved_after_remove ERROR %s | %s\n', e.identifier, e.message); + end + try, close(ad); catch, end + + txt2 = fileread(f); + tok2 = regexp(txt2, '"DefaultValue"\s*:\s*"[^"]*"', 'match', 'once'); + if isempty(tok2) + fprintf('AD.persist.file_key_after \n'); + else + fprintf('AD.persist.file_key_after %s\n', tok2); + end + + try + ad2 = Simulink.dictionary.archdata.open(f); + et2 = getDataType(ad2, 'MyEnum'); + fprintf('AD.persist.reloaded %s default=%s\n', enumeral_names(et2.Enumerals), brief(et2.DefaultValue)); + close(ad2); + catch e + fprintf('AD.persist.reload ERROR %s | %s\n', e.identifier, e.message); + end +end diff --git a/test/parity/matlab/probe_enum_stale_open.m b/test/parity/matlab/probe_enum_stale_open.m new file mode 100644 index 0000000..a2e0f18 --- /dev/null +++ b/test/parity/matlab/probe_enum_stale_open.m @@ -0,0 +1,77 @@ +% Copyright 2026 The MathWorks, Inc. +% +% Open a .sldd whose enum DefaultValue names an enumeral the file does not +% contain, and report what MATLAB makes of it. Our writer can produce exactly +% that file (delete the default enumeral in the UI and save); MATLAB's own save +% never does, because it drops the key -- so the only way to learn whether such a +% file is loadable at all is to hand MATLAB one and watch. +% +% Usage: probe_enum_stale_open('/path/a.sldd', '/path/b.sldd', ...) +function probe_enum_stale_open(varargin) + fprintf('STALE_BEGIN\n'); + for k = 1:numel(varargin) + f = varargin{k}; + fprintf('\n--- %s\n', f); + try + dd = Simulink.data.dictionary.open(f); + catch e + fprintf('OPEN_ERROR %s | %s\n', e.identifier, e.message); + continue + end + fprintf('OPEN_OK\n'); + try + en = getEntry(getSection(dd, 'Design Data'), 'MyEnum'); + catch e + fprintf('ENTRY_ERROR %s | %s\n', e.identifier, e.message); + safeClose(dd); continue + end + % getValue is where the stored DefaultValue would be pushed through the + % property's own validation, so this is the line most likely to throw. + try + v = getValue(en); + catch e + fprintf('GETVALUE_ERROR %s | %s\n', e.identifier, e.message); + safeClose(dd); continue + end + names = cell(1, numel(v.Enumerals)); + for i = 1:numel(v.Enumerals) + names{i} = char(v.Enumerals(i).Name); + end + fprintf('ENUMERALS [%s]\n', strjoin(names, ' ')); + fprintf('DEFAULTVALUE ''%s''\n', v.DefaultValue); + fprintf('DANGLING %d\n', ~isempty(v.DefaultValue) && ~any(strcmp(names, v.DefaultValue))); + + % A resave: does MATLAB tidy the key the way it tidies its own? + try + setValue(en, v); + saveChanges(dd); + fprintf('RESAVE_OK\n'); + catch e + fprintf('RESAVE_ERROR %s | %s\n', e.identifier, e.message); + end + safeClose(dd); + try + txt = fileread(f); + tok = regexp(txt, '"DefaultValue"\s*:\s*"[^"]*"', 'match', 'once'); + if isempty(tok) + fprintf('FILE_KEY_AFTER_RESAVE \n'); + else + fprintf('FILE_KEY_AFTER_RESAVE %s\n', tok); + end + catch e + fprintf('FILE_READ_ERROR %s\n', e.message); + end + end + fprintf('\nSTALE_END\n'); +end + +function safeClose(dd) + try + discardChanges(dd); + catch + end + try + close(dd); + catch + end +end