Fix #152 (#239)

antoineatstariongroup · web-flow · commit 57947a63fcb1 · 2026-05-18T09:41:17.000+02:00
* Fix #152 * Correct call to operation directly * claude.md update * update extension-impl.md
diff --git a/.claude/team-templates/extension-impl.md b/.claude/team-templates/extension-impl.md
@@ -187,6 +187,27 @@ hand-reformat of `FeatureExtensions.cs`. All four roles must enforce these.
 - **Use camelCase derived properties when in scope and implemented**: prefer
   `subject.fooBar` over re-deriving when that derived property is itself
   implemented.
+- **Invoke operations and derived properties via the POCO instance member,
+  not the static `Compute*` extension** — e.g.
+  `subject.IsDistinguishableFrom(other)`, `subject.qualifiedName`,
+  `subject.NamingFeature()`, NOT
+  `MembershipExtensions.ComputeIsDistinguishableFromOperation(subject, other)`,
+  `ElementExtensions.ComputeQualifiedName(subject)`,
+  `FeatureExtensions.ComputeNamingFeatureOperation(subject)`. The POCO's
+  instance member dispatches virtually and honors any subclass
+  **redefinition** of the operation/derived property; calling the static
+  extension directly bypasses dispatch and silently skips overrides — a
+  real defect class, easy to introduce and hard to spot.
+  **Exception (oclAsType pattern):** when the OCL itself uses
+  `self.oclAsType(SuperType).method()`, the C# translation MUST use the
+  static-extension-method form to BYPASS dispatch and target the SuperType's
+  body. Two precedents:
+  - `Usage::namingFeature()` → `FeatureExtensions.ComputeNamingFeatureOperation(usage)`
+    (would otherwise recurse into the Usage override).
+  - `OwningMembership::path()` → `RelationshipExtensions.ComputeRedefinedPathOperation(owningMembership)`
+    (same shape, OwningMembership upcast to Relationship).
+  Without an explicit `oclAsType(...)` in the source OCL, default to the
+  POCO instance call.
 - **Do not change the namespace, using directives, or method signatures.**
 
 ---
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -159,3 +159,4 @@ Auto-generated DTOs use structured namespaces reflecting the KerML/SysML package
 - Use 'NotSupportedException' (not 'NotImplementedException') for placeholder/stub methods that require manual implementation
 - Prefer C# property patterns ('x is IType { Prop: value }') over declared-variable-plus-predicate form ('x is IType name && name.Prop == value') when the narrowed variable is only consulted once; the property-pattern form is more concise and intent-revealing
 - Surround every braced block (`if`, `else if`, `while`, `for`, `foreach`, `switch`, `using`, `try`/`catch`/`finally`, `lock`, `do…while`, anonymous `{ }`) with a blank line on both sides — the rule does NOT apply at the very start/end of a method body, nor between a `}` and a continuation keyword (`else`, `catch`, `finally`, `while` of `do…while`) that belongs to the same control flow
+- When invoking an operation or derived property on a POCO from inside an extension method, call the POCO's instance member (e.g. `subject.IsDistinguishableFrom(other)`, `subject.qualifiedName`), NOT the static `ComputeXxxOperation` / `ComputeXxx` extension method. Virtual dispatch on the POCO honors operation/property REDEFINITION in subclass POCOs; calling the static extension directly bypasses dispatch and silently skips overrides. The static-extension form is reserved EXCLUSIVELY for the C# translation of OCL `self.oclAsType(SuperType).method()` — an explicit upcast that mandates targeting the SuperType's body (e.g. `Usage::namingFeature()` → `FeatureExtensions.ComputeNamingFeatureOperation(usage)`; `OwningMembership::path()` → `RelationshipExtensions.ComputeRedefinedPathOperation(owningMembership)`)
diff --git a/SysML2.NET.Tests/Extend/MembershipExtensionsTestFixture.cs b/SysML2.NET.Tests/Extend/MembershipExtensionsTestFixture.cs
@@ -21,20 +21,108 @@
 namespace SysML2.NET.Tests.Extend
 {
     using System;
-    
+
     using NUnit.Framework;
-    
+
     using SysML2.NET.Core.POCO.Root.Namespaces;
+    using SysML2.NET.Core.POCO.Systems.DefinitionAndUsage;
 
     [TestFixture]
     public class MembershipExtensionsTestFixture
     {
         [Test]
-        public void ComputeMemberElementId_ThrowsNotSupportedException()
+        public void VerifyComputeMemberElementId()
         {
-            Assert.That(() => ((IMembership)null).ComputeMemberElementId(), Throws.TypeOf<NotSupportedException>());
+            Assert.That(() => ((IMembership)null).ComputeMemberElementId(), Throws.TypeOf<ArgumentNullException>());
+
+            var membership = new Membership();
+            var element = new Definition { ElementId = "test-element-id-42" };
+            membership.MemberElement = element;
+
+            Assert.That(membership.ComputeMemberElementId(), Is.EqualTo("test-element-id-42"));
+        }
+
+        [Test]
+        public void VerifyComputeIsDistinguishableFromOperation()
+        {
+            // Null guards
+            Assert.That(() => ((IMembership)null).ComputeIsDistinguishableFromOperation(new Membership()), Throws.TypeOf<ArgumentNullException>());
+
+            Assert.That(() => new Membership().ComputeIsDistinguishableFromOperation(null), Throws.TypeOf<ArgumentNullException>());
+
+            // Clause C: incompatible metaclasses — types do not conform to each other → true
+            var subjectIncompat = new Membership { MemberShortName = "A", MemberName = "A" };
+            subjectIncompat.MemberElement = new Definition();
+
+            var otherIncompat = new Membership { MemberShortName = "A", MemberName = "A" };
+            otherIncompat.MemberElement = new Namespace();
+
+            Assert.That(subjectIncompat.ComputeIsDistinguishableFromOperation(otherIncompat), Is.True);
+
+            // Clause C edge case: null MemberElement on subject — no conformance possible → true
+            var subjectNullElement = new Membership { MemberShortName = "A", MemberName = "A" };
+            subjectNullElement.MemberElement = null;
+
+            var otherWithElement = new Membership { MemberShortName = "A", MemberName = "A" };
+            otherWithElement.MemberElement = new Definition();
+
+            Assert.That(subjectNullElement.ComputeIsDistinguishableFromOperation(otherWithElement), Is.True);
+
+            // Clause C edge case: null MemberElement on other — no conformance possible → true
+            var subjectWithElement = new Membership { MemberShortName = "A", MemberName = "A" };
+            subjectWithElement.MemberElement = new Definition();
+
+            var otherNullElement = new Membership { MemberShortName = "A", MemberName = "A" };
+            otherNullElement.MemberElement = null;
+
+            Assert.That(subjectWithElement.ComputeIsDistinguishableFromOperation(otherNullElement), Is.True);
+
+            // Clause B: same metaclass, all four cross-comparisons differ → true
+            var subjectClauseB = new Membership { MemberShortName = "A", MemberName = "B" };
+            subjectClauseB.MemberElement = new Definition();
+
+            var otherClauseB = new Membership { MemberShortName = "X", MemberName = "Y" };
+            otherClauseB.MemberElement = new Definition();
+
+            Assert.That(subjectClauseB.ComputeIsDistinguishableFromOperation(otherClauseB), Is.True);
+
+            // Clause A: both MemberShortName and MemberName null on subject → true (null name-part wins)
+            var subjectClauseA = new Membership { MemberShortName = null, MemberName = null };
+            subjectClauseA.MemberElement = new Definition();
+
+            var otherClauseA = new Membership { MemberShortName = "X", MemberName = "Y" };
+            otherClauseA.MemberElement = new Definition();
+
+            Assert.That(subjectClauseA.ComputeIsDistinguishableFromOperation(otherClauseA), Is.True);
+
+            // Indistinguishable: same metaclass + MemberShortName clash → false
+            var subjectShortClash = new Membership { MemberShortName = "A", MemberName = "B" };
+            subjectShortClash.MemberElement = new Definition();
+
+            var otherShortClash = new Membership { MemberShortName = "A", MemberName = "Z" };
+            otherShortClash.MemberElement = new Definition();
+
+            Assert.That(subjectShortClash.ComputeIsDistinguishableFromOperation(otherShortClash), Is.False);
+
+            // Indistinguishable: subject.MemberShortName matches other.MemberName → false
+            var subjectCrossClash = new Membership { MemberShortName = "A", MemberName = "B" };
+            subjectCrossClash.MemberElement = new Definition();
+
+            var otherCrossClash = new Membership { MemberShortName = null, MemberName = "A" };
+            otherCrossClash.MemberElement = new Definition();
+
+            Assert.That(subjectCrossClash.ComputeIsDistinguishableFromOperation(otherCrossClash), Is.False);
+
+            // Half-distinguishable: NamePart1 fires but NamePart2 fails (MemberName matches) → false
+            var subjectHalf = new Membership { MemberShortName = "A", MemberName = "B" };
+            subjectHalf.MemberElement = new Definition();
+
+            var otherHalf = new Membership { MemberShortName = "X", MemberName = "B" };
+            otherHalf.MemberElement = new Definition();
+
+            Assert.That(subjectHalf.ComputeIsDistinguishableFromOperation(otherHalf), Is.False);
         }
-        
+
         [Test]
         public void ComputeMembershipOwningNamespace_ThrowsNotSupportedException()
         {
diff --git a/SysML2.NET.Tests/Extend/NamespaceExtensionsTestFixture.cs b/SysML2.NET.Tests/Extend/NamespaceExtensionsTestFixture.cs
@@ -240,7 +240,28 @@ public void VerifyComputeImportedMembershipsOperation()
             var ownedMembership = new OwningMembership { Visibility = VisibilityKind.Public };
             namespaceElement.AssignOwnership(ownedMembership, collidingElement);
 
+            // Clause B (cross-comparisons): import and owned share both metaclass (Definition)
+            // and MemberName ("imported"), so they are NOT distinguishable -> import excluded.
             Assert.That(namespaceElement.ComputeImportedMembershipsOperation([]), Has.Count.EqualTo(0));
+
+            // Clause C (metaclass non-conformance): wire a second importedNamespace whose
+            // owned member is a Namespace (NOT a Definition) named "imported". It collides on
+            // MemberName with the owned Definition above, but the metaclasses are unrelated
+            // (Namespace vs Definition — neither IsAssignableFrom the other), so per
+            // Membership::isDistinguishableFrom Clause C the pair IS distinguishable, and the
+            // import must surface. The previous partial helper omitted Clause C and would have
+            // wrongly excluded this import; this assertion locks in the spec-correct behavior.
+            var crossMetaclassNamespace = new Namespace();
+            var crossMetaclassElement = new Namespace { DeclaredName = "imported" };
+            var crossMetaclassMembership = new OwningMembership { Visibility = VisibilityKind.Public };
+            crossMetaclassNamespace.AssignOwnership(crossMetaclassMembership, crossMetaclassElement);
+
+            var crossMetaclassImport = new NamespaceImport { ImportedNamespace = crossMetaclassNamespace };
+            namespaceElement.AssignOwnership(crossMetaclassImport);
+
+            Assert.That(
+                namespaceElement.ComputeImportedMembershipsOperation([]),
+                Is.EquivalentTo([crossMetaclassMembership]));
         }
 
         [Test]
diff --git a/SysML2.NET/Extend/MembershipExtensions.cs b/SysML2.NET/Extend/MembershipExtensions.cs
@@ -42,10 +42,11 @@ internal static class MembershipExtensions
         /// <returns>
         /// the computed result
         /// </returns>
-        [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
         internal static string ComputeMemberElementId(this IMembership membershipSubject)
         {
-            throw new NotSupportedException("Create a GitHub issue when this method is required");
+            return membershipSubject == null
+                ? throw new ArgumentNullException(nameof(membershipSubject))
+                : membershipSubject.MemberElement.ElementId;
         }
 
         /// <summary>
@@ -78,10 +79,48 @@ internal static INamespace ComputeMembershipOwningNamespace(this IMembership mem
         /// <returns>
         /// The expected <see cref="bool" />
         /// </returns>
-        [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
         internal static bool ComputeIsDistinguishableFromOperation(this IMembership membershipSubject, IMembership other)
         {
-            throw new NotSupportedException("Create a GitHub issue when this method is required");
+            if (membershipSubject == null)
+            {
+                throw new ArgumentNullException(nameof(membershipSubject));
+            }
+
+            if (other == null)
+            {
+                throw new ArgumentNullException(nameof(other));
+            }
+
+            // Clause C: metaclass incompatibility.
+            // OCL: not (memberElement.oclKindOf(other.memberElement.oclType())
+            //           or other.memberElement.oclKindOf(memberElement.oclType()))
+            // De Morgan: !A && !B. A null memberElement on either side trips this
+            // (no conformance is possible).
+            var thisType = membershipSubject.MemberElement?.GetType();
+            var otherType = other.MemberElement?.GetType();
+
+            if (thisType == null || otherType == null
+                || (!otherType.IsAssignableFrom(thisType)
+                    && !thisType.IsAssignableFrom(otherType)))
+            {
+                return true;
+            }
+
+            // NamePart1 (OCL spells it shortMemberName — known XMI typo, real
+            // attribute is MemberShortName):
+            //   memberShortName = null
+            //   OR (memberShortName != other.memberShortName
+            //       AND memberShortName != other.memberName)
+            var shortNamePart = string.IsNullOrWhiteSpace(membershipSubject.MemberShortName)
+                || (membershipSubject.MemberShortName != other.MemberShortName
+                    && membershipSubject.MemberShortName != other.MemberName);
+
+            // NamePart2: same shape, MemberName variant.
+            var namePart = string.IsNullOrWhiteSpace(membershipSubject.MemberName)
+                || (membershipSubject.MemberName != other.MemberShortName
+                    && membershipSubject.MemberName != other.MemberName);
+
+            return shortNamePart && namePart;
         }
     }
 }
diff --git a/SysML2.NET/Extend/NamespaceExtensions.cs b/SysML2.NET/Extend/NamespaceExtensions.cs
@@ -368,49 +368,14 @@ internal static List<IMembership> ComputeImportedMembershipsOperation(this IName
 
             var ownedMemberships = namespaceSubject.ownedMembership;
 
-            return 
+            return
             [
                 ..importedMemberships.Where(import =>
-                ownedMemberships.All(owned => IsDistinguishableMembership(import, owned))
-                && importedMemberships.All(other => other == import || IsDistinguishableMembership(import, other)))
+                ownedMemberships.All(import.IsDistinguishableFrom)
+                && importedMemberships.All(other => other == import || import.IsDistinguishableFrom(other)))
             ];
         }
 
-        /// <summary>
-        /// Determines whether <paramref name="left"/> is distinguishable from <paramref name="right"/>
-        /// according to the default OCL body of <c>Membership::isDistinguishableFrom</c>.
-        /// </summary>
-        /// <remarks>
-        /// OCL (KerML XMI):
-        /// <code>
-        /// memberShortName = null and memberName = null or
-        /// (memberShortName &lt;&gt; other.memberShortName and memberShortName &lt;&gt; other.memberName and
-        ///  memberName &lt;&gt; other.memberShortName and memberName &lt;&gt; other.memberName)
-        /// </code>
-        /// </remarks>
-        /// <param name="left">
-        /// The <see cref="IMembership" /> on whose perspective distinguishability is evaluated.
-        /// </param>
-        /// <param name="right">
-        /// The <see cref="IMembership" /> being compared against.
-        /// </param>
-        /// <returns>
-        /// <see langword="true"/> when <paramref name="left"/> can be distinguished from <paramref name="right"/>
-        /// per the default Membership distinguishability rule; otherwise <see langword="false"/>.
-        /// </returns>
-        private static bool IsDistinguishableMembership(IMembership left, IMembership right)
-        {
-            if (left.MemberShortName == null && left.MemberName == null)
-            {
-                return true;
-            }
-
-            return left.MemberShortName != right.MemberShortName
-                && left.MemberShortName != right.MemberName
-                && left.MemberName != right.MemberShortName
-                && left.MemberName != right.MemberName;
-        }
-
         /// <summary>
         /// If visibility is not null, return the Memberships of this Namespace with the given visibility,
         /// including ownedMemberships with the given visibility and Memberships imported with the given
