From 14648778a696a9b7fa26c2921a03863d138f6dcf Mon Sep 17 00:00:00 2001 From: atheate Date: Thu, 13 Aug 2026 09:21:06 +0200 Subject: [PATCH 1/3] Fix #294: Validation of 07 Variant Confiugration) --- ...tualNotationBuilderGeneratorTestFixture.cs | 76 ++++++ .../RuleProcessor.CollectionProcessing.cs | 40 +++ .../RuleProcessor.ElementProcessing.cs | 34 ++- .../RuleProcessor.PatternHandlers.cs | 217 ++++++++++++++++- .../HandleBarHelpers/RulesHelper.cs | 8 +- ...iant Configuration - General Concept.sysml | 40 +++ ...nt Configuration - General Concept-a.sysml | 64 +++++ .../7b-Variant Configurations.sysml | 101 ++++++++ .../TextualNotationValidationTestFixture.cs | 5 +- .../ElementTextualNotationBuilder.cs | 6 + ...erationDefinitionTextualNotationBuilder.cs | 6 +- .../ExpressionTextualNotationBuilder.cs | 2 +- .../SharedTextualNotationBuilder.cs | 8 +- .../TypeTextualNotationBuilder.cs | 15 +- .../UsageTextualNotationBuilder.cs | 17 +- .../Writers/OperatorPrecedence.cs | 227 +++++++++++------- .../Writers/TextualNotationWriterContext.cs | 6 + 17 files changed, 772 insertions(+), 100 deletions(-) create mode 100644 SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a-Variant Configuration - General Concept.sysml create mode 100644 SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a1-Variant Configuration - General Concept-a.sysml create mode 100644 SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7b-Variant Configurations.sysml diff --git a/SysML2.NET.CodeGenerator.Tests/Generators/UmlHandleBarsGenerators/UmlCoreTextualNotationBuilderGeneratorTestFixture.cs b/SysML2.NET.CodeGenerator.Tests/Generators/UmlHandleBarsGenerators/UmlCoreTextualNotationBuilderGeneratorTestFixture.cs index ff7487e5..ace604d3 100644 --- a/SysML2.NET.CodeGenerator.Tests/Generators/UmlHandleBarsGenerators/UmlCoreTextualNotationBuilderGeneratorTestFixture.cs +++ b/SysML2.NET.CodeGenerator.Tests/Generators/UmlHandleBarsGenerators/UmlCoreTextualNotationBuilderGeneratorTestFixture.cs @@ -20,6 +20,7 @@ namespace SysML2.NET.CodeGenerator.Tests.Generators.UmlHandleBarsGenerators { + using System; using System.IO; using System.Linq; using System.Threading.Tasks; @@ -103,5 +104,80 @@ public async Task Verify_that_ExtentExpression_case_carries_synthesised_guard() "Synthesised guard for ExtentExpression must include the parsed `ownedRelationship += TypeReferenceMember` cursor predicate."); }); } + + /// + /// Regression for the selective nested-alternation flattening fix. A metaclass reachable only + /// THROUGH a nested alternation rule used to be captured by an earlier arm typed on one of its + /// supertypes, because the depth sort ranks a nested-rule arm by the RULE's declared target + /// (BehaviorUsageElement : Usage, shallow) rather than by the deepest metaclass reachable + /// through it (PerformActionUsage, deep) — and forces that arm last as default: when + /// the target IS the generating class. So BuildVariantUsageElement matched every + /// IPerformActionUsage against case IEventOccurrenceUsage (a supertype per + /// IPerformActionUsage : IActionUsage, IEventOccurrenceUsage) and emitted + /// variant event doX; instead of variant perform doX;. The fix hoists one + /// case arm per genuinely-shadowed class to the top of the switch, delegating to the nested + /// rule's builder. This test pins both known instances — VariantUsageElement and + /// OwnedRelatedElement (where IMultiplicity : IFeature was swallowed by + /// case IFeature before reaching NonFeatureElement). + /// + [Test] + public async Task Verify_that_shadowed_nested_alternation_targets_are_hoisted() + { + await this.umlCoreTextualNotationBuilderGenerator.GenerateAsync(GeneratorSetupFixture.XmiReaderResult, this.textualNotationSpecification, this.umlPocoDirectoryInfo); + + var generatedUsageBuilderPath = Path.Combine(this.umlPocoDirectoryInfo.FullName, "UsageTextualNotationBuilder.cs"); + var generatedElementBuilderPath = Path.Combine(this.umlPocoDirectoryInfo.FullName, "ElementTextualNotationBuilder.cs"); + + Assert.That(File.Exists(generatedUsageBuilderPath), Is.True, $"Expected generator to emit {generatedUsageBuilderPath}"); + Assert.That(File.Exists(generatedElementBuilderPath), Is.True, $"Expected generator to emit {generatedElementBuilderPath}"); + + var buildVariantUsageElement = ExtractMethodBody(await File.ReadAllTextAsync(generatedUsageBuilderPath), "BuildVariantUsageElement"); + var buildOwnedRelatedElement = ExtractMethodBody(await File.ReadAllTextAsync(generatedElementBuilderPath), "BuildOwnedRelatedElement"); + + var performActionArmIndex = buildVariantUsageElement.IndexOf("case SysML2.NET.Core.POCO.Systems.Actions.IPerformActionUsage pocoPerformActionUsage:", StringComparison.Ordinal); + var eventOccurrenceArmIndex = buildVariantUsageElement.IndexOf("case SysML2.NET.Core.POCO.Systems.Occurrences.IEventOccurrenceUsage pocoEventOccurrenceUsage:", StringComparison.Ordinal); + var multiplicityArmIndex = buildOwnedRelatedElement.IndexOf("case SysML2.NET.Core.POCO.Core.Types.IMultiplicity pocoMultiplicity:", StringComparison.Ordinal); + var featureArmIndex = buildOwnedRelatedElement.IndexOf("case SysML2.NET.Core.POCO.Core.Features.IFeature pocoFeature:", StringComparison.Ordinal); + + using (Assert.EnterMultipleScope()) + { + Assert.That(performActionArmIndex, Is.GreaterThanOrEqualTo(0), + "BuildVariantUsageElement must hoist an IPerformActionUsage arm — PerformActionUsage is only reachable through the nested BehaviorUsageElement rule."); + Assert.That(performActionArmIndex, Is.LessThan(eventOccurrenceArmIndex), + "The hoisted IPerformActionUsage arm must precede case IEventOccurrenceUsage, otherwise the supertype arm swallows it and `variant perform` renders as `variant event`."); + Assert.That(buildVariantUsageElement, Does.Contain("BuildBehaviorUsageElement(pocoPerformActionUsage, writerContext, stringBuilder);"), + "The hoisted arm must delegate to the nested rule's own builder so no builder is bypassed."); + Assert.That(buildVariantUsageElement, Does.Contain("case SysML2.NET.Core.POCO.Systems.States.IExhibitStateUsage pocoExhibitStateUsage:"), + "ExhibitStateUsage (IExhibitStateUsage : IStateUsage, IPerformActionUsage) is shadowed by the same IEventOccurrenceUsage arm and must be hoisted too."); + Assert.That(buildVariantUsageElement, Does.Contain("case SysML2.NET.Core.POCO.Systems.UseCases.IIncludeUseCaseUsage pocoIncludeUseCaseUsage:"), + "IncludeUseCaseUsage (IIncludeUseCaseUsage : IUseCaseUsage, IPerformActionUsage) is shadowed by the same IEventOccurrenceUsage arm and must be hoisted too."); + Assert.That(multiplicityArmIndex, Is.GreaterThanOrEqualTo(0), + "BuildOwnedRelatedElement must hoist an IMultiplicity arm — Multiplicity is only reachable through the nested NonFeatureElement rule."); + Assert.That(multiplicityArmIndex, Is.LessThan(featureArmIndex), + "The hoisted IMultiplicity arm must precede case IFeature — Multiplicity is a NonFeatureElement alternative but IMultiplicity : IFeature."); + Assert.That(buildOwnedRelatedElement, Does.Contain("BuildNonFeatureElement(pocoMultiplicity, writerContext, stringBuilder);"), + "The hoisted IMultiplicity arm must delegate to BuildNonFeatureElement, the rule that lists Multiplicity as an alternative."); + } + } + + /// + /// Extracts the source of a single generated builder method, so arm-ordering assertions anchor on + /// the method under test rather than on the first file-wide match of a case label. + /// + /// The full generated builder source + /// The name of the public static void Build… method to extract + /// The method's source, up to the start of the next method + private static string ExtractMethodBody(string generatedSource, string methodName) + { + var methodStartIndex = generatedSource.IndexOf($"public static void {methodName}(", StringComparison.Ordinal); + + Assert.That(methodStartIndex, Is.GreaterThanOrEqualTo(0), $"Expected the generated source to declare {methodName}"); + + var nextMethodIndex = generatedSource.IndexOf("public static void ", methodStartIndex + 1, StringComparison.Ordinal); + + return nextMethodIndex < 0 + ? generatedSource[methodStartIndex..] + : generatedSource[methodStartIndex..nextMethodIndex]; + } } } diff --git a/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.CollectionProcessing.cs b/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.CollectionProcessing.cs index 85a40e91..e2e51fcf 100644 --- a/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.CollectionProcessing.cs +++ b/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.CollectionProcessing.cs @@ -171,6 +171,46 @@ private void EmitCollectionNonTerminalLoop(EncodedTextWriter writer, IClass umlC writer.WriteSafeString(Environment.NewLine); } + /// + /// Emits the fall-through arm of a repeated group's per-element switch. When the group has + /// bare non-terminal alternatives (dispatcher rules over the same cursor, e.g. TypeBodyElement + /// in ( TypeBodyElement | ownedRelationship += ReturnFeatureMember )*) the arm delegates to + /// them; those rules advance the cursor themselves, so no Move() is emitted here per the + /// Golden Rule. With no dispatcher alternative the arm just advances, which is what terminates the + /// loop on an element the group cannot render. + /// + /// The to emit to + /// The rule's target + /// The group's bare (non-assignment) non-terminal alternatives + /// The cursor driving the repeated group + /// The current + private static void EmitCollectionGroupFallThrough(EncodedTextWriter writer, IClass umlClass, List dispatcherNonTerminals, string cursorVariableName, RuleGenerationContext ruleGenerationContext) + { + var dispatcherCalls = dispatcherNonTerminals + .Select(nonTerminal => + { + var referencedRule = ruleGenerationContext.FindRule(nonTerminal.Name); + + return referencedRule?.EffectiveTarget == null + ? null + : ResolveBuilderCall(umlClass, nonTerminal, referencedRule.EffectiveTarget, ruleGenerationContext); + }) + .Where(call => call != null) + .ToList(); + + if (dispatcherCalls.Count == 0) + { + writer.WriteSafeString($"{cursorVariableName}.Move();{Environment.NewLine}"); + + return; + } + + foreach (var dispatcherCall in dispatcherCalls) + { + writer.WriteSafeString($"{dispatcherCall}{Environment.NewLine}"); + } + } + /// /// Resolves the type condition for a collection while loop. /// diff --git a/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.ElementProcessing.cs b/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.ElementProcessing.cs index 1fe71838..19712ed8 100644 --- a/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.ElementProcessing.cs +++ b/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.ElementProcessing.cs @@ -144,6 +144,18 @@ internal void ProcessRuleElement(EncodedTextWriter writer, IClass umlClass, Rule .OfType() .ToList(); + // Alternatives of the repeated group that are a BARE non-terminal rather than a + // `+=` assignment (e.g. `TypeBodyElement` in + // `( TypeBodyElement | ownedRelationship += ReturnFeatureMember )*`). Such a rule is a + // per-item dispatcher over the SAME cursor that advances the cursor itself, so it + // becomes the loop's fall-through arm — without it the group's switch has no case for + // those elements and the trailing `Move()` silently discards them. + var groupDispatcherNonTerminals = groupElement.Alternatives + .SelectMany(alternative => alternative.Elements) + .OfType() + .Where(nonTerminal => nonTerminal.Container is not AssignmentElement) + .ToList(); + if (groupAssignments.Count > 0 && groupNonTerminals.Count == groupAssignments.Count) { var groupPropertyName = groupAssignments[0].Property; @@ -174,7 +186,18 @@ internal void ProcessRuleElement(EncodedTextWriter writer, IClass umlClass, Rule var groupOrderedElements = RuleQueryUtilities.OrderElementsByInheritance(groupNonTerminals, umlClass.Cache, ruleGenerationContext); - writer.WriteSafeString($"while ({groupCursorVarName}.Current != null){Environment.NewLine}"); + // A `*` group must not swallow an element that a FOLLOWING sibling consumes + // from the same cursor — `( … )* ( ownedRelationship += ResultExpressionMember )?` + // left the trailing optional facing an exhausted cursor and dropped the result + // expression. Exclude the next sibling's target type from the loop condition. + var groupWhileCondition = this.ResolveCollectionWhileTypeCondition(groupCursorVarName, umlClass, null, groupPropertyName, ruleGenerationContext); + + if (string.IsNullOrWhiteSpace(groupWhileCondition)) + { + groupWhileCondition = $"{groupCursorVarName}.Current != null"; + } + + writer.WriteSafeString($"while ({groupWhileCondition}){Environment.NewLine}"); writer.WriteSafeString($"{{{Environment.NewLine}"); writer.WriteSafeString($"switch ({groupCursorVarName}.Current){Environment.NewLine}"); writer.WriteSafeString($"{{{Environment.NewLine}"); @@ -192,11 +215,16 @@ internal void ProcessRuleElement(EncodedTextWriter writer, IClass umlClass, Rule ruleGenerationContext.CurrentVariableName = previousVariableName; ruleGenerationContext.CallerRule = previousCaller; - writer.WriteSafeString($"{Environment.NewLine}break;{Environment.NewLine}"); + // The case body built the element itself, so this `+=` consumption owns the + // cursor advance (Golden Rule) — the fall-through arm below must NOT repeat it. + writer.WriteSafeString($"{Environment.NewLine}{groupCursorVarName}.Move();{Environment.NewLine}break;{Environment.NewLine}"); } + writer.WriteSafeString($"default:{Environment.NewLine}"); + EmitCollectionGroupFallThrough(writer, umlClass, groupDispatcherNonTerminals, groupCursorVarName, ruleGenerationContext); + writer.WriteSafeString($"break;{Environment.NewLine}"); + writer.WriteSafeString($"}}{Environment.NewLine}"); - writer.WriteSafeString($"{groupCursorVarName}.Move();{Environment.NewLine}"); writer.WriteSafeString($"}}{Environment.NewLine}"); } else diff --git a/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.PatternHandlers.cs b/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.PatternHandlers.cs index d5ae8e67..40e86d69 100644 --- a/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.PatternHandlers.cs +++ b/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.PatternHandlers.cs @@ -23,6 +23,7 @@ namespace SysML2.NET.CodeGenerator.HandleBarHelpers using System; using System.Collections.Generic; using System.Linq; + using System.Text.RegularExpressions; using HandlebarsDotNet; @@ -34,6 +35,7 @@ namespace SysML2.NET.CodeGenerator.HandleBarHelpers using uml4net.CommonStructure; using uml4net.Extensions; using uml4net.StructuredClassifiers; + using uml4net.Values; /// /// Pattern detection and specialized code emission for grammar alternatives @@ -842,9 +844,28 @@ private void ProcessUnitypedAlternativesWithOneElement(EncodedTextWriter writer, } } + var hoistedElements = CollectShadowedNestedRuleTargets(mappedNonTerminalElements, whenGuards, defaultElement.RuleElement, umlClass.Cache, ruleGenerationContext); + writer.WriteSafeString($"switch({variableName}){Environment.NewLine}"); writer.WriteSafeString("{"); + foreach (var hoistedElement in hoistedElements) + { + var previousHoistedVariableName = ruleGenerationContext.CurrentVariableName; + var hoistedCaseVarName = $"poco{hoistedElement.UmlClass.Name}"; + writer.WriteSafeString($"case {hoistedElement.UmlClass.QueryFullyQualifiedTypeName()} {hoistedCaseVarName}:{Environment.NewLine}"); + ruleGenerationContext.CurrentVariableName = hoistedCaseVarName; + + var previousHoistedCaller = ruleGenerationContext.CallerRule; + ruleGenerationContext.CallerRule = hoistedElement.RuleElement; + + this.ProcessNonTerminalElement(writer, hoistedElement.UmlClass, hoistedElement.RuleElement, ruleGenerationContext); + + ruleGenerationContext.CallerRule = previousHoistedCaller; + ruleGenerationContext.CurrentVariableName = previousHoistedVariableName; + writer.WriteSafeString($"{Environment.NewLine}break;{Environment.NewLine}"); + } + foreach (var orderedNonTerminalElement in mappedNonTerminalElements) { var previousVariableName = ruleGenerationContext.CurrentVariableName; @@ -1015,6 +1036,7 @@ private void ProcessUnitypedAlternativesWithOneElement(EncodedTextWriter writer, } var properties = umlClass.QueryAllProperties(); + var orderedAssignments = OrderAssignmentsByImplicationConstraints(assignmentElements, umlClass); for (var alternativeIndex = 0; alternativeIndex < alternatives.Count; alternativeIndex++) { @@ -1023,7 +1045,7 @@ private void ProcessUnitypedAlternativesWithOneElement(EncodedTextWriter writer, writer.WriteSafeString("else "); } - var assignment = assignmentElements[alternativeIndex]; + var assignment = orderedAssignments[alternativeIndex]; var targetProperty = properties.Single(x => string.Equals(x.Name, assignment.Property)); var iterator = ruleGenerationContext.DefinedCursors.SingleOrDefault(x => x.ApplicableRuleElements.Contains(assignment)); @@ -1049,6 +1071,83 @@ private void ProcessUnitypedAlternativesWithOneElement(EncodedTextWriter writer, } } + /// + /// Matches an OCL constraint body of the shape <antecedent> implies <consequent> + /// where both operands are bare property names. + /// + private static readonly Regex SimpleImplicationConstraint = new(@"^\s*(\w+)\s+implies\s+(\w+)\s*$", RegexOptions.Compiled); + + /// + /// Reorders mutually-exclusive boolean ?= assignment alternatives so that, whenever the + /// rule's target class carries an OCL invariant of the shape A implies B over two of the + /// assigned properties, the implying property A is tested before the implied property + /// B. + /// + /// + /// A grammar alternation such as ( isAbstract ?= 'abstract' | isVariation ?= 'variation' )? + /// compiles to an if/else if chain in grammar order. When the model satisfies + /// isVariation implies isAbstract (SysML v2 validateDefinitionVariationIsAbstract / + /// validateUsageVariationIsAbstract) a variation always carries isAbstract too, so the + /// grammar-ordered chain would always take the abstract arm and the stronger variation + /// keyword would never be emitted. Testing the antecedent first keeps the chain exhaustive while + /// selecting the most specific keyword. Ordering is stable — alternatives not related by an + /// implication keep their grammar order. + /// + /// The single-element alternatives' s, in grammar order + /// The rule's target , whose invariants (own and inherited) are consulted + /// The reordered assignments; the input order when no implication applies + private static List OrderAssignmentsByImplicationConstraints(List assignmentElements, IClass umlClass) + { + if (assignmentElements.Count < 2 || umlClass == null) + { + return assignmentElements; + } + + var ordered = new List(assignmentElements); + + var namespaces = umlClass.QueryAllGeneralClassifiers() + .OfType() + .Prepend(umlClass); + + var implications = namespaces + .SelectMany(owner => owner.OwnedRule) + .SelectMany(rule => rule.Specification?.OfType() ?? []) + .SelectMany(specification => specification.Body ?? []) + .Select(body => SimpleImplicationConstraint.Match(body ?? string.Empty)) + .Where(match => match.Success) + .Select(match => (Antecedent: match.Groups[1].Value, Consequent: match.Groups[2].Value)); + + foreach (var implication in implications) + { + var antecedentIndex = ordered.FindIndex(assignment => IsBooleanAssignmentTo(assignment, implication.Antecedent)); + var consequentIndex = ordered.FindIndex(assignment => IsBooleanAssignmentTo(assignment, implication.Consequent)); + + if (antecedentIndex < 0 || consequentIndex < 0 || antecedentIndex < consequentIndex) + { + continue; + } + + var antecedentAssignment = ordered[antecedentIndex]; + ordered.RemoveAt(antecedentIndex); + ordered.Insert(consequentIndex, antecedentAssignment); + } + + return ordered; + } + + /// + /// Determines whether is a boolean-flag assignment + /// (?=) targeting the property named . + /// + /// The to test + /// The property name an OCL implication operand refers to + /// true when the assignment is a ?= assignment to that property + private static bool IsBooleanAssignmentTo(AssignmentElement assignment, string propertyName) + { + return string.Equals(assignment.Operator, "?=", StringComparison.Ordinal) + && string.Equals(assignment.Property, propertyName, StringComparison.Ordinal); + } + /// /// Emits a single type-dispatched branch body. /// @@ -1120,6 +1219,122 @@ private static bool IsTypeDispatcherRule(TextualNotationRule rule) && alternative.Elements[0] is NonTerminalElement); } + /// + /// Collects the switch arms that must be hoisted ABOVE the depth-sorted arms so a metaclass that is + /// only reachable THROUGH a nested alternation rule is not captured by an earlier arm typed on one + /// of its supertypes. + /// The depth sort ranks a nested-rule arm by the RULE's own declared target (e.g. + /// BehaviorUsageElement : Usage, shallow), never by the deepest metaclass reachable through it + /// (e.g. PerformActionUsage, deep) — and when that target IS the generating class the arm is + /// forced last as the default: case, where no sort key can rescue it. C# pattern matching is + /// first-match-wins on runtime type, so case IEventOccurrenceUsage would swallow every + /// PerformActionUsage before the BehaviorUsageElement arm is ever reached. + /// Only UNGUARDED arms are treated as shadowing: a when-guarded arm declines runtime + /// types its guard excludes, which is precisely what those guards exist for. + /// + /// The depth-sorted arms, in emission order + /// The synthesized when guards, keyed by rule element + /// The rule element emitted as default:, or null + /// The used to resolve rule targets + /// The current + /// The arms to emit first, most-derived first; empty when no arm is shadowed + private static List<(NonTerminalElement RuleElement, IClass UmlClass)> CollectShadowedNestedRuleTargets(IReadOnlyList<(NonTerminalElement RuleElement, IClass UmlClass)> orderedElements, IReadOnlyDictionary whenGuards, NonTerminalElement defaultRuleElement, IXmiElementCache cache, RuleGenerationContext ruleGenerationContext) + { + var hoisted = new List<(NonTerminalElement RuleElement, IClass UmlClass)>(); + + for (var elementIndex = 0; elementIndex < orderedElements.Count; elementIndex++) + { + var element = orderedElements[elementIndex]; + var reachableClasses = QueryReachableTargetClasses(element.RuleElement, cache, ruleGenerationContext.AllRules); + + foreach (var reachableClass in reachableClasses.Where(reachableClass => reachableClass != element.UmlClass)) + { + var shadowingArm = orderedElements + .Take(elementIndex) + .Where(arm => !whenGuards.ContainsKey(arm.RuleElement) && arm.RuleElement != defaultRuleElement) + .FirstOrDefault(arm => reachableClass.QueryAllGeneralClassifiers().Contains(arm.UmlClass)); + + if (shadowingArm.RuleElement == null) + { + continue; + } + + if (QueryReachableTargetClasses(shadowingArm.RuleElement, cache, ruleGenerationContext.AllRules).Contains(reachableClass)) + { + continue; + } + + // Hoisted arms are emitted at the very top of the switch, so a hoist is only safe when no + // existing arm is typed on a STRICT SUBTYPE of the hoisted class — hoisting above such an + // arm would trade one shadowing defect for another. + if (orderedElements.Any(arm => arm.UmlClass != reachableClass && arm.UmlClass.QueryAllGeneralClassifiers().Contains(reachableClass))) + { + continue; + } + + if (hoisted.All(alreadyHoisted => alreadyHoisted.UmlClass != reachableClass)) + { + hoisted.Add((element.RuleElement, reachableClass)); + } + } + } + + hoisted.Sort((a, b) => b.UmlClass.QueryAllGeneralClassifiers().Count.CompareTo(a.UmlClass.QueryAllGeneralClassifiers().Count)); + + return hoisted; + } + + /// + /// Resolves every UML that can dispatch to, + /// flattening nested type-dispatcher rules transitively. + /// + /// The to resolve + /// The used to resolve rule targets + /// All available rules for recursive lookup + /// The reachable classes, including the non-terminal's own target class + private static List QueryReachableTargetClasses(NonTerminalElement nonTerminal, IXmiElementCache cache, IReadOnlyList allRules) + { + var reachableClasses = new List(); + CollectReachableTargetClasses(nonTerminal.Name, cache, allRules, reachableClasses, new HashSet(StringComparer.Ordinal)); + + return reachableClasses; + } + + /// + /// Recursively accumulates the target classes reachable from , descending + /// only into rules that are pure type-dispatchers (see ). + /// + /// The non-terminal rule name to resolve + /// The used to resolve rule targets + /// All available rules for recursive lookup + /// The accumulated classes + /// The already-visited rule names, preventing infinite recursion + private static void CollectReachableTargetClasses(string ruleName, IXmiElementCache cache, IReadOnlyList allRules, List reachableClasses, HashSet visitedRules) + { + if (!visitedRules.Add(ruleName)) + { + return; + } + + var rule = allRules.SingleOrDefault(x => x.RuleName == ruleName); + var targetClass = RuleQueryUtilities.FindClass(cache, rule?.EffectiveTarget ?? ruleName); + + if (targetClass != null && !reachableClasses.Contains(targetClass)) + { + reachableClasses.Add(targetClass); + } + + if (!IsTypeDispatcherRule(rule)) + { + return; + } + + foreach (var nestedNonTerminal in rule.Alternatives.Select(alternative => alternative.Elements[0]).OfType()) + { + CollectReachableTargetClasses(nestedNonTerminal.Name, cache, allRules, reachableClasses, visitedRules); + } + } + /// /// Runs the structural-predicate walk for and returns the raw clause /// list, seeding the per-walk visited-rules set and cursor-state bookkeeping. diff --git a/SysML2.NET.CodeGenerator/HandleBarHelpers/RulesHelper.cs b/SysML2.NET.CodeGenerator/HandleBarHelpers/RulesHelper.cs index 9efa052c..a1ec249d 100644 --- a/SysML2.NET.CodeGenerator/HandleBarHelpers/RulesHelper.cs +++ b/SysML2.NET.CodeGenerator/HandleBarHelpers/RulesHelper.cs @@ -140,7 +140,13 @@ public static void RegisterRulesHelper(this IHandlebars handlebars) if (isOwnedExpressionRule) { - writer.WriteSafeString("if (operatorParensNeeded) { stringBuilder.Append(')'); }" + Environment.NewLine); + // Emitted as the STRING ") " rather than the char ')': only the string + // overload of IndentedStringBuilder.Append runs the tight-left token + // normalisation that strips the space the operand left behind, and the + // trailing space restores the separator the enclosing binary-operator + // rule expects before it appends its own operator. The char overload + // bypasses both and renders `a and b )xor (c and d)`. + writer.WriteSafeString("if (operatorParensNeeded) { stringBuilder.Append(\") \"); }" + Environment.NewLine); } } }); diff --git a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a-Variant Configuration - General Concept.sysml b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a-Variant Configuration - General Concept.sysml new file mode 100644 index 00000000..9c30ab3c --- /dev/null +++ b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a-Variant Configuration - General Concept.sysml @@ -0,0 +1,40 @@ +package '7a-Variant Configuration - General Concept' { + part def Vehicle; + part part1; + part part2; + part part3; + part part4; + part part5; + part part6; + abstract part anyVehicleConfig: Vehicle { + variation part subsystemA { + variant part subsystem1 { + part :>> part1; + part :>> part2; + } + variant part subsystem2 { + part :>> part2; + part :>> part3; + } + } + variation part subsystemB { + variant part subsystem3 { + part :>> part4; + part :>> part5; + } + variant part subsystem4 { + part :>> part5; + part :>> part6; + } + } + assert constraint { subsystemA != subsystemA::subsystem2 | subsystemB == subsystemB::subsystem3 } + } + part vehicleConfigA :> anyVehicleConfig { + part :>> subsystemA = subsystemA::subsystem1; + part :>> subsystemB = subsystemB::subsystem3; + } + part VehicleConfigB :> anyVehicleConfig { + part :>> subsystemA = subsystemA::subsystem2; + part :>> subsystemB = subsystemB::subsystem3; + } +} diff --git a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a1-Variant Configuration - General Concept-a.sysml b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a1-Variant Configuration - General Concept-a.sysml new file mode 100644 index 00000000..f8f28bb2 --- /dev/null +++ b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a1-Variant Configuration - General Concept-a.sysml @@ -0,0 +1,64 @@ +package '7a1-Variant Configuration - General Concept-a' { + action doX; + action doY; + part part1; + part part2; + part part3 { + port p1; + } + part part4; + part part5 { + port p2; + variation perform action doXorY { + variant perform doX; + variant perform doY; + } + } + part part6; + abstract part def SubsystemA { + abstract part :>> part3[0..1]; + } + abstract part def SubsystemB { + abstract part :>> part5[1]; + } + part anyVehicleConfig { + variation part subsystemA: SubsystemA { + variant part subsystem1: SubsystemA { + part :>> part1[1]; + part :>> part2[1]; + } + variant part subsystem2: SubsystemA { + part :>> part2[1]; + part :>> part3[1]; + } + } + variation part subsystemB: SubsystemB { + variant part subsystem3: SubsystemB { + part :>> part4[1]; + part :>> part5[1]; + } + variant part subsystem4: SubsystemB { + part :>> part5[1]; + part :>> part6[1]; + } + } + connect[0..1] subsystemA.part3.p1 to[1] subsystemB.part5.p2; + assert constraint { subsystemA != subsystemA::subsystem2 | subsystemB == subsystemB::subsystem3 } + } + part vehicleConfigA :> anyVehicleConfig { + part :>> subsystemA = subsystemA::subsystem1; + part :>> subsystemB = subsystemB::subsystem3 { + part :>> part5 { + perform action :>> doXorY = doX; + } + } + } + part VehicleConfigB :> anyVehicleConfig { + part :>> subsystemA = subsystemA::subsystem2; + part :>> subsystemB = subsystemB::subsystem4 { + part :>> part5 { + perform action :>> doXorY = doY; + } + } + } +} diff --git a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7b-Variant Configurations.sysml b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7b-Variant Configurations.sysml new file mode 100644 index 00000000..7498e6dc --- /dev/null +++ b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7b-Variant Configurations.sysml @@ -0,0 +1,101 @@ +package '7b-Variant Configurations' { + private import RequirementsModel::*; + private import DesignModel::*; + private import VariantDefinitions::*; + private import ControlFunctions::forAll; + package RequirementsModel { + requirement def EnginePerformanceRequirement; + requirement highPerformanceRequirement: EnginePerformanceRequirement; + requirement normalPerformanceRequirement: EnginePerformanceRequirement; + } + package DesignModel { + part def Vehicle; + part def Engine; + part def Transmission; + part def Clutch; + part def Driveshaft; + part def RearAxleAssembly; + part def Wheel; + port def FuelCmdPort; + port def ClutchPort; + port def ShaftPort_b; + port def ShaftPort_c; + port def ShaftPort_d; + port def VehicleToRoadPort; + port def WheelToRoadPort; + part vehicle: Vehicle { + port fuelCmdPort; + bind fuelCmdPort = engine.fuelCmdPort; + part engine: Engine[1] { + port fuelCmdPort: FuelCmdPort; + } + part transmission: Transmission[1] { + part clutch: Clutch[1] { + port clutchPort: ClutchPort; + } + } + part driveshaft: Driveshaft[1] { + port shaftPort_b: ShaftPort_b; + port shaftPort_c: ShaftPort_c; + } + part rearAxleAssembly: RearAxleAssembly { + part rearWheels: Wheel[2] { + port wheelToRoadPort: WheelToRoadPort; + } + } + port vehicleToRoadPort: VehicleToRoadPort { + port wheelToRoadPort: WheelToRoadPort[2]; + } + } + } + package VariantDefinitions { + part def '4CylEngine' :> Engine; + part def '6CylEngine' :> Engine; + part def ManualTransmission :> Transmission; + part def AutomaticTransmission :> Transmission; + part def ManualClutch :> Clutch; + part def AutomaticClutch :> Clutch; + port def ManualClutchPort :> ClutchPort; + port def AutomaticClutchPort :> ClutchPort; + part def NarrowRimWheel :> Wheel; + part def WideRimWheel :> Wheel; + } + package VariabilityModel { + part anyVehicleConfig :> vehicle { + variation requirement engineRqtChoice: EnginePerformanceRequirement { + variant highPerformanceRequirement; + variant normalPerformanceRequirement; + } + variation part engineChoice :>> engine { + variant part '4cylEngine': '4CylEngine'; + variant part '6cylEngine': '6CylEngine'; + } + satisfy engineRqtChoice by engineChoice; + assert constraint 'engine choice constraint' { if engineRqtChoice == engineRqtChoice::highPerformanceRequirement ? engineChoice == engineChoice::'6cylEngine' else engineChoice == engineChoice::'4cylEngine' } + variation part transmissionChoice :>> transmission { + variant part manualTransmission: ManualTransmission { + part :>> clutch : ManualClutch { + port :>> clutchPort : ManualClutchPort; + } + } + variant part automaticTransmission: AutomaticTransmission { + part :>> clutch : AutomaticClutch { + port :>> clutchPort : AutomaticClutchPort; + } + } + } + assert constraint 'engine-transmission selection constraint' { (engineChoice == engineChoice::'4cylEngine' and transmissionChoice == transmissionChoice::manualTransmission) xor (engineChoice == engineChoice::'6cylEngine' and transmissionChoice == transmissionChoice::automaticTransmission) } + part :>> rearAxleAssembly { + variation part rearWheelChoice :>> rearWheels { + variant part narrowRimWheel: NarrowRimWheel; + variant part wideRimWheel: WideRimWheel; + } + assert constraint 'engine-wheel selection constraint' { (engineChoice == engineChoice::'4cylEngine' and rearWheelChoice -> forAll { in ref w; w == rearWheelChoice::narrowRimWheel }) xor (engineChoice == engineChoice::'6cylEngine' and rearWheelChoice -> forAll { in ref w; w == rearWheelChoice::wideRimWheel }) } + } + } + variation part vehicleChoice :> anyVehicleConfig { + variant part vehicle_c1; + variant part vehicle_c2; + } + } +} diff --git a/SysML2.NET.Serializer.TextualNotation.Tests/Writers/TextualNotationValidationTestFixture.cs b/SysML2.NET.Serializer.TextualNotation.Tests/Writers/TextualNotationValidationTestFixture.cs index 1014fdab..f2b37ab1 100644 --- a/SysML2.NET.Serializer.TextualNotation.Tests/Writers/TextualNotationValidationTestFixture.cs +++ b/SysML2.NET.Serializer.TextualNotation.Tests/Writers/TextualNotationValidationTestFixture.cs @@ -54,6 +54,9 @@ public class TextualNotationValidationTestFixture [TestCase("05-State-based Behavior", "5-State-based Behavior-1.sysmlx")] [TestCase("05-State-based Behavior", "5-State-based Behavior-2.sysmlx")] [TestCase("06-Individual and Snapshots", "6-Individual and Snapshots.sysmlx")] + [TestCase("07-Variant Configuration", "7a-Variant Configuration - General Concept.sysmlx")] + [TestCase("07-Variant Configuration", "7a1-Variant Configuration - General Concept-a.sysmlx")] + [TestCase("07-Variant Configuration", "7b-Variant Configurations.sysmlx")] public async Task VerifyValidationTextualNotationXmi(string folderName, string fileName) { var loggerFactory = LoggerFactory.Create(builder => @@ -79,7 +82,7 @@ public async Task VerifyValidationTextualNotationXmi(string folderName, string f // file's external references. They form the global Namespace (KerML §8.2.3.5.2), so the writer // needs them to shorten a reference routed through a library the model does not itself import. using var writerContext = new TextualNotationWriterContext(rootNamespace, readResult.ReferencedNamespaces); - writerContext.EmitOperatorParentheses = false; + writerContext.EmitOperatorParentheses = true; var stringBuilder = new IndentedStringBuilder(); try diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/ElementTextualNotationBuilder.cs b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/ElementTextualNotationBuilder.cs index 6c0df471..24916da3 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/ElementTextualNotationBuilder.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/ElementTextualNotationBuilder.cs @@ -173,6 +173,12 @@ public static void BuildOwnedRelatedElement(SysML2.NET.Core.POCO.Root.Elements.I { switch (poco) { + case SysML2.NET.Core.POCO.Kernel.Multiplicities.IMultiplicityRange pocoMultiplicityRange: + BuildNonFeatureElement(pocoMultiplicityRange, writerContext, stringBuilder); + break; + case SysML2.NET.Core.POCO.Core.Types.IMultiplicity pocoMultiplicity: + BuildNonFeatureElement(pocoMultiplicity, writerContext, stringBuilder); + break; case SysML2.NET.Core.POCO.Core.Features.IFeature pocoFeature: FeatureTextualNotationBuilder.BuildFeatureElement(pocoFeature, writerContext, stringBuilder); break; diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/EnumerationDefinitionTextualNotationBuilder.cs b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/EnumerationDefinitionTextualNotationBuilder.cs index 09d78e48..48e9a25b 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/EnumerationDefinitionTextualNotationBuilder.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/EnumerationDefinitionTextualNotationBuilder.cs @@ -58,12 +58,16 @@ public static void BuildEnumerationBody(SysML2.NET.Core.POCO.Systems.Enumeration { case SysML2.NET.Core.POCO.Systems.DefinitionAndUsage.IVariantMembership variantMembership: VariantMembershipTextualNotationBuilder.BuildEnumerationUsageMember(variantMembership, writerContext, stringBuilder); + ownedRelationshipCursor.Move(); break; case SysML2.NET.Core.POCO.Root.Namespaces.IOwningMembership owningMembership: OwningMembershipTextualNotationBuilder.BuildAnnotatingMember(owningMembership, writerContext, stringBuilder); + ownedRelationshipCursor.Move(); + break; + default: + ownedRelationshipCursor.Move(); break; } - ownedRelationshipCursor.Move(); } stringBuilder.DecreaseIndent(); diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/ExpressionTextualNotationBuilder.cs b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/ExpressionTextualNotationBuilder.cs index 94b529fd..7f1f532a 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/ExpressionTextualNotationBuilder.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/ExpressionTextualNotationBuilder.cs @@ -71,7 +71,7 @@ public static void BuildOwnedExpression(SysML2.NET.Core.POCO.Kernel.Functions.IE BuildPrimaryExpression(poco, writerContext, stringBuilder); break; } - if (operatorParensNeeded) { stringBuilder.Append(')'); } + if (operatorParensNeeded) { stringBuilder.Append(") "); } } diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/SharedTextualNotationBuilder.cs b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/SharedTextualNotationBuilder.cs index e7b77444..8fd3c16a 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/SharedTextualNotationBuilder.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/SharedTextualNotationBuilder.cs @@ -45,13 +45,13 @@ public static partial class SharedTextualNotationBuilder /// The that accumulates the entire textual notation with indentation public static void BuildBasicDefinitionPrefix(SysML2.NET.Core.POCO.Systems.DefinitionAndUsage.IDefinition poco, TextualNotationWriterContext writerContext, IndentedStringBuilder stringBuilder) { - if (poco.IsAbstract) + if (poco.IsVariation) { - stringBuilder.Append(" abstract "); + stringBuilder.Append(" variation "); } - else if (poco.IsVariation) + else if (poco.IsAbstract) { - stringBuilder.Append(" variation "); + stringBuilder.Append(" abstract "); } } diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/TypeTextualNotationBuilder.cs b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/TypeTextualNotationBuilder.cs index 225a7ea8..b2ab917e 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/TypeTextualNotationBuilder.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/TypeTextualNotationBuilder.cs @@ -442,18 +442,24 @@ public static void BuildMetadataBody(SysML2.NET.Core.POCO.Core.Types.IType poco, { case SysML2.NET.Core.POCO.Core.Types.IFeatureMembership featureMembership: FeatureMembershipTextualNotationBuilder.BuildMetadataBodyUsageMember(featureMembership, writerContext, stringBuilder); + ownedRelationshipCursor.Move(); break; case SysML2.NET.Core.POCO.Root.Namespaces.IOwningMembership owningMembership: OwningMembershipTextualNotationBuilder.BuildDefinitionMember(owningMembership, writerContext, stringBuilder); + ownedRelationshipCursor.Move(); break; case SysML2.NET.Core.POCO.Root.Namespaces.IMembership membership: MembershipTextualNotationBuilder.BuildAliasMember(membership, writerContext, stringBuilder); + ownedRelationshipCursor.Move(); break; case SysML2.NET.Core.POCO.Root.Namespaces.IImport import: ImportTextualNotationBuilder.BuildImport(import, writerContext, stringBuilder); + ownedRelationshipCursor.Move(); + break; + default: + ownedRelationshipCursor.Move(); break; } - ownedRelationshipCursor.Move(); } stringBuilder.DecreaseIndent(); @@ -912,15 +918,18 @@ public static void BuildFunctionBody(SysML2.NET.Core.POCO.Core.Types.IType poco, public static void BuildFunctionBodyPart(SysML2.NET.Core.POCO.Core.Types.IType poco, TextualNotationWriterContext writerContext, IndentedStringBuilder stringBuilder) { var ownedRelationshipCursor = writerContext.CursorCache.GetOrCreateCursor(poco.Id, "ownedRelationship", poco.OwnedRelationship); - while (ownedRelationshipCursor.Current != null) + while (ownedRelationshipCursor.Current is not null and not SysML2.NET.Core.POCO.Kernel.Functions.IResultExpressionMembership) { switch (ownedRelationshipCursor.Current) { case SysML2.NET.Core.POCO.Kernel.Functions.IReturnParameterMembership returnParameterMembership: ReturnParameterMembershipTextualNotationBuilder.BuildReturnFeatureMember(returnParameterMembership, writerContext, stringBuilder); + ownedRelationshipCursor.Move(); + break; + default: + BuildTypeBodyElement(poco, writerContext, stringBuilder); break; } - ownedRelationshipCursor.Move(); } diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/UsageTextualNotationBuilder.cs b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/UsageTextualNotationBuilder.cs index a700af45..b491b39f 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/UsageTextualNotationBuilder.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/AutoGenTextualNotationBuilder/UsageTextualNotationBuilder.cs @@ -78,13 +78,13 @@ public static void BuildRefPrefix(SysML2.NET.Core.POCO.Systems.DefinitionAndUsag stringBuilder.Append(' '); } - if (poco.IsAbstract) + if (poco.IsVariation) { - stringBuilder.Append(" abstract "); + stringBuilder.Append(" variation "); } - else if (poco.IsVariation) + else if (poco.IsAbstract) { - stringBuilder.Append(" variation "); + stringBuilder.Append(" abstract "); } @@ -454,6 +454,15 @@ public static void BuildVariantUsageElement(SysML2.NET.Core.POCO.Systems.Definit { switch (poco) { + case SysML2.NET.Core.POCO.Systems.UseCases.IIncludeUseCaseUsage pocoIncludeUseCaseUsage: + BuildBehaviorUsageElement(pocoIncludeUseCaseUsage, writerContext, stringBuilder); + break; + case SysML2.NET.Core.POCO.Systems.States.IExhibitStateUsage pocoExhibitStateUsage: + BuildBehaviorUsageElement(pocoExhibitStateUsage, writerContext, stringBuilder); + break; + case SysML2.NET.Core.POCO.Systems.Actions.IPerformActionUsage pocoPerformActionUsage: + BuildBehaviorUsageElement(pocoPerformActionUsage, writerContext, stringBuilder); + break; case SysML2.NET.Core.POCO.Systems.Flows.ISuccessionFlowUsage pocoSuccessionFlowUsage: SuccessionFlowUsageTextualNotationBuilder.BuildSuccessionFlowUsage(pocoSuccessionFlowUsage, writerContext, stringBuilder); break; diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/OperatorPrecedence.cs b/SysML2.NET.Serializer.TextualNotation/Writers/OperatorPrecedence.cs index a053427f..6f07fff8 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/OperatorPrecedence.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/OperatorPrecedence.cs @@ -20,6 +20,8 @@ namespace SysML2.NET.Serializer.TextualNotation.Writers { + using System.Linq; + using SysML2.NET.Core.POCO.Kernel.Expressions; using SysML2.NET.Core.POCO.Kernel.Functions; @@ -36,41 +38,42 @@ namespace SysML2.NET.Serializer.TextualNotation.Writers /// returns true when the inner expression /// would be ambiguous without parens. /// + /// + /// Parentheses are emitted in two tiers. REQUIRED parens keep the text re-parsing to the same + /// model, and are decided by precedence alone. CLARIFYING parens are emitted where precedence + /// already settles the grouping but a reader benefits from seeing it — see + /// . Both tiers are gated by + /// TextualNotationWriterContext.EmitOperatorParentheses. + /// /// internal static class OperatorPrecedence { /// - /// Levels are inferred from the OwnedExpression rule alternative ordering - /// in Resources/KerML-textual-bnf.kebnf and tuned to reproduce the - /// canonical parenthesization in Resources/Quantities.sysml. Lower = binds - /// less tightly. + /// Levels transcribe the normative operator precedence table — OMG KerML v1.0, + /// Clause 8.2.5.8.1, Table 6 "Operator Precedence (highest to lowest)" — which + /// Note 2 of the same clause makes the sole basis for implicit grouping of nested + /// OperatorExpressions. Higher value = binds more tightly. The table is a + /// total order, so the alternation order of the OwnedExpression rule in + /// Resources/KerML-textual-bnf.kebnf carries no precedence information and + /// must not be used to infer one. /// private const int LevelConditional = 1; - private const int LevelConditionalBinary = 2; - private const int LevelBitwise = 3; - private const int LevelEquality = 4; - private const int LevelRelational = 5; - private const int LevelClassification = 6; - private const int LevelAdditive = 7; - private const int LevelMultiplicative = 8; - private const int LevelPower = 9; - private const int LevelUnary = 10; - private const int LevelExtent = 11; + private const int LevelNullCoalescing = 2; + private const int LevelImplies = 3; + private const int LevelInclusiveOr = 4; + private const int LevelExclusiveOr = 5; + private const int LevelAnd = 6; + private const int LevelEquality = 7; + private const int LevelClassification = 8; + private const int LevelRelational = 9; + private const int LevelRange = 10; + private const int LevelAdditive = 11; + private const int LevelMultiplicative = 12; + private const int LevelPower = 13; + private const int LevelUnary = 14; + private const int LevelExtent = 15; private const int LevelPrimary = int.MaxValue; - /// - /// Coarse-grained operator families used by the cross-family wrap rule. - /// - private enum PrecedenceBucket - { - Conditional, - Binary, - Classification, - Unary, - Extent, - Primary, - } - /// /// Returns the precedence level of . Lower = binds less /// tightly. Primary forms (literals, feature references, invocations, etc.) return @@ -138,14 +141,31 @@ private static int GetOperatorExpressionPrecedence(IOperatorExpression op) return LevelConditional; } - if (@operator is "or" or "and" or "implies" or "??") + if (@operator == "??") + { + return LevelNullCoalescing; + } + + if (@operator == "implies") + { + return LevelImplies; + } + + // Table 6 keeps each bitwise operator on the level of its logical spelling: + // `|` sits with `or`, `&` sits with `and`, and `xor` sits alone between them. + if (@operator is "|" or "or") { - return LevelConditionalBinary; + return LevelInclusiveOr; } - if (@operator is "|" or "&" or "xor") + if (@operator == "xor") { - return LevelBitwise; + return LevelExclusiveOr; + } + + if (@operator is "&" or "and") + { + return LevelAnd; } if (@operator is "==" or "!=" or "===" or "!==") @@ -153,14 +173,19 @@ private static int GetOperatorExpressionPrecedence(IOperatorExpression op) return LevelEquality; } - if (@operator is "<" or ">" or "<=" or ">=" or "..") + if (@operator is "as" or "istype" or "hastype" or "@" or "@@" or "meta") + { + return LevelClassification; + } + + if (@operator is "<" or ">" or "<=" or ">=") { return LevelRelational; } - if (@operator is "as" or "istype" or "hastype" or "@" or "@@" or "meta") + if (@operator == "..") { - return LevelClassification; + return LevelRange; } if (@operator is "*" or "/" or "%") @@ -197,13 +222,19 @@ private static int GetOperatorExpressionPrecedence(IOperatorExpression op) /// /// Determines whether needs to be wrapped in - /// (…) when it appears as an operand of . The rule: - /// cross-family nesting always wraps (binary inside conditional, etc.); same-family - /// nesting wraps only when the inner has same-or-lower precedence than the outer. + /// (…) when it appears as an operand of . + /// + /// The rule follows KerML Clause 8.2.5.8.1 Note 2 directly: grouping is implied by + /// operator precedence alone, so an operand needs no parentheses as soon as it binds + /// more tightly than the operator it is an operand of. Parentheses are emitted only + /// where re-parsing the unparenthesized text would regroup the expression — i.e. when + /// the operand binds less tightly, or equally tightly (where associativity, not + /// precedence, decides the grouping, and the operand's slot is not known here). + /// /// /// The enclosing operator expression. /// The candidate operand expression. - /// true when parens are required for unambiguous rendering. + /// true when parens are required, or emitted for clarity. internal static bool NeedsParenthesesAsOperand(IExpression outer, IExpression operand) { if (operand == null || outer == null) @@ -218,71 +249,105 @@ internal static bool NeedsParenthesesAsOperand(IExpression outer, IExpression op return false; } - var outerPrecedence = GetExpressionPrecedence(outer); - - var innerBucket = GetBucket(innerPrecedence); - var outerBucket = GetBucket(outerPrecedence); - - // Unary prefix operators (`~`, `not`, unary `+`/`-`) bind tighter than any binary - // operator and have no LHS to confuse with — they are unambiguous as operands and - // never need parenthesization, even when crossing operator-family buckets. E.g. - // `not a and b` re-parses unambiguously as `(not a) and b`. - if (innerBucket == PrecedenceBucket.Unary) + // Unary prefix operators (`~`, `not`, unary `+`/`-`) and the extent operator (`all`) + // bind tighter than every binary operator and have no left operand that the + // enclosing operator could absorb — they are unambiguous as operands and never need + // parenthesization. E.g. `not a and b` re-parses unambiguously as `(not a) and b`. + if (innerPrecedence >= LevelUnary) { return false; } - if (innerBucket != outerBucket) + if (innerPrecedence <= GetExpressionPrecedence(outer)) { return true; } - return innerPrecedence <= outerPrecedence; + return NeedsClarifyingParentheses(outer, operand); } /// - /// Counts the IOperatorExpression's argument-members to distinguish binary additive - /// (a + b) from unary sign (+ a). Two argument-members indicate the - /// binary form. + /// Determines whether should be wrapped in (…) purely for + /// READABILITY, in the cases where has already + /// established that precedence alone makes the grouping unambiguous. + /// + /// Two mixes are clarified, following the grouping convention mainstream linters apply to the + /// same problem (e.g. ESLint's no-mixed-operators): two DIFFERENT binary logical + /// connectives — a and b xor c and d renders as (a and b) xor (c and d) — and two + /// arithmetic operators from different precedence tiers — a + b * c renders as + /// a + (b * c). + /// + /// + /// Comparisons nested in logical connectives (a == b and c == d) are deliberately left + /// bare: that grouping is read the same way by everyone, so parenthesizing it is noise. The + /// OMG pilot renders the same corpus the same way. + /// /// - /// The operator expression. - /// true when at least two argument-members are present. - private static bool HasTwoArguments(IOperatorExpression op) + /// The enclosing operator expression. + /// The candidate operand expression, known to bind more tightly. + /// true when parens clarify an otherwise unambiguous grouping. + private static bool NeedsClarifyingParentheses(IExpression outer, IExpression operand) { - var count = 0; - - foreach (var relationship in op.OwnedRelationship) + if (outer is not IOperatorExpression outerOperator || operand is not IOperatorExpression operandOperator) { - if (relationship is SysML2.NET.Core.POCO.Kernel.Behaviors.IParameterMembership) - { - count++; + return false; + } - if (count >= 2) - { - return true; - } - } + if (IsLogicalConnective(outerOperator.Operator) && IsLogicalConnective(operandOperator.Operator)) + { + return outerOperator.Operator != operandOperator.Operator; } - return false; + // The operand is already known to bind more tightly than the outer operator, so two + // arithmetic operators here necessarily sit on different tiers of Table 6. + return IsArithmeticOperator(outerOperator.Operator) && IsArithmeticOperator(operandOperator.Operator); } /// - /// Maps a precedence level to its coarse-grained operator family bucket. + /// Determines whether is one of the binary logical connectives — + /// the short-circuiting ConditionalBinaryOperators plus their non-short-circuiting + /// spellings and xor. /// - /// The precedence level. - /// The bucket. - private static PrecedenceBucket GetBucket(int precedence) + /// The operator discriminator. + /// true when the operator is a binary logical connective. + private static bool IsLogicalConnective(string @operator) { - return precedence switch - { - LevelConditional or LevelConditionalBinary => PrecedenceBucket.Conditional, - LevelBitwise or LevelEquality or LevelRelational or LevelAdditive or LevelMultiplicative or LevelPower => PrecedenceBucket.Binary, - LevelClassification => PrecedenceBucket.Classification, - LevelUnary => PrecedenceBucket.Unary, - LevelExtent => PrecedenceBucket.Extent, - _ => PrecedenceBucket.Primary, - }; + return @operator is "&" or "and" or "|" or "or" or "xor" or "implies" or "??"; + } + + /// + /// Determines whether is one of the arithmetic operators. + /// The unary + / - forms never reach this test — they classify as + /// and are returned early by + /// . + /// + /// The operator discriminator. + /// true when the operator is arithmetic. + private static bool IsArithmeticOperator(string @operator) + { + return @operator is "+" or "-" or "*" or "/" or "%" or "^" or "**"; + } + + /// + /// Counts the IOperatorExpression's argument-members to distinguish binary additive + /// (a + b) from unary sign (+ a). Two argument-members indicate the + /// binary form. + /// + /// The result-member must be excluded from the count: every OperatorExpression + /// — unary ones included — owns an EmptyResultMember : ReturnParameterMembership + /// per the UnaryOperatorExpression / BinaryOperatorExpression rules in + /// Resources/KerML-textual-bnf.kebnf, and IReturnParameterMembership + /// derives from IParameterMembership. Counting it would make every unary + /// +/- look binary and misclassify it as additive rather than unary. + /// + /// + /// The operator expression. + /// true when at least two argument-members are present. + private static bool HasTwoArguments(IOperatorExpression op) + { + return op.OwnedRelationship + .OfType() + .Count(membership => membership is not IReturnParameterMembership) >= 2; } } } diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/TextualNotationWriterContext.cs b/SysML2.NET.Serializer.TextualNotation/Writers/TextualNotationWriterContext.cs index 4adac1fe..c12b309c 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/TextualNotationWriterContext.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/TextualNotationWriterContext.cs @@ -71,6 +71,12 @@ public TextualNotationWriterContext(INamespace contextNamespace, IEnumerable(…) to guarantee round-trip /// fidelity against the precedence-climbing parser. /// + /// In addition to the parens that fidelity requires, this mode emits clarifying parens + /// where precedence already settles the grouping but the mix reads ambiguously — + /// two different logical connectives ((a and b) xor (c and d)) and arithmetic + /// across precedence tiers (a + (b * c)). See . + /// + /// /// Set to false to suppress the writer-side disambiguation parens entirely. /// The resulting output is more compact and matches the idiomatic shorthand used /// throughout the SysML tutorials (e.g. a and b or c), but a model whose From 1d9ad3898f4e7ab4240d680940e550d9d39a362f Mon Sep 17 00:00:00 2001 From: atheate Date: Thu, 13 Aug 2026 10:18:31 +0200 Subject: [PATCH 2/3] fix naming --- ...iant Configuration - General Concept.sysml | 8 ++--- ...nt Configuration - General Concept-a.sysml | 12 +++---- .../7b-Variant Configurations.sysml | 4 +-- .../Writers/NameResolutionCache.cs | 36 +++++++++++++++++-- 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a-Variant Configuration - General Concept.sysml b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a-Variant Configuration - General Concept.sysml index 9c30ab3c..2363b837 100644 --- a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a-Variant Configuration - General Concept.sysml +++ b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a-Variant Configuration - General Concept.sysml @@ -30,11 +30,11 @@ package '7a-Variant Configuration - General Concept' { assert constraint { subsystemA != subsystemA::subsystem2 | subsystemB == subsystemB::subsystem3 } } part vehicleConfigA :> anyVehicleConfig { - part :>> subsystemA = subsystemA::subsystem1; - part :>> subsystemB = subsystemB::subsystem3; + part :>> subsystemA = subsystem1; + part :>> subsystemB = subsystem3; } part VehicleConfigB :> anyVehicleConfig { - part :>> subsystemA = subsystemA::subsystem2; - part :>> subsystemB = subsystemB::subsystem3; + part :>> subsystemA = subsystem2; + part :>> subsystemB = subsystem3; } } diff --git a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a1-Variant Configuration - General Concept-a.sysml b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a1-Variant Configuration - General Concept-a.sysml index f8f28bb2..6a74a769 100644 --- a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a1-Variant Configuration - General Concept-a.sysml +++ b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7a1-Variant Configuration - General Concept-a.sysml @@ -46,18 +46,18 @@ package '7a1-Variant Configuration - General Concept-a' { assert constraint { subsystemA != subsystemA::subsystem2 | subsystemB == subsystemB::subsystem3 } } part vehicleConfigA :> anyVehicleConfig { - part :>> subsystemA = subsystemA::subsystem1; - part :>> subsystemB = subsystemB::subsystem3 { + part :>> subsystemA = subsystem1; + part :>> subsystemB = subsystem3 { part :>> part5 { - perform action :>> doXorY = doX; + perform action :>> doXorY = '7a1-Variant Configuration - General Concept-a'::doX; } } } part VehicleConfigB :> anyVehicleConfig { - part :>> subsystemA = subsystemA::subsystem2; - part :>> subsystemB = subsystemB::subsystem4 { + part :>> subsystemA = subsystem2; + part :>> subsystemB = subsystem4 { part :>> part5 { - perform action :>> doXorY = doY; + perform action :>> doXorY = '7a1-Variant Configuration - General Concept-a'::doY; } } } diff --git a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7b-Variant Configurations.sysml b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7b-Variant Configurations.sysml index 7498e6dc..41e94c1b 100644 --- a/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7b-Variant Configurations.sysml +++ b/SysML2.NET.Serializer.TextualNotation.Tests/Expected/07-Variant Configuration/7b-Variant Configurations.sysml @@ -70,7 +70,7 @@ package '7b-Variant Configurations' { variant part '4cylEngine': '4CylEngine'; variant part '6cylEngine': '6CylEngine'; } - satisfy engineRqtChoice by engineChoice; + assert satisfy engineRqtChoice by engineChoice; assert constraint 'engine choice constraint' { if engineRqtChoice == engineRqtChoice::highPerformanceRequirement ? engineChoice == engineChoice::'6cylEngine' else engineChoice == engineChoice::'4cylEngine' } variation part transmissionChoice :>> transmission { variant part manualTransmission: ManualTransmission { @@ -90,7 +90,7 @@ package '7b-Variant Configurations' { variant part narrowRimWheel: NarrowRimWheel; variant part wideRimWheel: WideRimWheel; } - assert constraint 'engine-wheel selection constraint' { (engineChoice == engineChoice::'4cylEngine' and rearWheelChoice -> forAll { in ref w; w == rearWheelChoice::narrowRimWheel }) xor (engineChoice == engineChoice::'6cylEngine' and rearWheelChoice -> forAll { in ref w; w == rearWheelChoice::wideRimWheel }) } + assert constraint 'engine-wheel selection constraint' { (engineChoice == engineChoice::'4cylEngine' and rearWheelChoice -> forAll { in w; w == rearWheelChoice::narrowRimWheel }) xor (engineChoice == engineChoice::'6cylEngine' and rearWheelChoice -> forAll { in w; w == rearWheelChoice::wideRimWheel }) } } } variation part vehicleChoice :> anyVehicleConfig { diff --git a/SysML2.NET.Serializer.TextualNotation/Writers/NameResolutionCache.cs b/SysML2.NET.Serializer.TextualNotation/Writers/NameResolutionCache.cs index 665b5bb1..f1f9fdde 100644 --- a/SysML2.NET.Serializer.TextualNotation/Writers/NameResolutionCache.cs +++ b/SysML2.NET.Serializer.TextualNotation/Writers/NameResolutionCache.cs @@ -34,6 +34,7 @@ namespace SysML2.NET.Serializer.TextualNotation.Writers using SysML2.NET.Core.POCO.Root.Elements; using SysML2.NET.Core.POCO.Root.Namespaces; using SysML2.NET.Core.POCO.Systems.Actions; + using SysML2.NET.Core.POCO.Systems.DefinitionAndUsage; using SysML2.NET.Core.Root.Namespaces; using SysML2.NET.Extensions; @@ -935,6 +936,21 @@ private INamespace QueryRedefinedFeatureScope(IRedefinition redefinition, IEleme return null; } + // A variant Usage must directly or indirectly specialize its owning variation — SysML v2 + // §8.3.6.4, checkUsageVariationUsageSpecialization ("If a Usage has an owningVariationUsage, + // then it must directly or indirectly specialize that Usage") and its Definition counterpart. + // That Specialization is IMPLIED, so it is absent from a model exported without implied + // relationships; without adding the variation scope here, a redefinition of a member the + // variant inherits THROUGH the variation cannot shorten and degrades to a fully qualified + // name. Appended last so declared supertypes keep priority. + if (owningType.owningMembership is IVariantMembership + && owningType.owningNamespace is { } owningVariation + && !ReferenceEquals(owningVariation, owningType) + && !generalScopes.Contains(owningVariation)) + { + generalScopes.Add(owningVariation); + } + if (generalScopes.Count == 0) { return null; @@ -1538,9 +1554,23 @@ private static bool IsVisibleWhenGlobal(IRelationship relationship, bool isGloba } /// - /// Indexes the entries inherited from 's transitive supertypes. Deliberately - /// bypasses the RemoveRedefinedFeatures filter so :>> references stay reachable; - /// namespace supertypes are enqueued as scopes in their own right. + /// Indexes the entries inherited from 's transitive supertypes; namespace + /// supertypes are enqueued as scopes in their own right. + /// + /// KNOWN DIVERGENCE from Type::inheritedMembership (KerML §8.3.3.1.10): this walk flattens + /// the hierarchy and applies only removeRedefinedFeatures condition 2, at the leaf type + /// alone, so a membership an intermediate supertype redefined away still reaches this index. It + /// also admits private supertype members and misses their public/protected + /// imports. The SDK's type.inheritedMembership is spec-faithful on all three counts and is + /// verified for the transitive case by + /// TypeExtensionsTestFixture.VerifyComputeInheritedMembershipsOperation, so it is the + /// intended replacement, and delegating to it is a small, well-understood diff. It was attempted + /// and backed out for COST, not correctness: inheritedMembership recomputes the transitive + /// closure on every access (no memoisation) and this method runs once per indexed Type, which took + /// the textual-notation validation fixture from 17 s to over 4 minutes (measured back-to-back on an + /// otherwise idle machine). Delegating therefore needs a memoisation layer first — either inside + /// TypeExtensions or as a per-Type memo held by this cache. + /// /// /// The type whose inherited memberships are indexed. /// The destination index. From 64fb9c6a56bdb588d3dce58fdbf3fe68a07ae5a2 Mon Sep 17 00:00:00 2001 From: atheate Date: Thu, 13 Aug 2026 10:59:30 +0200 Subject: [PATCH 3/3] Fix SQ issues --- .../RuleProcessor.PatternHandlers.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.PatternHandlers.cs b/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.PatternHandlers.cs index 40e86d69..4e456a2e 100644 --- a/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.PatternHandlers.cs +++ b/SysML2.NET.CodeGenerator/HandleBarHelpers/RuleProcessor.PatternHandlers.cs @@ -42,6 +42,12 @@ namespace SysML2.NET.CodeGenerator.HandleBarHelpers /// internal sealed partial class RuleProcessor { + /// + /// Upper bound on a single regular-expression match, guarding against catastrophic backtracking + /// on a pathological OCL body. + /// + private const int MatchTimeoutMilliseconds = 1000; + /// /// Pattern B: detects operator-literal alternations and generates a switch on the operator property. /// @@ -1075,7 +1081,9 @@ private void ProcessUnitypedAlternativesWithOneElement(EncodedTextWriter writer, /// Matches an OCL constraint body of the shape <antecedent> implies <consequent> /// where both operands are bare property names. /// - private static readonly Regex SimpleImplicationConstraint = new(@"^\s*(\w+)\s+implies\s+(\w+)\s*$", RegexOptions.Compiled); + /// The compile-time generated . + [GeneratedRegex(@"^\s*(\w+)\s+implies\s+(\w+)\s*$", RegexOptions.None, MatchTimeoutMilliseconds)] + private static partial Regex SimpleImplicationConstraint(); /// /// Reorders mutually-exclusive boolean ?= assignment alternatives so that, whenever the @@ -1113,7 +1121,7 @@ private static List OrderAssignmentsByImplicationConstraints( .SelectMany(owner => owner.OwnedRule) .SelectMany(rule => rule.Specification?.OfType() ?? []) .SelectMany(specification => specification.Body ?? []) - .Select(body => SimpleImplicationConstraint.Match(body ?? string.Empty)) + .Select(body => SimpleImplicationConstraint().Match(body ?? string.Empty)) .Where(match => match.Success) .Select(match => (Antecedent: match.Groups[1].Value, Consequent: match.Groups[2].Value)); @@ -1238,7 +1246,7 @@ private static bool IsTypeDispatcherRule(TextualNotationRule rule) /// The used to resolve rule targets /// The current /// The arms to emit first, most-derived first; empty when no arm is shadowed - private static List<(NonTerminalElement RuleElement, IClass UmlClass)> CollectShadowedNestedRuleTargets(IReadOnlyList<(NonTerminalElement RuleElement, IClass UmlClass)> orderedElements, IReadOnlyDictionary whenGuards, NonTerminalElement defaultRuleElement, IXmiElementCache cache, RuleGenerationContext ruleGenerationContext) + private static List<(NonTerminalElement RuleElement, IClass UmlClass)> CollectShadowedNestedRuleTargets(List<(NonTerminalElement RuleElement, IClass UmlClass)> orderedElements, Dictionary whenGuards, NonTerminalElement defaultRuleElement, IXmiElementCache cache, RuleGenerationContext ruleGenerationContext) { var hoisted = new List<(NonTerminalElement RuleElement, IClass UmlClass)>(); @@ -1324,7 +1332,7 @@ private static void CollectReachableTargetClasses(string ruleName, IXmiElementCa reachableClasses.Add(targetClass); } - if (!IsTypeDispatcherRule(rule)) + if (rule == null || !IsTypeDispatcherRule(rule)) { return; }