Fix false positives in cycle detection with child scopes. (#398)

Currently, cycle detection works by assigning each constructor node
an "order"/index for the scope it was given in. If an edge exists
between an index and itself, a cycle is detected.

When we create a subscope, we copy all of the parent's nodes
to the child scope, but we do not copy their orders.
This causes all root scope constructors to effectively have
order 0 in the child scope (zero value for a mapping to int).
This causes false positives when doing cycle detection
as an edge from order 0 to order 0 gets detected.

This PR fixes this bug by copying the order for constructor nodes
from parent to child scope when a child scope is created.

The added test case fails before this PR, passes with it.

Ref: #397

constructor.go

@@ -129,6 +129,11 @@ func (n *constructorNode) CType() reflect.Type        { return n.ctype }
 func (n *constructorNode) Order(s *Scope) int         { return n.orders[s] }
 func (n *constructorNode) OrigScope() *Scope          { return n.origS }
 
+// CopyOrder copies the order for the given parent scope to the given child scope.
+func (n *constructorNode) CopyOrder(parent, child *Scope) {
+	n.orders[child] = n.orders[parent]
+}
+
 func (n *constructorNode) String() string {
 	return fmt.Sprintf("deps: %v, ctor: %v", n.paramList, n.ctype)
 }

scope.go

@@ -121,7 +121,12 @@ func (s *Scope) Scope(name string, opts ...ScopeOption) *Scope {
 	child.recoverFromPanics = s.recoverFromPanics
 
 	// child copies the parent's graph nodes.
-	child.gh.nodes = append(child.gh.nodes, s.gh.nodes...)
+	for _, node := range s.gh.nodes {
+		child.gh.nodes = append(child.gh.nodes, node)
+		if ctrNode, ok := node.Wrapped.(*constructorNode); ok {
+			ctrNode.CopyOrder(s, child)
+		}
+	}
 
 	for _, opt := range opts {
 		opt.noScopeOption()

scope_test.go

@@ -397,3 +397,20 @@ func TestScopeValueGroups(t *testing.T) {
 		child.RequireInvoke(func(T1) {})
 	})
 }
+
+// This tests that a child scope correctly copies its parent's graph,
+// including information about the order of each node.
+// Otherwise, during cycle detection, constructor nodes might
+// return 0 as the order for all functions in the root scope,
+// causing cycle detection to detect cycles that don't exist.
+func TestFalsePositiveScopeCycleDetection(t *testing.T) {
+	t.Run("single provide", func(t *testing.T) {
+		root := digtest.New(t)
+		root.RequireProvide(func(val string) int { return 0 })
+		root.RequireProvide(func() string { return "sample" })
+		child := root.Scope("child")
+		// Cycle detection would error here because previous two provides
+		// would both have order 0 for child scope.
+		child.RequireProvide(func() float32 { return 0 })
+	})
+}