Provide Order Matters for Scopes -- Detects Nonexistent Cyclical Dependencies

[bryanroute]

Describe the bug The order in which values are provided to the container appears to matter but only when using scopes, otherwise it does not.

To Reproduce

import (
    "go.uber.org/dig"
    "testing"
)

type sampleStruct struct{}

type otherStruct struct{}

func Test_Order(t *testing.T) {
    c := dig.New()

    // This depends on the next provide but normally order doesn't matter
    err := c.Provide(func(val string) sampleStruct { return sampleStruct{} })
    if err != nil {
        t.Error(err)
    }

    // If you put this Provide first the test will pass
    err = c.Provide(func() string { return "Sample" })
    if err != nil {
        t.Error(err)
    }

    scope := c.Scope("test")

    // This will error with an error about a cyclical dependency but there is no cycle.
    // If you call Provide directly on the container without a scope no issue
    err = scope.Provide(func() otherStruct { return otherStruct{} })
    if err != nil {
        t.Error(err)
    }
}

Expected behavior I would expect this test to pass but it does not unless you move the order of the Provide calls on the container around. I hit this really weird scenario in a much more complicated example and it took me hours to track it down and create this simple repro. I don't understand why normally the order of Provide calls doesn't matter but once a Scope is introduced it does? This seems incorrect especially since the error comes when providing something completely unrelated. Perhaps more importantly the error is about a cyclical dependency which isn't actually a cyclical dependency.

Additional context If the order of the Provide calls is meant to matter that should be clearly documented and I'd expect the same behavior with the container and scopes.

[JacobOaks]

Hey @bryanroute thanks for reporting this. I have reproduced it and I believe I have found the root cause too. When we create a subscope, although we copy the graph nodes to the subscope, we do not update each node's "orders" map to contain their correct index in the new sub-scope. Because of this, when we check for cycles within the subscope, (*constructorNode).Order() is returning 0 since the sub-scope is not in its orders map. This is incorrect and is causing the graph algorithm to think a cycle exists between a function and itself.

I'll put up a fix for this soon.

[bryanroute]

Awesome thanks for looking into it! I dug into the code a bit with the debugger and expected it was something with how that was set up but didn't know the code well enough or have time to fully grok how to make fix.

[JacobOaks]

Hey @bryanroute, closing this now as https://github.com/uber-go/dig/pull/398 should have fixed it. Thanks again for reporting this issue!

[bryanroute]

Awesome thank you! Any idea when you'll tag another release?

[JacobOaks]

@bryanroute Now :)

https://github.com/uber-go/dig/releases/tag/v1.17.1

[bryanroute]

Perfect, pulled and confirmed in my tests the problem is resolved. Thank you so much for the quick response!