fix(templates): override rows on a derived template are not second definitions in the collision check (ScadaBridge#119)
CollisionDetector counted a derived template's own override row (same name as a parent member, IsInherited=false) as a second definition alongside the parent's row, so AddAttribute/AddAlarm/AddScript for a fresh name failed with a false "Naming collision" once any override existed. The inheritance walk now keeps a (type, name) shadow set: a member defined at a nearer level of the chain shadows the same member further up (effective member set, by name), including overrides at mid levels. Real collisions (e.g. against a composed module) still report. Regression tests in TemplateEngine.Tests (detector + service, attribute/alarm/ script, mid-level override, positive control); 6 fail without the fix. The #118 importer's additions-before-overrides ordering is now unnecessary but harmless and was left in place. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
@@ -36,7 +36,11 @@ public static class CollisionDetector
|
||||
|
||||
// Collect direct (top-level) members. Inherited placeholder rows are skipped:
|
||||
// the inheritance walk below re-adds them under the parent's origin.
|
||||
CollectDirectMembers(template, prefix: null, originPrefix: template.Name, allMembers, skipInherited: true);
|
||||
// Names defined by a nearer level of the chain shadow the same (type, name) further
|
||||
// up: an own row sharing a parent member's name is an OVERRIDE of it (the effective
|
||||
// member set holds the name once), not a second definition (ScadaBridge#119).
|
||||
var shadowed = new HashSet<(string Type, string Name)>();
|
||||
CollectDirectMembers(template, prefix: null, originPrefix: template.Name, allMembers, skipInherited: true, shadowed);
|
||||
|
||||
// Collect members from composed modules recursively
|
||||
foreach (var composition in template.Compositions)
|
||||
@@ -53,7 +57,7 @@ public static class CollisionDetector
|
||||
}
|
||||
|
||||
// Collect inherited members (walk parent chain)
|
||||
CollectInheritedMembers(template, lookup, allMembers, moduleMemo, new HashSet<int> { template.Id });
|
||||
CollectInheritedMembers(template, lookup, allMembers, moduleMemo, new HashSet<int> { template.Id }, shadowed);
|
||||
|
||||
// Detect duplicates by canonical name
|
||||
var collisions = new List<string>();
|
||||
@@ -93,28 +97,43 @@ public static class CollisionDetector
|
||||
string? prefix,
|
||||
string originPrefix,
|
||||
List<ResolvedMember> members,
|
||||
bool skipInherited)
|
||||
bool skipInherited,
|
||||
HashSet<(string Type, string Name)>? shadowed = null)
|
||||
{
|
||||
// Rows shadowed by a nearer level are skipped; this level's own names are only
|
||||
// added to the shadow set afterwards so same-level duplicates stay visible.
|
||||
var levelKeys = new List<(string, string)>();
|
||||
bool Skip(string type, string name, bool inherited)
|
||||
{
|
||||
if (skipInherited && inherited) return true;
|
||||
if (shadowed != null && shadowed.Contains((type, name))) return true;
|
||||
levelKeys.Add((type, name));
|
||||
return false;
|
||||
}
|
||||
|
||||
foreach (var attr in template.Attributes)
|
||||
{
|
||||
if (skipInherited && attr.IsInherited) continue;
|
||||
if (Skip("Attribute", attr.Name, attr.IsInherited)) continue;
|
||||
var canonicalName = prefix == null ? attr.Name : $"{prefix}.{attr.Name}";
|
||||
members.Add(new ResolvedMember(canonicalName, "Attribute", originPrefix));
|
||||
}
|
||||
|
||||
foreach (var alarm in template.Alarms)
|
||||
{
|
||||
if (skipInherited && alarm.IsInherited) continue;
|
||||
if (Skip("Alarm", alarm.Name, alarm.IsInherited)) continue;
|
||||
var canonicalName = prefix == null ? alarm.Name : $"{prefix}.{alarm.Name}";
|
||||
members.Add(new ResolvedMember(canonicalName, "Alarm", originPrefix));
|
||||
}
|
||||
|
||||
foreach (var script in template.Scripts)
|
||||
{
|
||||
if (skipInherited && script.IsInherited) continue;
|
||||
if (Skip("Script", script.Name, script.IsInherited)) continue;
|
||||
var canonicalName = prefix == null ? script.Name : $"{prefix}.{script.Name}";
|
||||
members.Add(new ResolvedMember(canonicalName, "Script", originPrefix));
|
||||
}
|
||||
|
||||
if (shadowed != null)
|
||||
foreach (var k in levelKeys) shadowed.Add(k);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -234,7 +253,8 @@ public static class CollisionDetector
|
||||
Dictionary<int, Template> lookup,
|
||||
List<ResolvedMember> members,
|
||||
Dictionary<int, IReadOnlyList<RelativeMember>> moduleMemo,
|
||||
HashSet<int> visited)
|
||||
HashSet<int> visited,
|
||||
HashSet<(string Type, string Name)> shadowed)
|
||||
{
|
||||
if (!template.ParentTemplateId.HasValue)
|
||||
return;
|
||||
@@ -248,7 +268,7 @@ public static class CollisionDetector
|
||||
// Inherited direct members (no prefix). Skip the parent's OWN inherited
|
||||
// placeholders too: the next recursion adds the grandparent's real rows, so
|
||||
// counting the parent's copies would re-introduce the collision one level up.
|
||||
CollectDirectMembers(parent, prefix: null, $"parent '{parent.Name}'", members, skipInherited: true);
|
||||
CollectDirectMembers(parent, prefix: null, $"parent '{parent.Name}'", members, skipInherited: true, shadowed);
|
||||
|
||||
// Inherited composed modules
|
||||
foreach (var composition in parent.Compositions)
|
||||
@@ -265,6 +285,6 @@ public static class CollisionDetector
|
||||
}
|
||||
|
||||
// Continue up the inheritance chain
|
||||
CollectInheritedMembers(parent, lookup, members, moduleMemo, visited);
|
||||
CollectInheritedMembers(parent, lookup, members, moduleMemo, visited, shadowed);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user