More intuitive behavior for FindAllRelationshipsDeep

- Expand all roots
- Add all crossing edges between any root.

Author: ATrefzer

CSharpCodeAnalyst.CodeGraph/Exploration/CodeGraphExplorer.cs

@@ -80,7 +80,6 @@ public SearchResult FindMissingTypesForLonelyTypeMembers(HashSet<string> knownId
     ///     All relationships crossing the element's boundary outwards: the source is the element
     ///     or one of its descendants, the target lies outside. Relationships between two
     ///     descendants are internal and not part of the result.
-    ///     <see cref="FindAllRelationshipsDeep" /> shows those.
     ///     The result contains the start element, the elements taking part in a found relationship
     ///     and the containers connecting those to the start element. The reached targets come
     ///     without their gaps filled. If the caller needs those, it runs
@@ -107,7 +106,6 @@ public SearchResult FindOutgoingRelationshipsDeep(string id)
     ///     All relationships crossing the element's boundary inwards: the target is the element
     ///     or one of its descendants, the source lies outside. Relationships between two
     ///     descendants are internal and not part of the result.
-    ///     <see cref="FindAllRelationshipsDeep" /> shows those.
     ///     The result contains the start element, the elements taking part in a found relationship
     ///     and the containers connecting those to the start element. The found sources come
     ///     without their gaps filled. If the caller needs those, it runs
@@ -167,30 +165,64 @@ public IEnumerable<Relationship> FindAllRelationships(HashSet<string> ids)
         return GetRelationships(d => ids.Contains(d.SourceId) && ids.Contains(d.TargetId));
     }
 
-    public SearchResult FindAllRelationshipsDeep(HashSet<string> ids)
+    /// <summary>
+    ///     All relationships crossing the root element's boundary inwards or outwards.
+    ///     The result however is limited to be contained in the given roots
+    ///     The result contains the start element, the elements taking part in a found relationship
+    ///     and the gap filling elements.
+    /// 
+    ///     FindIncomingRelationshipsDeep / FindOutgoingRelationshipsDeep have an open result. Here,
+    ///     all new nodes are descendants of the root elements.
+    /// </summary>
+    public SearchResult FindAllRelationshipsDeep(HashSet<string> rootIds)
     {
         if (_codeGraph is null)
         {
             return new SearchResult([], []);
         }
 
-        var expandedIds = ids
-            .Where(_codeGraph.Nodes.ContainsKey)
-            .SelectMany(id => _codeGraph.Nodes[id].GetChildrenIncludingSelf())
-            .ToHashSet();
+        // Expanded set per id
+        // To which selected root does an element belong. This may be more than one since
+        // the roots may be nested (e.g. class and namespace are selected. Both contain same method.)
+        var rootsByElement = new Dictionary<string, HashSet<string>>();
+        foreach (var rootId in rootIds.Where(_codeGraph.Nodes.ContainsKey))
+        {
+            foreach (var id in _codeGraph.Nodes[rootId].GetChildrenIncludingSelf())
+            {
+                if (!rootsByElement.TryGetValue(id, out var roots))
+                {
+                    roots = [];
+                    rootsByElement[id] = roots;
+                }
 
-        var relationships = FindAllRelationships(expandedIds).ToList();
+                roots.Add(rootId);
+            }
+        }
+
+        // Are there a source root s and a target root t that are different?” That is exactly the definition of a crossing edge.
+        bool CrossesRoots(Relationship r)
+        {
+            return rootsByElement.TryGetValue(r.SourceId, out var sourceRoots) &&
+                   rootsByElement.TryGetValue(r.TargetId, out var targetRoots) &&
+                   sourceRoots.Any(s => targetRoots.Any(t => s != t));
+        }
+
+        var relationships = _codeGraph.GetAllRelationships().Where(CrossesRoots).ToHashSet();
+
+        // We collected all edges.
         var relationshipElementIds = relationships
             .SelectMany(r => new[] { r.SourceId, r.TargetId })
             .ToHashSet();
 
         // Found elements can be deep inside the given roots (e.g. two methods buried in two
         // selected assemblies). Fill in the namespaces/classes connecting them to a root that is
         // already known, so they don't end up added without their hierarchy.
-        var gapIds = FillGapsInHierarchy(ids.Union(relationshipElementIds).ToHashSet());
+        var gapIds = FillGapsInHierarchy(rootIds.Union(relationshipElementIds).ToHashSet());
 
         var elements = relationshipElementIds
             .Union(gapIds)
+            .Union(rootIds) // to be self-contained
+            .Where(_codeGraph.Nodes.ContainsKey)
             .Select(id => _codeGraph.Nodes[id])
             .ToHashSet();
 
@@ -261,7 +293,6 @@ public SearchResult FindIncomingCallsRecursive(string id)
         return new SearchResult(foundMethods, foundCalls);
     }
 
-
     /// <summary>
     ///     This gives a heuristic only.
     ///     The graph model does not contain the information for analyzing dynamic behavior.
@@ -646,6 +677,7 @@ private SearchResult CollectDeepResult(CodeElement startElement, List<Relationsh
         var elements = relationshipElementIds
             .Union(gapIds)
             .Union([startElement.Id])
+            .Where(_codeGraph!.Nodes.ContainsKey)
             .Select(i => _codeGraph!.Nodes[i])
             .ToHashSet();
 

Tests/UnitTests/Exploration/CodeGraphExplorerTests.cs

@@ -326,7 +326,7 @@ public void FindAllRelationshipsDeep_FindsRelationshipsBetweenDescendantsOfGiven
         Assert.Multiple(() =>
         {
             Assert.That(result.Relationships, Is.EquivalentTo(new[] { callAtoB, callBtoA }));
-            Assert.That(result.Elements.Select(e => e.Id), Is.EquivalentTo([methodA.Id, methodB.Id]));
+            Assert.That(result.Elements.Select(e => e.Id), Is.EquivalentTo([methodA.Id, methodB.Id, classA.Id, classB.Id]));
         });
     }
 
@@ -372,9 +372,40 @@ public void FindAllRelationshipsDeep_FillsHierarchyBetweenRootsAndFoundElements(
             Assert.That(result.Relationships, Is.EquivalentTo(new[] { call }));
 
             // The methods themselves plus the namespaces and classes connecting them to their
-            // (already known) assembly must be included - the assemblies themselves must not.
+            // (already known) assembly must be included
             Assert.That(result.Elements.Select(e => e.Id), Is.EquivalentTo(
-                new[] { nsA.Id, classA.Id, methodA.Id, nsB.Id, classB.Id, methodB.Id }));
+                new[] { nsA.Id, classA.Id, methodA.Id, nsB.Id, classB.Id, methodB.Id, asmA.Id, asmB.Id }));
+        });
+    }
+
+    /// <summary>
+    ///     Nested/overlapping roots: a class and one of its own methods are both selected.
+    ///     The default "complete deep" command selects every element on the canvas, so a
+    ///     container and its children are roots at the same time - this is the normal case,
+    ///     not an edge case.
+    ///     An element that is a root in its own right must be treated as distinct from the
+    ///     outer root it also sits in. Therefore, MethodA1 -> MethodA2 counts as crossing
+    ///     (from the MethodA1 root into the ClassA root) even though both methods live in
+    ///     ClassA. Without this, an edge from a separately shown child would be swallowed
+    ///     as "internal" and never surface.
+    /// </summary>
+    [Test]
+    public void FindAllRelationshipsDeep_WithNestedRoots_ReportsEdgeFromInnerRootToItsSibling()
+    {
+        var classA = _graph.CreateClass("ClassA");
+        var methodA1 = _graph.CreateMethod("MethodA1", classA);
+        var methodA2 = _graph.CreateMethod("MethodA2", classA);
+        var call = Rel(methodA1, methodA2, RelationshipType.Calls);
+
+        // ClassA and its own MethodA1 are both roots (MethodA1 sits inside ClassA's subtree).
+        var roots = new HashSet<string> { classA.Id, methodA1.Id };
+        var result = _explorer.FindAllRelationshipsDeep(roots);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(result.Relationships, Is.EquivalentTo(new[] { call }));
+            Assert.That(result.Elements.Select(e => e.Id),
+                Is.EquivalentTo([classA.Id, methodA1.Id, methodA2.Id]));
         });
     }