search

uber-go / dig

A reflection based dependency injection toolkit for Go.
https://go.uber.org/dig
MIT License
3.88k stars 206 forks source link

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

Closed JacobOaks closed 11 months ago

JacobOaks commented 11 months ago

Currently, cycle detection works by assigning each constructor node an "order"/index for the scope it was given in. If a path 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: https://github.com/uber-go/dig/issues/397

codecov[bot] commented 11 months ago

Codecov Report

Merging #398 (f5533e4) into master (119025f) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #398   +/-   ##
=======================================
  Coverage   98.40%   98.40%           
=======================================
  Files          22       22           
  Lines        1502     1505    +3     
=======================================
+ Hits         1478     1481    +3     
  Misses         15       15           
  Partials        9        9
Files Coverage Δ
constructor.go 97.50% <100.00%> (+0.03%) :arrow_up:
scope.go 98.92% <100.00%> (+0.02%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more