Merge pull request #1631 from gavin-ts/simple_edges_across_diagrams

Simple edges across nested diagrams

ci/release/changelogs/next.md

@@ -3,6 +3,7 @@
 #### Improvements 🧹
 
 - Grid cells can now contain nested edges [#1629](https://github.com/terrastruct/d2/pull/1629)
+- Edges can now go across constant nears, sequence diagrams, and grids including nested ones. [#1631](https://github.com/terrastruct/d2/pull/1631)
 
 #### Bugfixes ⛑️
 

d2compiler/compile.go

@@ -1068,20 +1068,13 @@ func (c *compiler) validateNear(g *d2graph.Graph) {
 	}
 
 	for _, edge := range g.Edges {
-		srcNearContainer := edge.Src.OuterNearContainer()
-		dstNearContainer := edge.Dst.OuterNearContainer()
-
-		var isSrcNearConst, isDstNearConst bool
-
-		if srcNearContainer != nil {
-			_, isSrcNearConst = d2graph.NearConstants[d2graph.Key(srcNearContainer.NearKey)[0]]
-		}
-		if dstNearContainer != nil {
-			_, isDstNearConst = d2graph.NearConstants[d2graph.Key(dstNearContainer.NearKey)[0]]
+		if edge.Src.IsConstantNear() && edge.Dst.IsDescendantOf(edge.Src) {
+			c.errorf(edge.GetAstEdge(), "edge from constant near %#v cannot enter itself", edge.Src.AbsID())
+			continue
 		}
-
-		if (isSrcNearConst || isDstNearConst) && srcNearContainer != dstNearContainer {
-			c.errorf(edge.References[0].Edge, "cannot connect objects from within a container, that has near constant set, to objects outside that container")
+		if edge.Dst.IsConstantNear() && edge.Src.IsDescendantOf(edge.Dst) {
+			c.errorf(edge.GetAstEdge(), "edge from constant near %#v cannot enter itself", edge.Dst.AbsID())
+			continue
 		}
 	}
 
@@ -1101,47 +1094,22 @@ func (c *compiler) validateEdges(g *d2graph.Graph) {
 			c.errorf(edge.GetAstEdge(), "edge from grid diagram %#v cannot enter itself", edge.Dst.AbsID())
 			continue
 		}
-
-		srcGrid := edge.Src.Parent.ClosestGridDiagram()
-		dstGrid := edge.Dst.Parent.ClosestGridDiagram()
-		if srcGrid != nil || dstGrid != nil {
-			if srcGrid != dstGrid {
-				// valid: a -> grid
-				// invalid: a -> grid.child
-				if dstGrid != nil && !(srcGrid != nil && srcGrid.IsDescendantOf(dstGrid)) {
-					c.errorf(edge.GetAstEdge(), "edge cannot enter grid diagram %#v", dstGrid.AbsID())
-				} else {
-					c.errorf(edge.GetAstEdge(), "edge cannot exit grid diagram %#v", srcGrid.AbsID())
-				}
-				continue
-			}
-
-			srcCell := edge.Src.ClosestGridCell()
-			dstCell := edge.Dst.ClosestGridCell()
-			// edges within a grid cell are ok now
-			//   grid.cell.a -> grid.cell.b : ok
-			//   grid.cell.a.c -> grid.cell.b.d : ok
-			// edges between grid cells themselves are ok
-			//   grid.cell -> grid.cell2 : ok
-			//   grid.cell -> grid.cell.inside : not ok
-			//   grid.cell -> grid.cell2.inside : not ok
-			srcIsGridCell := edge.Src == srcCell
-			dstIsGridCell := edge.Dst == dstCell
-			if srcIsGridCell != dstIsGridCell {
-				if srcIsGridCell {
-					c.errorf(edge.GetAstEdge(), "grid cell %#v can only connect to another grid cell", edge.Src.AbsID())
-				} else {
-					c.errorf(edge.GetAstEdge(), "grid cell %#v can only connect to another grid cell", edge.Dst.AbsID())
-				}
-				continue
-			}
-
-			if srcCell != dstCell && (!srcIsGridCell || !dstIsGridCell) {
-				c.errorf(edge.GetAstEdge(), "edge cannot exit grid cell %#v", srcCell.AbsID())
-				continue
-			}
+		if edge.Src.Parent.IsGridDiagram() && edge.Dst.IsDescendantOf(edge.Src) {
+			c.errorf(edge.GetAstEdge(), "edge from grid cell %#v cannot enter itself", edge.Src.AbsID())
+			continue
+		}
+		if edge.Dst.Parent.IsGridDiagram() && edge.Src.IsDescendantOf(edge.Dst) {
+			c.errorf(edge.GetAstEdge(), "edge from grid cell %#v cannot enter itself", edge.Dst.AbsID())
+			continue
+		}
+		if edge.Src.IsSequenceDiagram() && edge.Dst.IsDescendantOf(edge.Src) {
+			c.errorf(edge.GetAstEdge(), "edge from sequence diagram %#v cannot enter itself", edge.Src.AbsID())
+			continue
+		}
+		if edge.Dst.IsSequenceDiagram() && edge.Src.IsDescendantOf(edge.Dst) {
+			c.errorf(edge.GetAstEdge(), "edge from sequence diagram %#v cannot enter itself", edge.Dst.AbsID())
+			continue
 		}
-
 	}
 }
 

d2compiler/compile_test.go

@@ -1616,7 +1616,7 @@ d2/testdata/d2compiler/TestCompile/near-invalid.d2:14:9: near keys cannot be set
 				}
 				x -> y
 			`,
-			expErr: `d2/testdata/d2compiler/TestCompile/near_bad_connected.d2:5:5: cannot connect objects from within a container, that has near constant set, to objects outside that container`,
+			expErr: ``,
 		},
 		{
 			name: "near_descendant_connect_to_outside",
@@ -1627,7 +1627,7 @@ d2/testdata/d2compiler/TestCompile/near-invalid.d2:14:9: near keys cannot be set
 				}
 				x.y -> z
 			`,
-			expErr: "d2/testdata/d2compiler/TestCompile/near_descendant_connect_to_outside.d2:6:5: cannot connect objects from within a container, that has near constant set, to objects outside that container",
+			expErr: "",
 		},
 		{
 			name: "nested_near_constant",
@@ -2040,7 +2040,7 @@ b
 }
 b -> x.a
 `,
-			expErr: `d2/testdata/d2compiler/TestCompile/leaky_sequence.d2:5:1: connections within sequence diagrams can connect only to other objects within the same sequence diagram`,
+			expErr: ``,
 		},
 		{
 			name: "sequence_scoping",
@@ -2484,9 +2484,7 @@ hey -> hey.a
 
 hey -> c: ok
 `,
-			expErr: `d2/testdata/d2compiler/TestCompile/grid_edge.d2:5:1: edge cannot enter grid diagram "hey"
-d2/testdata/d2compiler/TestCompile/grid_edge.d2:6:1: edge cannot exit grid diagram "hey"
-d2/testdata/d2compiler/TestCompile/grid_edge.d2:7:1: edge from grid diagram "hey" cannot enter itself`,
+			expErr: `d2/testdata/d2compiler/TestCompile/grid_edge.d2:7:1: edge from grid diagram "hey" cannot enter itself`,
 		},
 		{
 			name: "grid_deeper_edge",
@@ -2506,19 +2504,47 @@ d2/testdata/d2compiler/TestCompile/grid_edge.d2:7:1: edge from grid diagram "hey
 			g -> h: ok
 			g -> h.h: ok
 		}
-		e -> f.i: not ok
-		e.g -> f.i: not ok
+		e -> f.i: ok now
+		e.g -> f.i: ok now
 	}
-	a -> b.c: not yet
-	a.e -> b.c: also not yet
+	a -> b.c: ok now
+	a.e -> b.c: ok now
 	a -> a.e: not ok
 }
 `,
-			expErr: `d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:17:3: grid cell "hey.a.e" can only connect to another grid cell
-d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:18:3: edge cannot exit grid cell "hey.a.e"
-d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:20:2: grid cell "hey.a" can only connect to another grid cell
-d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:21:2: edge cannot exit grid diagram "hey.a"
-d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:22:2: edge from grid diagram "hey.a" cannot enter itself`,
+			expErr: `d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:22:2: edge from grid diagram "hey.a" cannot enter itself`,
+		},
+		{
+			name: "parent_graph_edge_to_descendant",
+			text: `tl: {
+	near: top-left
+	a.b
+}
+grid: {
+	grid-rows: 1
+	cell.c.d
+}
+seq: {
+	shape: sequence_diagram
+	e.f
+}
+tl -> tl.a: no
+tl -> tl.a.b: no
+grid-> grid.cell: no
+grid-> grid.cell.c: no
+grid.cell -> grid.cell.c: no
+grid.cell -> grid.cell.c.d: no
+seq -> seq.e: no
+seq -> seq.e.f: no
+`,
+			expErr: `d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:13:1: edge from constant near "tl" cannot enter itself
+d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:14:1: edge from constant near "tl" cannot enter itself
+d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:17:1: edge from grid cell "grid.cell" cannot enter itself
+d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:18:1: edge from grid cell "grid.cell" cannot enter itself
+d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:15:1: edge from grid diagram "grid" cannot enter itself
+d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:16:1: edge from grid diagram "grid" cannot enter itself
+d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:19:1: edge from sequence diagram "seq" cannot enter itself
+d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:20:1: edge from sequence diagram "seq" cannot enter itself`,
 		},
 		{
 			name: "grid_nested",

d2graph/d2graph.go

@@ -1225,10 +1225,6 @@ func (obj *Object) Connect(srcID, dstID []string, srcArrow, dstArrow bool, label
 	src := obj.ensureChildEdge(srcID)
 	dst := obj.ensureChildEdge(dstID)
 
-	if src.OuterSequenceDiagram() != dst.OuterSequenceDiagram() {
-		return nil, errors.New("connections within sequence diagrams can connect only to other objects within the same sequence diagram")
-	}
-
 	e := &Edge{
 		Attributes: Attributes{
 			Label: Scalar{

d2layouts/d2layouts.go

@@ -14,7 +14,9 @@ import (
 	"oss.terrastruct.com/d2/d2layouts/d2near"
 	"oss.terrastruct.com/d2/d2layouts/d2sequence"
 	"oss.terrastruct.com/d2/lib/geo"
+	"oss.terrastruct.com/d2/lib/label"
 	"oss.terrastruct.com/d2/lib/log"
+	"oss.terrastruct.com/util-go/go2"
 )
 
 type DiagramType string
@@ -80,6 +82,7 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
 	// Before we can layout these nodes, we need to handle all nested diagrams first.
 	extracted := make(map[string]*d2graph.Graph)
 	var extractedOrder []string
+	var extractedEdges []*d2graph.Edge
 
 	var constantNears []*d2graph.Graph
 	restoreOrder := SaveOrder(g)
@@ -100,14 +103,15 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
 		if isGridCellContainer && gi.isDefault() {
 			// if we are in a grid diagram, and our children have descendants
 			// we need to run layout on them first, even if they are not special diagram types
-			nestedGraph := ExtractSubgraph(curr, true)
+			nestedGraph, externalEdges := ExtractSubgraph(curr, true)
 			id := curr.AbsID()
 			err := LayoutNested(ctx, nestedGraph, GraphInfo{}, coreLayout)
 			if err != nil {
 				return err
 			}
 
 			InjectNested(g.Root, nestedGraph, false)
+			g.Edges = append(g.Edges, externalEdges...)
 			restoreOrder()
 
 			// need to update curr *Object incase layout changed it
@@ -138,7 +142,8 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
 			}
 
 			// now we keep the descendants out until after grid layout
-			nestedGraph = ExtractSubgraph(curr, false)
+			nestedGraph, externalEdges = ExtractSubgraph(curr, false)
+			extractedEdges = append(extractedEdges, externalEdges...)
 
 			extracted[id] = nestedGraph
 			extractedOrder = append(extractedOrder, id)
@@ -152,7 +157,8 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
 			}
 
 			// There is a nested diagram here, so extract its contents and process in the same way
-			nestedGraph := ExtractSubgraph(curr, gi.IsConstantNear)
+			nestedGraph, externalEdges := ExtractSubgraph(curr, gi.IsConstantNear)
+			extractedEdges = append(extractedEdges, externalEdges...)
 
 			log.Info(ctx, "layout nested", slog.F("level", curr.Level()), slog.F("child", curr.AbsID()), slog.F("gi", gi))
 			nestedInfo := gi
@@ -228,24 +234,51 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
 		}
 	}
 
+	idToObj := make(map[string]*d2graph.Object)
+	for _, o := range g.Objects {
+		idToObj[o.AbsID()] = o
+	}
+
 	// With the layout set, inject all the extracted graphs
 	for _, id := range extractedOrder {
 		nestedGraph := extracted[id]
 		// we have to find the object by ID because coreLayout can replace the Objects in graph
-		var obj *d2graph.Object
-		for _, o := range g.Objects {
-			if o.AbsID() == id {
-				obj = o
-				break
-			}
-		}
-		if obj == nil {
+		obj, exists := idToObj[id]
+		if !exists {
 			return fmt.Errorf("could not find object %#v after layout", id)
 		}
 		InjectNested(obj, nestedGraph, true)
 		PositionNested(obj, nestedGraph)
 	}
 
+	// update map with injected objects
+	for _, o := range g.Objects {
+		idToObj[o.AbsID()] = o
+	}
+
+	// Restore cross-graph edges and route them
+	g.Edges = append(g.Edges, extractedEdges...)
+	for _, e := range extractedEdges {
+		// update object references
+		src, exists := idToObj[e.Src.AbsID()]
+		if !exists {
+			return fmt.Errorf("could not find object %#v after layout", e.Src.AbsID())
+		}
+		e.Src = src
+		dst, exists := idToObj[e.Dst.AbsID()]
+		if !exists {
+			return fmt.Errorf("could not find object %#v after layout", e.Dst.AbsID())
+		}
+		e.Dst = dst
+
+		// simple straight line edge routing when going across graphs
+		e.Route = []*geo.Point{e.Src.Center(), e.Dst.Center()}
+		e.TraceToShape(e.Route, 0, 1)
+		if e.Label.Value != "" {
+			e.LabelPosition = go2.Pointer(string(label.InsideMiddleCenter))
+		}
+	}
+
 	log.Debug(ctx, "done", slog.F("rootlevel", g.RootLevel), slog.F("shapes", g.PrintString()))
 	return err
 }
@@ -262,10 +295,10 @@ func NestedGraphInfo(obj *d2graph.Object) (gi GraphInfo) {
 	return gi
 }
 
-func ExtractSubgraph(container *d2graph.Object, includeSelf bool) *d2graph.Graph {
+func ExtractSubgraph(container *d2graph.Object, includeSelf bool) (nestedGraph *d2graph.Graph, externalEdges []*d2graph.Edge) {
 	// includeSelf: when we have a constant near or a grid cell that is a container,
 	// we want to include itself in the nested graph, not just its descendants,
-	nestedGraph := d2graph.NewGraph()
+	nestedGraph = d2graph.NewGraph()
 	nestedGraph.RootLevel = int(container.Level())
 	if includeSelf {
 		nestedGraph.RootLevel--
@@ -284,8 +317,21 @@ func ExtractSubgraph(container *d2graph.Object, includeSelf bool) *d2graph.Graph
 	g := container.Graph
 	remainingEdges := make([]*d2graph.Edge, 0, len(g.Edges))
 	for _, edge := range g.Edges {
-		if isNestedObject(edge.Src) && isNestedObject(edge.Dst) {
+		srcIsNested := isNestedObject(edge.Src)
+		if d2sequence.IsLifelineEnd(edge.Dst) {
+			// special handling for lifelines since their edge.Dst is a special Object
+			if srcIsNested {
+				nestedGraph.Edges = append(nestedGraph.Edges, edge)
+			} else {
+				remainingEdges = append(remainingEdges, edge)
+			}
+			continue
+		}
+		dstIsNested := isNestedObject(edge.Dst)
+		if srcIsNested && dstIsNested {
 			nestedGraph.Edges = append(nestedGraph.Edges, edge)
+		} else if srcIsNested || dstIsNested {
+			externalEdges = append(externalEdges, edge)
 		} else {
 			remainingEdges = append(remainingEdges, edge)
 		}
@@ -333,7 +379,7 @@ func ExtractSubgraph(container *d2graph.Object, includeSelf bool) *d2graph.Graph
 		container.ChildrenArray = nil
 	}
 
-	return nestedGraph
+	return nestedGraph, externalEdges
 }
 
 func InjectNested(container *d2graph.Object, nestedGraph *d2graph.Graph, isRoot bool) {

d2layouts/d2sequence/sequence_diagram.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"math"
 	"sort"
+	"strconv"
 	"strings"
 
 	"oss.terrastruct.com/util-go/go2"
@@ -411,6 +412,25 @@ func (sd *sequenceDiagram) addLifelineEdges() {
 	}
 }
 
+func IsLifelineEnd(obj *d2graph.Object) bool {
+	// lifeline ends only have ID and no graph parent or box set
+	if obj.Graph != nil || obj.Parent != nil || obj.Box != nil {
+		return false
+	}
+	if !strings.Contains(obj.ID, "-lifeline-end-") {
+		return false
+	}
+	parts := strings.Split(obj.ID, "-lifeline-end-")
+	if len(parts) > 1 {
+		hash := parts[len(parts)-1]
+		actorID := strings.Join(parts[:len(parts)-1], "-lifeline-end-")
+		if strconv.Itoa(go2.StringToIntHash(actorID+"-lifeline-end")) == hash {
+			return true
+		}
+	}
+	return false
+}
+
 func (sd *sequenceDiagram) placeNotes() {
 	rankToX := make(map[int]float64)
 	for _, actor := range sd.actors {