Repository: uber-go/dig
Pull request #334 (2022-03-22 06:05:20)

Error out when trying to decorate a value group with single value
When a value group is being decorated with a single value, it currently
panics because decorate tries to take element of a non-slice value.

While it's possible to allow this technically, after some consideration,
this should just not be allowed because it can be confusing to distinguish
between the two cases. For example, when providing a slice of slices as a
value group, it's unclear whether or not the target type is the slice of types,
or the slice of slices of types.

In addition, if there is a good way to support it in the future, it is
possible to add that later.

Fix https://github.com/uber-go/fx/issues/859
Ref GO-1261

codecov[bot]: # [Codecov](https://codecov.io/gh/uber-go/dig/pull/334?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) Report
> Merging [#334](https://codecov.io/gh/uber-go/dig/pull/334?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) (0ebcf27) into [master](https://codecov.io/gh/uber-go/dig/commit/42c40227a9791447569fa304e83a501174e321e2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) (42c4022) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##           master     #334   +/-   ##
=======================================
  Coverage   98.31%   98.32%           
=======================================
  Files          21       21           
  Lines        1425     1429    +4     
=======================================
+ Hits         1401     1405    +4     
  Misses         15       15           
  Partials        9        9           
```


| [Impacted Files](https://codecov.io/gh/uber-go/dig/pull/334?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) | Coverage Δ | |
|---|---|---|
| [decorate.go](https://codecov.io/gh/uber-go/dig/pull/334/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-ZGVjb3JhdGUuZ28=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber-go/dig/pull/334?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/uber-go/dig/pull/334?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). Last update [42c4022...0ebcf27](https://codecov.io/gh/uber-go/dig/pull/334?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go).


Patch:
diff --git a/decorate.go b/decorate.go
index 9752611..9c21abb 100644
--- a/decorate.go
+++ b/decorate.go
@@ -21,6 +21,7 @@
 package dig
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 
@@ -214,7 +215,10 @@ func (s *Scope) Decorate(decorator interface{}, opts ...DecorateOption) error {
 		return err
 	}
 
-	keys := findResultKeys(dn.results)
+	keys, err := findResultKeys(dn.results)
+	if err != nil {
+		return err
+	}
 	for _, k := range keys {
 		if _, ok := s.decorators[k]; ok {
 			return fmt.Errorf("cannot decorate using function %v: %s already decorated",
@@ -251,7 +255,7 @@ func (s *Scope) Decorate(decorator interface{}, opts ...DecorateOption) error {
 	return nil
 }
 
-func findResultKeys(r resultList) []key {
+func findResultKeys(r resultList) ([]key, error) {
 	// use BFS to search for all keys included in a resultList.
 	var (
 		q    []result
@@ -267,6 +271,9 @@ func findResultKeys(r resultList) []key {
 		case resultSingle:
 			keys = append(keys, key{t: innerResult.Type, name: innerResult.Name})
 		case resultGrouped:
+			if innerResult.Type.Kind() != reflect.Slice {
+				return nil, errors.New("decorating a value group requires decorating the entire value group, not a single value")
+			}
 			keys = append(keys, key{t: innerResult.Type.Elem(), group: innerResult.Group})
 		case resultObject:
 			for _, f := range innerResult.Fields {
@@ -276,5 +283,5 @@ func findResultKeys(r resultList) []key {
 			q = append(q, innerResult.Results...)
 		}
 	}
-	return keys
+	return keys, nil
 }


Test patch:
diff --git a/decorate_test.go b/decorate_test.go
index 4208bd4..ca57be9 100644
--- a/decorate_test.go
+++ b/decorate_test.go
@@ -673,6 +673,22 @@ func TestDecorateFailure(t *testing.T) {
 		assert.Contains(t, err.Error(), "function func(dig_test.Param) dig_test.Result")
 		assert.Contains(t, err.Error(), "*dig_test.A already decorated")
 	})
+
+	t.Run("decorate value group with a single value", func(t *testing.T) {
+		type A struct {
+			dig.Out
+
+			Value int `group:"val"`
+		}
+
+		root := digtest.New(t)
+
+		root.RequireProvide(func() A { return A{Value: 1} })
+		err := root.Decorate(func() A { return A{Value: 11} })
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "decorating a value group requires decorating the entire value group")
+	})
 }
 
 func TestMultipleDecorates(t *testing.T) {