diff --git a/cycle.go b/cycle.go index 356c1dd3..59604034 100644 --- a/cycle.go +++ b/cycle.go @@ -73,6 +73,9 @@ func verifyAcyclic(c containerStore, n provider, k key) error { return err } +// When first called from `(c *Container) verifyAcyclic()` (`DeferAcyclicVerification()` +// option is used and this is triggered lazily in `Invoke()`) the `path` here +// is set to nil. func detectCycles(n provider, c containerStore, path []cycleEntry, visited map[key]struct{}) error { var err error walkParam(n.ParamList(), paramVisitorFunc(func(param param) bool { @@ -121,6 +124,7 @@ func detectCycles(n provider, c containerStore, path []cycleEntry, visited map[k // Alternatively, if deferAcyclicVerification was set and detectCycles // is only being called before the first Invoke, each node in the // graph will be tested as the first element of the path, so any + // ^^^^^^^^^^^^^^^^^^^^^^^^^ WHY? // cycle that exists is guaranteed to trip the following condition. if path[0].Key == k { err = errCycleDetected{Path: append(path, entry)} diff --git a/dig.go b/dig.go index 9e81923e..5018a418 100644 --- a/dig.go +++ b/dig.go @@ -449,6 +449,9 @@ func (c *Container) Invoke(function interface{}, opts ...InvokeOption) error { return nil } +// What is the motivation for introducing a function very similar to +// `verifyAcyclic(c containerStore, n provider, k key)` and why is +// the path set to nil in this case? func (c *Container) verifyAcyclic() error { visited := make(map[key]struct{}) for _, n := range c.nodes { diff --git a/dig_test.go b/dig_test.go index 44e2f1c4..bfbd8aaa 100644 --- a/dig_test.go +++ b/dig_test.go @@ -1797,24 +1797,26 @@ func testProvideCycleFails(t *testing.T, dryRun bool) { }) t.Run("DeferAcyclicVerification bypasses cycle check, VerifyAcyclic catches cycle", func(t *testing.T) { - // A <- B <- C <- D - // | ^ - // |_________| + // The offending node `C` is now *not* the first on the dependency `path` + // of `detectCycles()` (`D` is: we first call `detectCycles()` for the + // inovked `A` with the initial path set to nil, the following call for + // its dependency `D` will be in path[0]). + // A <-- C <- D + // | |__^ ^ + // |______________| type A struct{} - type B struct{} type C struct{} type D struct{} - newA := func(*C) *A { return &A{} } - newB := func(*A) *B { return &B{} } - newC := func(*B) *C { return &C{} } + newA := func(*D) *A { return &A{} } + newC := func(*C) *C { return &C{} } newD := func(*C) *D { return &D{} } c := New(DeferAcyclicVerification()) assert.NoError(t, c.Provide(newA)) - assert.NoError(t, c.Provide(newB)) assert.NoError(t, c.Provide(newC)) assert.NoError(t, c.Provide(newD)) + // Will stack overflow in this call: err := c.Invoke(func(*A) {}) require.Error(t, err, "expected error when introducing cycle") assert.True(t, IsCycleDetected(err))