diff --git a/.claude/team-templates/extension-impl.md b/.claude/team-templates/extension-impl.md index 1a2c80be..b3bb0f58 100644 --- 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 index 7f4b4880..22679c85 100644 --- 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 index a304b730..488c40a4 100644 --- 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()); + Assert.That(() => ((IMembership)null).ComputeMemberElementId(), Throws.TypeOf()); + + 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()); + + Assert.That(() => new Membership().ComputeIsDistinguishableFromOperation(null), Throws.TypeOf()); + + // 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 index 0886d1c7..c343f316 100644 --- 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 index c577aa9e..1f09a55a 100644 --- a/SysML2.NET/Extend/MembershipExtensions.cs +++ b/SysML2.NET/Extend/MembershipExtensions.cs @@ -42,10 +42,11 @@ internal static class MembershipExtensions /// /// the computed result /// - [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; } /// @@ -78,10 +79,48 @@ internal static INamespace ComputeMembershipOwningNamespace(this IMembership mem /// /// The expected /// - [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 index e4c86430..a549e83d 100644 --- a/SysML2.NET/Extend/NamespaceExtensions.cs +++ b/SysML2.NET/Extend/NamespaceExtensions.cs @@ -368,49 +368,14 @@ internal static List 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))) ]; } - /// - /// Determines whether is distinguishable from - /// according to the default OCL body of Membership::isDistinguishableFrom. - /// - /// - /// OCL (KerML XMI): - /// - /// memberShortName = null and memberName = null or - /// (memberShortName <> other.memberShortName and memberShortName <> other.memberName and - /// memberName <> other.memberShortName and memberName <> other.memberName) - /// - /// - /// - /// The on whose perspective distinguishability is evaluated. - /// - /// - /// The being compared against. - /// - /// - /// when can be distinguished from - /// per the default Membership distinguishability rule; otherwise . - /// - 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; - } - /// /// 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