param: Search scopes during Build, support value groups (#312)

This significantly changes how we instantiate dependencies for a
constructor and adds support for value groups.

Previously, for each parameter of a constructor, we attempted to
instantiate the value in every scope until one succeeded without
dependency errors. Effectively:

    for param in constructor
        for scope in scopes_to_root
            err = scope.instantiate(param)
            if err != missing_dependencies {
                return err
            }

This made the behavior less certain because we were relying on fuzzily
matching errors.

With this change, we search the container for constructors as needed
for singular parameters (paramSingle) and for value groups
(paramGroupedSlice). Roughly:

    for param in constructor
        switch param {
        case paramSingle:
            for scope in scopes_to_root {
                if scope.provides(param) {
                    found = true
                }
            }
            // ...
        
        case paramGroupedSlice:
            for scope in scopes_to_root {
                providers.append(scope.providers_for(param))
            }
        
        default:
            // ...
        }

This is cleaner because it works by searching the container for what it
needs rather than trying and letting it fail.

This includes a subtle change to constructorNode.Call:
previously, when a constructor ran, its results were stored in the
containerStore that it used to instantiate its dependencies.
Now, results are stored in the containerStore (Scope) to which that
constructor was provided. This works because the leaf param types
(paramSingle and paramGroupedSlice) will search the scope tree to find
the value.

Author: abhinav

constructor.go

@@ -148,7 +148,12 @@ func (n *constructorNode) Call(c containerStore) error {
 	if err := n.resultList.ExtractList(receiver, results); err != nil {
 		return errConstructorFailed{Func: n.location, Reason: err}
 	}
-	receiver.Commit(c)
+
+	// Commit the result to the original container that this constructor
+	// was supplied to. The provided constructor is only used for a view of
+	// the rest of the graph to instantiate the dependencies of this
+	// container.
+	receiver.Commit(n.s)
 	n.called = true
 
 	return nil

param.go

@@ -146,28 +146,11 @@ func (pl paramList) Build(containerStore) (reflect.Value, error) {
 // to the underlying constructor.
 func (pl paramList) BuildList(c containerStore) ([]reflect.Value, error) {
 	args := make([]reflect.Value, len(pl.Params))
-	allContainers := c.storesToRoot()
 	for i, p := range pl.Params {
-		// iterate through the tree path of scopes.
-	containerLoop:
-		for _, c := range allContainers {
-			arg, err := p.Build(c)
-			if err == nil {
-				args[i] = arg
-				break containerLoop
-			}
-			// If argument has successfully been built, it's possible
-			// for these errors to occur in child scopes that don't
-			// contain the given parameter type. We can safely ignore
-			// these.
-			// If it's an error other than missing types/dependencies,
-			// this means some constructor returned an error that must
-			// be reported.
-			_, isErrMissingTypes := err.(errMissingTypes)
-			_, isErrMissingDeps := err.(errMissingDependencies)
-			if err != nil && !isErrMissingTypes && !isErrMissingDeps {
-				return nil, err
-			}
+		var err error
+		args[i], err = p.Build(c)
+		if err != nil {
+			return nil, err
 		}
 	}
 	return args, nil
@@ -213,12 +196,36 @@ func (ps paramSingle) String() string {
 
 	return fmt.Sprintf("%v[%v]", ps.Type, strings.Join(opts, ", "))
 }
+
+// searches the given container and its parent for a matching value.
+func (ps paramSingle) getValue(c containerStore) (reflect.Value, bool) {
+	for _, c := range c.storesToRoot() {
+		if v, ok := c.getValue(ps.Name, ps.Type); ok {
+			return v, ok
+		}
+	}
+	return _noValue, false
+}
+
 func (ps paramSingle) Build(c containerStore) (reflect.Value, error) {
-	if v, ok := c.getValue(ps.Name, ps.Type); ok {
+	if v, ok := ps.getValue(c); ok {
 		return v, nil
 	}
 
-	providers := c.getValueProviders(ps.Name, ps.Type)
+	// Starting at the given container and working our way up its parents,
+	// find one that provides this dependency.
+	//
+	// Once found, we'll use that container for the rest of the invocation.
+	// Dependencies of this type will begin searching at that container,
+	// rather than starting at base.
+	var providers []provider
+	for _, container := range c.storesToRoot() {
+		providers = container.getValueProviders(ps.Name, ps.Type)
+		if len(providers) > 0 {
+			break
+		}
+	}
+
 	if len(providers) == 0 {
 		if ps.Optional {
 			return reflect.Zero(ps.Type), nil
@@ -247,7 +254,7 @@ func (ps paramSingle) Build(c containerStore) (reflect.Value, error) {
 
 	// If we get here, it's impossible for the value to be absent from the
 	// container.
-	v, _ := c.getValue(ps.Name, ps.Type)
+	v, _ := ps.getValue(c)
 	return v, nil
 }
 
@@ -479,21 +486,25 @@ func newParamGroupedSlice(f reflect.StructField, c containerStore) (paramGrouped
 }
 
 func (pt paramGroupedSlice) Build(c containerStore) (reflect.Value, error) {
-	for _, n := range c.getGroupProviders(pt.Group, pt.Type.Elem()) {
-		if err := n.Call(c); err != nil {
-			return _noValue, errParamGroupFailed{
-				CtorID: n.ID(),
-				Key:    key{group: pt.Group, t: pt.Type.Elem()},
-				Reason: err,
+	var itemCount int
+	stores := c.storesToRoot()
+	for _, c := range stores {
+		providers := c.getGroupProviders(pt.Group, pt.Type.Elem())
+		itemCount += len(providers)
+		for _, n := range providers {
+			if err := n.Call(c); err != nil {
+				return _noValue, errParamGroupFailed{
+					CtorID: n.ID(),
+					Key:    key{group: pt.Group, t: pt.Type.Elem()},
+					Reason: err,
+				}
 			}
 		}
 	}
 
-	items := c.getValueGroup(pt.Group, pt.Type.Elem())
-
-	result := reflect.MakeSlice(pt.Type, len(items), len(items))
-	for i, v := range items {
-		result.Index(i).Set(v)
+	result := reflect.MakeSlice(pt.Type, 0, itemCount)
+	for _, c := range stores {
+		result = reflect.Append(result, c.getValueGroup(pt.Group, pt.Type.Elem())...)
 	}
 	return result, nil
 }

scope_test.go

@@ -309,3 +309,81 @@ func TestScopeFailures(t *testing.T) {
 		assert.NoError(t, gc.Invoke(func(a *A) {}), "expected Invoke in grandchild container on child's private-provided type to fail")
 	})
 }
+
+func TestScopeValueGroups(t *testing.T) {
+	t.Run("provide in parent and child", func(t *testing.T) {
+		type result struct {
+			Out
+
+			Value string `group:"foo"`
+		}
+
+		root := New()
+		require.NoError(t, root.Provide(func() result {
+			return result{Value: "a"}
+		}))
+		require.NoError(t, root.Provide(func() result {
+			return result{Value: "b"}
+		}))
+		require.NoError(t, root.Provide(func() result {
+			return result{Value: "c"}
+		}))
+
+		child := root.Scope("child")
+		require.NoError(t,
+			child.Provide(func() result {
+				return result{Value: "d"}
+			}))
+
+		type param struct {
+			In
+
+			Values []string `group:"foo"`
+		}
+
+		t.Run("invoke parent", func(t *testing.T) {
+			require.NoError(t, root.Invoke(func(i param) {
+				assert.ElementsMatch(t, []string{"a", "b", "c"}, i.Values)
+			}), "only values added to parent should be visible")
+		})
+
+		t.Run("invoke child", func(t *testing.T) {
+			require.NoError(t, child.Invoke(func(i param) {
+				assert.ElementsMatch(t, []string{"a", "b", "c", "d"}, i.Values)
+			}), "values added to both, parent and child should be visible")
+		})
+	})
+
+	t.Run("value group as a parent dependency", func(t *testing.T) {
+		// Tree:
+		//
+		//   root      defines a function that consumes the value group
+		//    |
+		//    |
+		//   child     produces values to the value group
+
+		type T1 struct{}
+		type param struct {
+			In
+
+			Values []string `group:"foo"`
+		}
+
+		root := New()
+
+		require.NoError(t, root.Provide(func(p param) T1 {
+			assert.ElementsMatch(t, []string{"a", "b", "c"}, p.Values)
+			return T1{}
+		}))
+
+		child := root.Scope("child")
+		require.NoError(t, child.Provide(func() string { return "a" }, Group("foo")))
+		require.NoError(t, child.Provide(func() string { return "b" }, Group("foo")))
+		require.NoError(t, child.Provide(func() string { return "c" }, Group("foo")))
+
+		// Invocation in child should see values provided to the child,
+		// even though the constructor we're invoking is provided in
+		// the parent.
+		require.NoError(t, child.Invoke(func(T1) {}))
+	})
+}