diff --git a/.cspell.yaml b/.cspell.yaml index e2613f40..8896c91b 100644 --- a/.cspell.yaml +++ b/.cspell.yaml @@ -30,6 +30,7 @@ words: - docversion - errorlevel - fileassert + - boxless - frameless - Fruchterman - Hanan diff --git a/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md b/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md index 23f8c628..9e115bf6 100644 --- a/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md +++ b/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md @@ -32,7 +32,64 @@ be declared in, since composition structure and namespace/file organization are SysML v2), and assembles the root container box with the interior content nested as that box's own `Children` (mirroring the nesting `MakePartBox` already uses for a container part, so the root box is never a bare sibling of its own content) into the `LayoutTree`. -Returns a minimal 200×100 empty `LayoutTree` when no root or no parts are found. +When `FindRoot` selects no root, falls back to the **no-single-root scoped fallback** described +below when the resolved scope directly includes one or more top-level `part` feature usages; +otherwise returns a minimal 200×100 empty `LayoutTree`. + +###### No-single-root scoped fallback (`CollectTopLevelScopedParts`, `ResolveTopLevelConnections`) + +Not every resolved `expose` scope names a single `part def` worth treating as "the" subject of the +diagram: per SysML v2 spec §8.3.26.11 (an InterconnectionView's subject need not be one definition) +and §9.2.20.2.6 ("exposed features as nodes, nested features as nested nodes"), the exposed content +can be one or more concrete `part` feature usages directly, with no enclosing definition of its own +— e.g. `expose PublishingSubsystem::*;` where `PublishingSubsystem` is itself only a namespace-like +`part def` whose sole nested `part markdownFormatter : MarkdownFormatter;` is the only thing worth +drawing. Before this fallback existed, `FindRoot` returning `null` always produced a totally empty +canvas in this shape, even though the scope named something concrete. + +When `FindRoot` returns `null` and a scope is resolved, `BuildLayout` calls +`CollectTopLevelScopedParts(workspace, scope, theme, defsByName)`, which scans +`workspace.Declarations` for every non-standard-library `SysmlFeatureNode` with +`FeatureKeyword == "part"` whose qualified name satisfies `ExposeScopeResolver.IsInSubjectScope`, +excludes any matched feature that is itself nested (`"::"`-prefixed) under another matched +feature's own qualified name (it is already reachable as that ancestor's own nested part, so must +not also be duplicated as a separate top-level node), and builds a `PartItem` for each survivor via +`BuildPartItem` — the same container-vs-leaf recursion `CollectParts` uses for every other nested +part, extracted so the logic is never duplicated. When at least one top-level part is found, +`BuildPartIndex` and `ResolveTopLevelConnections` (a `ResolveConnections` analogue that scans every +definition's own connections, since a top-level connection may be declared inside any definition in +the workspace, keeping only a connection whose own qualified name is itself in scope and whose +endpoints both resolve into the top-level part set) resolve any connections between the top-level +parts, and `LayOutInteriorWithConnections` is called directly with `boxDepth: 0` and +`reserveTitleArea: false` — producing boxless top-level nodes placed side by side by the same +`LayeredPlacement.PlaceWithPorts` containment-packing algorithm used everywhere else, with no +enclosing frame/title reserved. The resulting `LayoutTree.Nodes` therefore holds the placed part +boxes (and any ports/lines) directly as top-level siblings, instead of the usual single root +container box. When `CollectTopLevelScopedParts` returns no parts (no scope, or a scope matching no +`part` feature), the original minimal 200×100 empty canvas is preserved unchanged. + +###### `BuildPartItem(feature, theme, depth, defsByName, visited)` + +Extracted from `CollectParts`'s per-feature loop body: resolves the feature's `FeatureTyping` +against `defsByName` via `TryResolveContainer`, recursing into `LayOutInterior` at `depth + 1` for a +container part (with the resolved child's qualified name added to a copy of `visited`, and +`scope: null` — nested composition structure is never re-scoped, matching `CollectParts`'s own +documented recursion behavior) or computing an intrinsic leaf size via `ComputePartSize` otherwise. +Both `CollectParts` (depth > 0 or an already-scope-filtered depth-0 feature) and +`CollectTopLevelScopedParts` (a scope-matched top-level feature, always at `depth: 0`) call this one +method, so the container-vs-leaf recursion is defined exactly once. + +###### `LayOutInteriorWithConnections(parts, pairs, theme, boxDepth, reserveTitleArea = true)` + +Places `parts` and routes `pairs` via `LayeredPlacement.PlaceWithPorts`, unchanged from before this +feature except for two now-parameterized behaviors that previously assumed an enclosing container +box always exists: `boxDepth` is stamped directly onto each placed part's own `LayoutBox.Depth` +(the existing `LayOutInterior` call site passes `depth + 1`, preserving today's exact depth +numbering; the no-single-root fallback passes `0` directly, since there is no enclosing container +box in that path at all), and `reserveTitleArea` (default `true`, unchanged for every existing +caller) gates whether a title band is reserved above the placed content — `false` only for the +boxless fallback, where there is no enclosing frame/title to make room for, so the returned size is +just the bounding box of the placed content plus normal padding on every side. ###### Recursive nested layout (`LayOutInterior`, `CollectParts`, `BuildDefinitionIndex`) @@ -184,9 +241,11 @@ container so every connector waypoint is enclosed, without ever moving a box. ##### Error Handling Null `context` or `options` arguments throw `ArgumentNullException`. The absence of an eligible -part definition or of nested parts is not an error: the method returns the minimal empty canvas. -Connectors that cannot be routed cleanly are still drawn; this strategy does not itself construct -`LayoutWarnings` diagnostics, so the returned `LayoutTree` carries no layout-quality warnings. +part definition or of nested parts is not an error: the method returns the minimal empty canvas, +unless the resolved scope directly includes one or more top-level `part` feature usages, in which +case the no-single-root scoped fallback renders those instead (see above). Connectors that cannot +be routed cleanly are still drawn; this strategy does not itself construct `LayoutWarnings` +diagnostics, so the returned `LayoutTree` carries no layout-quality warnings. ##### Dependencies diff --git a/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml b/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml index 45332395..539f09ab 100644 --- a/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml +++ b/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml @@ -251,3 +251,56 @@ sections: part. tests: - InterconnectionView_BuildLayout_PartWithPorts_PortsNeverOverlapBoxTitleArea + + - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-ScopedTopLevelFeatureFallback + title: >- + When no candidate root definition is scope-relevant but the resolved expose scope + directly includes one or more top-level part feature usages, InterconnectionViewLayoutStrategy + shall render those features as boxless nodes side by side instead of an empty canvas, + recursing into each feature's own interior exactly as a normal nested container part + would. + justification: | + Per SysML v2 spec §8.3.26.11 and §9.2.20.2.6, an InterconnectionView's exposed content + need not be a single definition — a scope such as `expose PublishingSubsystem::*;` where + `PublishingSubsystem` is itself only a namespace-like `part def` with a single nested + `part` feature usage names something concrete to draw even though no single `part def` + qualifies as "the" root. Before this fallback, `FindRoot` returning no root always + produced a totally empty canvas in this shape, silently discarding a valid diagram. Boxless + side-by-side placement (reusing the same `LayeredPlacement.PlaceWithPorts` containment + packing every other case already uses) and reusing the same container-vs-leaf recursion + `CollectParts` uses for every other nested part (via the shared `BuildPartItem` helper) + avoids duplicating any layout or recursion logic for this fallback. + tests: + - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoRootDef_RendersTopLevelFeatureWithoutFrame + - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TwoTopLevelParts_ArrangesSideBySideNoFrame + - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TopLevelFeatureIsContainer_RecursesInterior + - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NestedTopLevelFeatureExcluded_NotDuplicated + + - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-ScopedTopLevelFeatureFallbackConnections + title: >- + When two or more boxless top-level parts are connected by a connection whose own + qualified name is itself within the resolved expose scope, InterconnectionViewLayoutStrategy + shall draw a connector between them; otherwise no edges are drawn for the fallback. + justification: | + A connector between two top-level exposed features is a rare but valid shape a resolved + `expose` scope can name; if it is itself resolvable and in scope, it must be drawn so the + diagram does not silently omit a relationship the model actually declares. This reuses the + same endpoint-resolution logic (`ResolveEndpoint`) as every other connection in this + strategy. + tests: + - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_ConnectionBetweenTopLevelFeatures_DrawsEdge + + - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-ScopedTopLevelFeatureFallbackEmptyCanvas + title: >- + When no candidate root definition is scope-relevant and the resolved expose scope + matches no part feature usage, InterconnectionViewLayoutStrategy shall return the + existing minimal empty canvas, unchanged. + justification: | + The no-single-root scoped fallback must never regress the pre-existing empty-canvas + behavior for a scope (or absence of scope) that genuinely names nothing concrete to + draw — for example a scope whose only member is a non-part feature. Preserving the exact + `new LayoutTree(200.0, 100.0, [])` result in that case keeps every other unit that + consumes an Interconnection View's `LayoutTree` unaffected. + tests: + - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoMatchingFeature_ReturnsMinimalCanvas + - InterconnectionView_BuildLayout_EmptyWorkspace_ReturnsMinimalCanvas diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index 2593f80c..6f11fb85 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -237,6 +237,14 @@ children (parts, states, actions, or lifelines) to those within the resolved sco no `expose` statement (including the `--auto`-synthesized view) renders unchanged, exactly as before this scoping behavior was introduced, for every view kind. +For an Interconnection View specifically, a scope that names no single root definition is not +always an empty diagram: when the scope directly includes one or more concrete top-level `part` +feature usages instead — for example `expose Subsystem::*;` where `Subsystem` is itself only a +namespace-like `part def` whose only nested content is a single `part` feature usage, so no +single `part def` qualifies as "the" root — those feature usages render directly, side by side, +with no enclosing frame around them. A scope that matches neither a root definition nor any +top-level `part` feature usage still renders the empty diagram described above. + Named `view Name { ... }` usages (not just `view def` declarations) are also now recognized as their own renderable declarations: a workspace containing both `view def` declarations and named `view` usages surfaces both kinds as views the `render` command discovers and renders. diff --git a/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md b/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md index 96440e41..fa7583ff 100644 --- a/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md +++ b/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md @@ -27,6 +27,19 @@ heuristic. A further test confirms every left/right port sits below its own box' reservation activates for it. No mocking is required; the strategy depends only on the in-memory model, `LayeredPlacement`, and render options. +A dedicated set of tests exercises the no-single-root scoped fallback: a workspace shaped exactly +like the reported bug (a namespace-like `part def` whose only nested content is a single `part` +feature usage, exposed via `expose Namespace::*;`) asserts the previously-empty canvas now renders +that feature directly, with no frame for the enclosing namespace anywhere in the tree; a two-sibling +variant asserts two independent top-level parts render as two non-overlapping top-level boxes; a +container-typed top-level feature variant asserts the recursion into its own nested parts still +fires (proving `BuildPartItem` is shared, not duplicated); a connection-between-top-level-parts +variant asserts a connector is drawn when the connection's own qualified name is itself in scope; +and a nested-qualified-name variant asserts a matched feature nested under another matched +feature's own qualified name is excluded from the top-level set (not duplicated). A final +regression test pins down the preserved empty-canvas fallback when the scope matches no `part` +feature at all. + ##### Test Environment Tests run via `dotnet test` against net8.0, net9.0, and net10.0. No external services, files, or @@ -85,6 +98,20 @@ configuration are required beyond a standard .NET SDK installation. connection count — the layered algorithm's automatic title-vs-side-port reservation, activated by flagging every part node `HasLabel: true, HasKeyword: true`, keeps ports clear of the box's own "«keyword» / name : type" header row. +- When `FindRoot` selects no root but the resolved `expose` scope directly includes a top-level + `part` feature usage (e.g. `expose Namespace::*;` where `Namespace` is only a `part def` with one + nested `part`), that feature renders directly as a boxless node — `LayoutTree.Nodes` holds exactly + one box, with no frame labeled for the enclosing namespace anywhere in the tree. +- Two independent top-level scoped parts render as two non-overlapping top-level boxes with no + wrapping frame. +- A top-level scoped feature that is itself typed by a container definition still recurses into its + own nested parts, which appear as that top-level box's own `Children`. +- A connection between two top-level scoped parts is drawn as a connector line when the connection's + own qualified name is itself within the resolved scope. +- A matched top-level feature whose qualified name is nested under another matched feature's own + qualified name is excluded from the top-level set (not duplicated as its own separate node). +- When the resolved `expose` scope matches no `part` feature usage at all, the pre-existing minimal + empty canvas is returned unchanged. ##### Test Scenarios @@ -114,3 +141,9 @@ configuration are required beyond a standard .NET SDK installation. | `InterconnectionView_BuildLayout_ExposedUsage_ResolvesThroughTypingToRoot` | Usage resolves via `Typing` to root | | `InterconnectionView_BuildLayout_ExposeInnerPartOfNestedDefinition_SelectsNestedDefinitionNotAncestor` | Nested wins | | `InterconnectionView_BuildLayout_ExposeBothSameDepthSiblings_ScoreBreaksTieNotLength` | Score breaks the tie | +| `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoRootDef_RendersTopLevelFeatureWithoutFrame` | Alone | +| `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TwoTopLevelParts_ArrangesSideBySideNoFrame` | 2 boxes | +| `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TopLevelFeatureIsContainer_RecursesInterior` | Nested | +| `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoMatchingFeature_ReturnsMinimalCanvas` | No match | +| `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_ConnectionBetweenTopLevelFeatures_DrawsEdge` | Linked | +| `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NestedTopLevelFeatureExcluded_NotDuplicated` | Dedup | diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs index 1f69def5..c6003f05 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs @@ -245,9 +245,10 @@ private static bool IsDirectChildOf(string qualifiedName, string container) /// , , or /// would otherwise pick by its own heuristic) is related /// to the resolved expose scope in , in either containment - /// direction: the candidate itself is an exposed subject/matched member, the candidate lies - /// within an exposed subject's containment subtree, or an exposed subject/matched member lies - /// within the candidate's own containment subtree (the common "expose an inner + /// direction: the candidate itself is an exposed subject/matched member (subject to the same + /// per-recursion-kind self-exclusion applies — see below), the + /// candidate lies within an exposed subject's containment subtree, or an exposed subject/matched + /// member lies within the candidate's own containment subtree (the common "expose an inner /// state/action/part/lifeline of the root" case). /// /// @@ -258,6 +259,17 @@ private static bool IsDirectChildOf(string qualifiedName, string container) /// must break the tie themselves; the single-root FindRoot strategies do so via /// , which prefers the most deeply nested relevant candidate /// over the plain per-strategy score. + /// + /// The "candidate equals the subject" direction only applies when the subject's + /// kind actually includes the subject itself + /// ( or + /// ). Per formal-26-03-02.md §8.3.26.4, a + /// NamespaceExpose ( and + /// ) exposes the subject's + /// Memberships — its members — not the subject itself, so a candidate that is only reachable + /// via "candidate == subject" for a namespace-form subject must not be treated as root-relevant; + /// otherwise the excluded subject would still be drawn as the diagram's outer box. + /// /// /// The candidate root definition's qualified name. /// The resolved expose scope. @@ -267,12 +279,33 @@ private static bool IsDirectChildOf(string qualifiedName, string container) /// public static bool IsRootRelevantToScope(string candidateQualifiedName, ExposedScope scope) { - bool RelevantTo(string subject) => - candidateQualifiedName == subject || - candidateQualifiedName.StartsWith(subject + "::", StringComparison.Ordinal) || - subject.StartsWith(candidateQualifiedName + "::", StringComparison.Ordinal); + bool RelevantToSubject(ExposeSubject subject) + { + var subjectName = subject.QualifiedName; + + // Containment in either direction is always relevant, regardless of recursion kind: + // the candidate descending from the subject, or an exposed subject/feature nested + // inside the candidate. + if (candidateQualifiedName.StartsWith(subjectName + "::", StringComparison.Ordinal) || + subjectName.StartsWith(candidateQualifiedName + "::", StringComparison.Ordinal)) + { + return true; + } + + // "candidate == subject" is only relevant when this subject's recursion kind actually + // includes the subject itself in scope -- NamespaceRecursive/NamespaceDirectChildren + // exclude the subject (see remarks), so an excluded subject must not be root-relevant + // purely by virtue of matching itself. + return candidateQualifiedName == subjectName && + subject.Recursion is ExposeRecursionKind.MembershipRecursive or ExposeRecursionKind.MembershipExact; + } + + bool RelevantToMember(string member) => + candidateQualifiedName == member || + candidateQualifiedName.StartsWith(member + "::", StringComparison.Ordinal) || + member.StartsWith(candidateQualifiedName + "::", StringComparison.Ordinal); - return scope.Subjects.Select(s => s.QualifiedName).Any(RelevantTo) || scope.ExplicitMembers.Any(RelevantTo); + return scope.Subjects.Any(RelevantToSubject) || scope.ExplicitMembers.Any(RelevantToMember); } /// diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs index 2a4449eb..11174d79 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs @@ -41,6 +41,16 @@ namespace DemaConsulting.SysML2Tools.Layout.Internal; /// non-recursive layout. Recursion is driven here, at the strategy level, because container detection /// is a semantic-model concern the model-independent algorithm cannot see. /// +/// +/// Not every resolved expose scope names a single part def worth treating as "the" +/// subject: per SysML v2 §8.3.26.11/§9.2.20.2.6, an InterconnectionView's exposed content can be one +/// or more concrete feature usages directly, with no enclosing definition of its own. When +/// selects no root but the scope directly includes one or more top-level +/// part feature usages, those features are rendered as boxless nodes side by side (via +/// ) instead of the diagram falling back to an empty canvas +/// — each one recursing into its own interior exactly as a normal nested container part would, +/// reusing so the container-vs-leaf logic is never duplicated. +/// /// internal sealed class InterconnectionViewLayoutStrategy : ILayoutStrategy { @@ -103,6 +113,27 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) var root = FindRoot(context.Workspace, scope, defsByName); if (root is null) { + // No single part def qualifies as "the" root. Per SysML v2 §8.3.26.11/§9.2.20.2.6, an + // InterconnectionView's subject need not be one definition — when the resolved expose + // scope directly names one or more concrete part feature usages, render those as + // boxless nodes side by side (via the same sensible existing layout helper, + // LayeredPlacement.PlaceWithPorts, used for every other case) instead of falling back to + // a totally empty canvas. When there is nothing concrete to draw either (no scope at + // all, or a scope that matches no part feature), the existing empty-canvas fallback is + // preserved unchanged. + if (scope is not null) + { + var topLevelParts = CollectTopLevelScopedParts(context.Workspace, scope, theme, defsByName); + if (topLevelParts.Count > 0) + { + var partIndex = BuildPartIndex(topLevelParts); + var pairs = ResolveTopLevelConnections(defsByName, partIndex, scope); + var layout = LayOutInteriorWithConnections( + topLevelParts, pairs, theme, boxDepth: 0, reserveTitleArea: false); + return new LayoutTree(layout.Width, layout.Height, layout.Content); + } + } + return new LayoutTree(200.0, 100.0, []); } @@ -170,18 +201,35 @@ private static InteriorLayout LayOutInterior( var partIndex = BuildPartIndex(parts); var pairs = ResolveConnections(def, partIndex); - return LayOutInteriorWithConnections(parts, pairs, theme, depth); + return LayOutInteriorWithConnections(parts, pairs, theme, boxDepth: depth + 1); } /// /// Lays out a definition's parts when at least one connection exists between them, delegating /// placement and orthogonal edge routing to the bundled layered algorithm. /// + /// The parts to place. + /// The resolved connections between the parts. + /// The active rendering theme. + /// + /// The to stamp on each placed part's own box (not the + /// container's — the caller passes its own container depth, plus one, for a normal nested + /// interior; the no-single-root fallback in passes 0 directly + /// since there is no enclosing container box at all in that path). + /// + /// + /// Whether to reserve a title band above the placed content, as a normal container box's own + /// title requires. (the default) for every existing container-interior + /// caller; only for the no-single-root boxless fallback in + /// , where there is no enclosing frame/title to make room for — the + /// returned size is then just the bounding box of the placed content plus normal padding. + /// private static InteriorLayout LayOutInteriorWithConnections( IReadOnlyList parts, IReadOnlyList pairs, Theme theme, - int depth) + int boxDepth, + bool reserveTitleArea = true) { var nodeSizes = parts.Select(p => (p.Width, p.Height, HasLabel: true, HasKeyword: true)).ToList(); @@ -205,8 +253,10 @@ private static InteriorLayout LayOutInteriorWithConnections( // independently-routed connector instead of collapsing onto one shared route. var placed = LayeredPlacement.PlaceWithPorts(nodeSizes, portEdges, LayoutFlowDirection.Right); - // Shift placed content down/right to sit inside the container box. - var titleArea = BoxMetrics.TitleAreaHeight(theme, hasLabel: true, hasKeyword: true); + // Shift placed content down/right to sit inside the container box. When there is no + // enclosing container (the boxless top-level fallback), no title band is reserved, so the + // top offset collapses to the same padding-only inset used on every other side. + var titleArea = reserveTitleArea ? BoxMetrics.TitleAreaHeight(theme, hasLabel: true, hasKeyword: true) : 0.0; var offsetX = theme.LabelPadding * 2.0; var offsetY = titleArea + (theme.LabelPadding * 2.0); @@ -233,7 +283,7 @@ private static InteriorLayout LayOutInteriorWithConnections( for (var i = 0; i < parts.Count; i++) { var r = placed.Rects[i]; - content.Add(MakePartBox(parts[i], new Rect(r.X + offsetX, r.Y + offsetY, r.Width, r.Height), depth + 1)); + content.Add(MakePartBox(parts[i], new Rect(r.X + offsetX, r.Y + offsetY, r.Width, r.Height), boxDepth)); } // One port pair and one connector line per connection. The algorithm returns exactly one @@ -430,29 +480,158 @@ private static IReadOnlyList CollectParts( continue; } - var name = feature.Name ?? feature.FeatureTyping ?? "part"; + result.Add(BuildPartItem(feature, theme, depth, defsByName, visited)); + } + + return result; + } + + /// + /// Builds a single nested part usage's , recursing into its interior when + /// the part's type resolves to a container definition (a non-stdlib part def with its own + /// internal parts, not already on the recursion path) and computing an intrinsic leaf size + /// otherwise. Extracted from so the same container-vs-leaf recursion + /// is reused verbatim by — the boxless top-level + /// fallback path must lay out each top-level feature exactly as a normal nested part would be, + /// per the "no duplicated logic" coding principle. + /// + /// The part feature usage to build a for. + /// The active rendering theme. + /// Nesting depth of the feature's own container box (0 for a top-level part). + /// Container-definition index keyed by qualified and simple name. + /// Qualified names already on the recursion path, guarding against cycles. + /// The built , container or leaf. + private static PartItem BuildPartItem( + SysmlFeatureNode feature, + Theme theme, + int depth, + IReadOnlyDictionary defsByName, + ISet visited) + { + var name = feature.Name ?? feature.FeatureTyping ?? "part"; + + if (TryResolveContainer(feature.FeatureTyping, defsByName, visited, out var childDef)) + { + // Container part: lay out its interior bottom-up and treat it as an atomic node. + // Scope is intentionally not carried into this recursive call — see the remarks on + // CollectParts for why nested composition structure is always shown once its owning + // part has been included, regardless of the exposed namespace scope. + var childVisited = new HashSet(visited, StringComparer.Ordinal) { childDef.QualifiedName! }; + var inner = LayOutInterior(childDef, theme, depth + 1, defsByName, childVisited, scope: null); + return new PartItem(name, "part", feature.FeatureTyping, inner.Width, inner.Height, inner.Content); + } + + // Leaf part: intrinsic size, no nested content. + var (width, height) = ComputePartSize(name, feature.FeatureTyping, theme); + return new PartItem(name, "part", feature.FeatureTyping, width, height, null); + } + + /// + /// Collects every top-level part feature usage the resolved expose scope directly + /// includes, for the no-single-root fallback path: found no part def + /// worth rendering as a container, but the scope itself names one or more concrete features to + /// draw. Per SysML v2 spec §9.2.20.2.6 ("exposed features as nodes, nested features as nested + /// nodes") and §8.3.26.11 (an InterconnectionView's subject need not be a single definition), + /// each matching feature is rendered directly as its own top-level node rather than the diagram + /// falling back to an empty canvas. + /// + /// The workspace, scanned for every non-stdlib part feature usage. + /// The view's resolved expose containment-subtree scope. + /// The active rendering theme. + /// Container-definition index keyed by qualified and simple name. + /// + /// The matched top-level parts, each recursively laid out via exactly + /// as a normal nested container part would be, in 's declaration + /// order. Empty when no non-stdlib part feature satisfies the scope. + /// + private static IReadOnlyList CollectTopLevelScopedParts( + SysmlWorkspace workspace, + ExposedScope scope, + Theme theme, + IReadOnlyDictionary defsByName) + { + var matched = new List(); + foreach (var (qualifiedName, node) in workspace.Declarations) + { + if (node is not SysmlFeatureNode feature || feature.FeatureKeyword != "part") + { + continue; + } - if (TryResolveContainer(feature.FeatureTyping, defsByName, visited, out var childDef)) + if (StdlibFilter.IsStdlibElement(qualifiedName, workspace.StdlibNames)) { - // Container part: lay out its interior bottom-up and treat it as an atomic node. - // Scope is intentionally not carried into this recursive call — see the remarks - // above on why nested composition structure is always shown once its owning part - // has been included, regardless of the exposed namespace scope. - var childVisited = new HashSet(visited, StringComparer.Ordinal) { childDef.QualifiedName! }; - var inner = LayOutInterior(childDef, theme, depth + 1, defsByName, childVisited, scope: null); - result.Add(new PartItem(name, "part", feature.FeatureTyping, inner.Width, inner.Height, inner.Content)); + continue; } - else + + if (!ExposeScopeResolver.IsInSubjectScope(qualifiedName, scope)) { - // Leaf part: intrinsic size, no nested content. - var (width, height) = ComputePartSize(name, feature.FeatureTyping, theme); - result.Add(new PartItem(name, "part", feature.FeatureTyping, width, height, null)); + continue; } + + matched.Add(feature); + } + + // Exclude any matched feature nested ("::"-prefixed) under another matched feature's own + // qualified name: it is already reachable as that ancestor's own nested part (rendered via + // the recursive BuildPartItem call below), so it must not also appear as its own separate + // top-level node. + var matchedNames = matched.Select(f => f.QualifiedName).Where(n => n is { Length: > 0 }).ToHashSet(StringComparer.Ordinal); + var topLevel = matched + .Where(f => f.QualifiedName is not { Length: > 0 } fqn || + !matchedNames.Any(other => other != fqn && fqn.StartsWith(other + "::", StringComparison.Ordinal))) + .ToList(); + + var result = new List(); + foreach (var feature in topLevel) + { + var visited = new HashSet(StringComparer.Ordinal); + result.Add(BuildPartItem(feature, theme, depth: 0, defsByName, visited)); } return result; } + /// + /// Resolves connections between the boxless top-level parts collected by + /// , reusing verbatim. + /// Unlike (which only looks at one definition's own direct + /// children), a top-level connection may be declared inside any definition in the workspace, so + /// every definition's connections are scanned; a connection is only drawn when its own + /// QualifiedName is itself in scope and both endpoints resolve into the top-level + /// part set — an incidental connection between unrelated parts must never be surfaced just + /// because it happens to share a name with one of the rendered top-level features. + /// + /// Container-definition index keyed by qualified and simple name. + /// Name → index lookup for the collected top-level parts. + /// The view's resolved expose containment-subtree scope. + /// The resolved connection pairs between top-level parts, if any. + private static IReadOnlyList ResolveTopLevelConnections( + IReadOnlyDictionary defsByName, + Dictionary partIndex, + ExposedScope scope) + { + var pairs = new List(); + foreach (var def in defsByName.Values.Distinct()) + { + foreach (var conn in def.Children.OfType()) + { + if (conn.QualifiedName is not { Length: > 0 } fqn || !ExposeScopeResolver.IsInSubjectScope(fqn, scope)) + { + continue; + } + + var (a, labelA) = ResolveEndpoint(conn.EndpointA, partIndex); + var (b, labelB) = ResolveEndpoint(conn.EndpointB, partIndex); + if (a >= 0 && b >= 0 && a != b) + { + pairs.Add(new ConnPair(a, b, labelA, labelB)); + } + } + } + + return pairs; + } + /// /// Builds an index of candidate container definitions — non-standard-library part defs /// that have at least one nested part usage — keyed by both qualified and simple name diff --git a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs index 0e600531..ef6f1d33 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs @@ -1402,4 +1402,372 @@ public void InterconnectionView_BuildLayout_UnconnectedPeerParts_PacksIntoCompac Assert.True(distinctX > 1, "Expected parts to span more than one column, not a single vertical column."); Assert.True(distinctY < parts.Count, "Expected some parts to share a row, not one row per part."); } + + /// + /// Regression test for the reported crash/empty-diagram bug: a view exposing a namespace's + /// direct children (expose PublishingSubsystem::*;) where PublishingSubsystem + /// is itself only a namespace-like part def with a single nested part feature + /// usage (markdownFormatter) typed by a leaf part def. FindRoot selects + /// no root (PublishingSubsystem is excluded from self-matching by the + /// NamespaceDirectChildren recursion kind), so before this fix the diagram rendered + /// as a totally empty canvas. Now the exposed direct-child feature renders directly as a + /// single boxless leaf node, with no PublishingSubsystem-labeled frame anywhere. + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoRootDef_RendersTopLevelFeatureWithoutFrame() + { + // Arrange: PublishingSubsystem::markdownFormatter : MarkdownFormatter (a leaf part def), + // exposed via "expose PublishingSubsystem::*;" (NamespaceDirectChildren, no trailing "::**"). + var strategy = new InterconnectionViewLayoutStrategy(); + var publishingSubsystem = new SysmlDefinitionNode + { + Name = "PublishingSubsystem", + QualifiedName = "M::PublishingSubsystem", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode + { + Name = "markdownFormatter", + QualifiedName = "M::PublishingSubsystem::markdownFormatter", + FeatureKeyword = "part", + FeatureTyping = "MarkdownFormatter" + } + ] + }; + var markdownFormatter = new SysmlDefinitionNode + { + Name = "MarkdownFormatter", + QualifiedName = "M::MarkdownFormatter", + DefinitionKeyword = "part def" + }; + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["M::PublishingSubsystem"] = publishingSubsystem, + ["M::PublishingSubsystem::markdownFormatter"] = publishingSubsystem.Children[0], + ["M::MarkdownFormatter"] = markdownFormatter + } + }; + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "M::V", + ExposeMembers = [new ExposeMember("PublishingSubsystem", null, ExposeRecursionKind.NamespaceDirectChildren)], + ResolvedEdges = [new SysmlEdge("M::V", "M::PublishingSubsystem", SysmlEdgeKind.Expose)] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: exactly one box overall (no PublishingSubsystem frame anywhere), a leaf/interior + // rounded box labeled "markdownFormatter : MarkdownFormatter" with keyword "part". + var boxes = CollectBoxes(layout.Nodes).ToList(); + var box = Assert.Single(boxes); + Assert.Equal("part", box.Keyword); + Assert.Contains("markdownFormatter", box.Label, StringComparison.Ordinal); + Assert.Equal(BoxShape.RoundedRectangle, box.Shape); + Assert.DoesNotContain(boxes, b => b.Label is not null && b.Label.Contains("PublishingSubsystem", StringComparison.Ordinal)); + } + + /// + /// Two independent top-level part feature usages that are both direct children of a + /// common exposed namespace render as two separate boxless nodes placed side by side (via + /// the shared LayeredPlacement.PlaceWithPorts containment-packing algorithm), with no + /// wrapping frame and no overlap between them. + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TwoTopLevelParts_ArrangesSideBySideNoFrame() + { + // Arrange: NsRoot::widgetA and NsRoot::widgetB, each typed by an unrelated leaf part def, + // exposed via "expose NsRoot::*;" (NamespaceDirectChildren). + var strategy = new InterconnectionViewLayoutStrategy(); + var workspace = BuildTwoTopLevelPartsWorkspace(); + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "M::V", + ExposeMembers = [new ExposeMember("NsRoot", null, ExposeRecursionKind.NamespaceDirectChildren)], + ResolvedEdges = [new SysmlEdge("M::V", "NsRoot", SysmlEdgeKind.Expose)] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: exactly two top-level boxes (no wrapping frame), and they do not overlap. + Assert.Equal(2, layout.Nodes.Count); + var boxes = layout.Nodes.OfType().ToList(); + Assert.Equal(2, boxes.Count); + Assert.False(Overlaps(boxes[0], boxes[1]), "the two top-level boxes overlap"); + } + + /// + /// When the exposed namespace's direct-child part is itself typed by a part def that + /// has its own nested parts, the single top-level box's own Children contain the + /// nested part boxes — proving the boxless top-level fallback reuses the same recursive + /// container detection (BuildPartItem) as every other nested part, rather than + /// duplicating a shallow copy of that logic. + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TopLevelFeatureIsContainer_RecursesInterior() + { + // Arrange: NsRoot::board : Motherboard { cpu, chipset, connect cpu to chipset } — two levels + // deep — exposed via "expose NsRoot::*;" (NamespaceDirectChildren). + var strategy = new InterconnectionViewLayoutStrategy(); + var motherboard = new SysmlDefinitionNode + { + Name = "Motherboard", + QualifiedName = "M::Motherboard", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "cpu", QualifiedName = "M::Motherboard::cpu", FeatureKeyword = "part", FeatureTyping = "Cpu" }, + new SysmlFeatureNode { Name = "chipset", QualifiedName = "M::Motherboard::chipset", FeatureKeyword = "part", FeatureTyping = "Chipset" }, + new SysmlConnectionNode { ConnectionKeyword = "connection", EndpointA = "cpu", EndpointB = "chipset" } + ] + }; + var board = new SysmlFeatureNode + { + Name = "board", + QualifiedName = "NsRoot::board", + FeatureKeyword = "part", + FeatureTyping = "Motherboard" + }; + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["NsRoot::board"] = board, + ["M::Motherboard"] = motherboard + } + }; + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "M::V", + ExposeMembers = [new ExposeMember("NsRoot", null, ExposeRecursionKind.NamespaceDirectChildren)], + ResolvedEdges = [new SysmlEdge("M::V", "NsRoot", SysmlEdgeKind.Expose)] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: exactly one top-level box (no frame), and its own Children hold the nested + // cpu/chipset part boxes recursed via the shared BuildPartItem logic. + var topBox = Assert.Single(layout.Nodes.OfType()); + Assert.Contains("board", topBox.Label, StringComparison.Ordinal); + var nestedLabels = CollectBoxes(topBox.Children).Select(b => b.Label).ToList(); + Assert.Contains(nestedLabels, l => l is not null && l.Contains("cpu", StringComparison.Ordinal)); + Assert.Contains(nestedLabels, l => l is not null && l.Contains("chipset", StringComparison.Ordinal)); + } + + /// + /// Pins down the preserved existing empty-canvas fallback (requirement: no regression): when + /// the resolved expose scope matches no part feature at all (its only member + /// is a non-part feature), FindRoot still returns no root, and + /// CollectTopLevelScopedParts returns an empty list, so the diagram falls back to the + /// unchanged minimal canvas rather than the new boxless-nodes path. + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoMatchingFeature_ReturnsMinimalCanvas() + { + // Arrange: NsRoot::onlyAttribute is an "attribute" feature, not a "part" — no part + // feature is a direct child of the exposed namespace. + var strategy = new InterconnectionViewLayoutStrategy(); + var onlyAttribute = new SysmlFeatureNode + { + Name = "onlyAttribute", + QualifiedName = "NsRoot::onlyAttribute", + FeatureKeyword = "attribute", + FeatureTyping = "String" + }; + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary { ["NsRoot::onlyAttribute"] = onlyAttribute } + }; + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "M::V", + ExposeMembers = [new ExposeMember("NsRoot", null, ExposeRecursionKind.NamespaceDirectChildren)], + ResolvedEdges = [new SysmlEdge("M::V", "NsRoot", SysmlEdgeKind.Expose)] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: unchanged minimal-canvas fallback. + Assert.Empty(layout.Nodes); + } + + /// + /// A connector declared between two top-level scoped part features is still drawn when the + /// connector's own qualified name is itself in scope and both endpoints resolve into the + /// top-level part set (requirement: "if a connector is itself resolvable and in scope, draw + /// it"). + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_ConnectionBetweenTopLevelFeatures_DrawsEdge() + { + // Arrange: NsRoot { widgetA, widgetB, connect widgetA to widgetB } — the connection's own + // qualified name is a direct child of NsRoot, so it too satisfies the NamespaceDirectChildren + // scope alongside its two endpoints. + var strategy = new InterconnectionViewLayoutStrategy(); + var workspace = BuildTwoTopLevelPartsWithConnectionWorkspace(); + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "M::V", + ExposeMembers = [new ExposeMember("NsRoot", null, ExposeRecursionKind.NamespaceDirectChildren)], + ResolvedEdges = [new SysmlEdge("M::V", "NsRoot", SysmlEdgeKind.Expose)] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: two top-level boxes, connected by exactly one connector line. + Assert.Equal(2, layout.Nodes.OfType().Count()); + Assert.Single(CollectLines(layout.Nodes)); + } + + /// + /// A matched feature whose own qualified name is itself nested ("::"-prefixed) under + /// another matched feature's qualified name is excluded from the top-level set — it is + /// already reachable as that ancestor's own nested content, so it must not also be + /// duplicated as its own separate top-level node. + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NestedTopLevelFeatureExcluded_NotDuplicated() + { + // Arrange: an exotic scope shape where both "NsRoot::widgetA" (the exposed subject itself, + // MembershipRecursive so self-matches) and "NsRoot::widgetA::sub" (nested under it by + // qualified-name prefix alone) satisfy IsInSubjectScope for the same subject. + var strategy = new InterconnectionViewLayoutStrategy(); + var widgetA = new SysmlFeatureNode + { + Name = "widgetA", + QualifiedName = "NsRoot::widgetA", + FeatureKeyword = "part", + FeatureTyping = "WidgetA" + }; + var sub = new SysmlFeatureNode + { + Name = "sub", + QualifiedName = "NsRoot::widgetA::sub", + FeatureKeyword = "part", + FeatureTyping = "SubPart" + }; + var widgetADef = new SysmlDefinitionNode { Name = "WidgetA", QualifiedName = "M::WidgetA", DefinitionKeyword = "part def" }; + var subPartDef = new SysmlDefinitionNode { Name = "SubPart", QualifiedName = "M::SubPart", DefinitionKeyword = "part def" }; + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["NsRoot::widgetA"] = widgetA, + ["NsRoot::widgetA::sub"] = sub, + ["M::WidgetA"] = widgetADef, + ["M::SubPart"] = subPartDef + } + }; + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "M::V", + ExposeMembers = [new ExposeMember("widgetA", null, ExposeRecursionKind.MembershipRecursive)], + ResolvedEdges = [new SysmlEdge("M::V", "NsRoot::widgetA", SysmlEdgeKind.Expose)] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: only the outer (ancestor) feature appears as a top-level box; "sub" is not + // duplicated as its own separate top-level node. + var topBoxes = layout.Nodes.OfType().ToList(); + var topBox = Assert.Single(topBoxes); + Assert.Contains("widgetA", topBox.Label, StringComparison.Ordinal); + Assert.DoesNotContain("sub", topBox.Label, StringComparison.Ordinal); + } + + /// + /// Builds a workspace with two independent top-level parts (NsRoot::widgetA and + /// NsRoot::widgetB), each typed by its own unrelated leaf part def, and no + /// connection between them. + /// + private static SysmlWorkspace BuildTwoTopLevelPartsWorkspace() + { + var widgetA = new SysmlFeatureNode { Name = "widgetA", QualifiedName = "NsRoot::widgetA", FeatureKeyword = "part", FeatureTyping = "WidgetA" }; + var widgetB = new SysmlFeatureNode { Name = "widgetB", QualifiedName = "NsRoot::widgetB", FeatureKeyword = "part", FeatureTyping = "WidgetB" }; + var widgetADef = new SysmlDefinitionNode { Name = "WidgetA", QualifiedName = "M::WidgetA", DefinitionKeyword = "part def" }; + var widgetBDef = new SysmlDefinitionNode { Name = "WidgetB", QualifiedName = "M::WidgetB", DefinitionKeyword = "part def" }; + return new SysmlWorkspace + { + Declarations = new Dictionary + { + ["NsRoot::widgetA"] = widgetA, + ["NsRoot::widgetB"] = widgetB, + ["M::WidgetA"] = widgetADef, + ["M::WidgetB"] = widgetBDef + } + }; + } + + /// + /// Builds the same two-top-level-parts shape as , + /// but with both features and a connector between them nested as children of a common + /// NsRoot part def container — so the connector's own qualified name + /// (NsRoot::conn1) is itself a direct child of the exposed namespace, and the + /// container is scanned by ResolveTopLevelConnections via BuildDefinitionIndex + /// without itself qualifying as the diagram's single root (excluded from self-matching by + /// the NamespaceDirectChildren recursion kind, same as PublishingSubsystem + /// above). + /// + private static SysmlWorkspace BuildTwoTopLevelPartsWithConnectionWorkspace() + { + var widgetADef = new SysmlDefinitionNode { Name = "WidgetA", QualifiedName = "M::WidgetA", DefinitionKeyword = "part def" }; + var widgetBDef = new SysmlDefinitionNode { Name = "WidgetB", QualifiedName = "M::WidgetB", DefinitionKeyword = "part def" }; + var nsRoot = new SysmlDefinitionNode + { + Name = "NsRoot", + QualifiedName = "NsRoot", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "widgetA", QualifiedName = "NsRoot::widgetA", FeatureKeyword = "part", FeatureTyping = "WidgetA" }, + new SysmlFeatureNode { Name = "widgetB", QualifiedName = "NsRoot::widgetB", FeatureKeyword = "part", FeatureTyping = "WidgetB" }, + new SysmlConnectionNode + { + Name = "conn1", + QualifiedName = "NsRoot::conn1", + ConnectionKeyword = "connection", + EndpointA = "widgetA", + EndpointB = "widgetB" + } + ] + }; + return new SysmlWorkspace + { + Declarations = new Dictionary + { + ["NsRoot"] = nsRoot, + ["NsRoot::widgetA"] = nsRoot.Children[0], + ["NsRoot::widgetB"] = nsRoot.Children[1], + ["NsRoot::conn1"] = nsRoot.Children[2], + ["M::WidgetA"] = widgetADef, + ["M::WidgetB"] = widgetBDef + } + }; + } }