Repository: uber-go/dig
Pull request #390 (2023-06-08 22:25:26)

Add suggestions for slices and array element missing types
Changes:

Added suggestions for requesting slice/array of pointers instead of the elements and vice versa

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/uber-go/dig?pullRequest=390) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/uber-go/dig?pullRequest=390) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/uber-go/dig?pullRequest=390) it.</sub>
paullen: Hey. Just wanted to follow up on this PR. This seems like a good suggestion to have, it is a part of the solution for Issue #184 . Then there is the issue #389 that addresses the other half of the #184 issue.
JacobOaks: Hey @paullen, thanks for this PR!

I think this is a good addition, can we just add a test case similar to [this one](https://github.com/uber-go/dig/blob/a30081d72915995cb8f934ba9393256c74df9143/dig_test.go#L3251) that tests that the suggestions get properly populated from a call to `fx.Invoke`?
paullen: @JacobOaks Added test cases, the test coverage is now 99%, up from 98.3%. Please review. Thanks!
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/uber-go/dig/pull/390?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) Report
> Merging [#390](https://app.codecov.io/gh/uber-go/dig/pull/390?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) (a5d35dc) into [master](https://app.codecov.io/gh/uber-go/dig/commit/a30081d72915995cb8f934ba9393256c74df9143?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) (a30081d) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #390      +/-   ##
==========================================
+ Coverage   98.39%   98.40%   +0.01%     
==========================================
  Files          22       22              
  Lines        1492     1502      +10     
==========================================
+ Hits         1468     1478      +10     
  Misses         15       15              
  Partials        9        9              
```


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

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go)


Patch:
diff --git a/error.go b/error.go
index 24c5685..8aba9ee 100644
--- a/error.go
+++ b/error.go
@@ -414,6 +414,28 @@ func newErrMissingTypes(c containerStore, k key) errMissingTypes {
 		suggestions = append(suggestions, k.t.Elem())
 	}
 
+	if k.t.Kind() == reflect.Slice {
+		// Maybe the user meant a slice of pointers while we have the slice of elements
+		suggestions = append(suggestions, reflect.SliceOf(reflect.PtrTo(k.t.Elem())))
+
+		// Maybe the user meant a slice of elements while we have the slice of pointers
+		sliceElement := k.t.Elem()
+		if sliceElement.Kind() == reflect.Ptr {
+			suggestions = append(suggestions, reflect.SliceOf(sliceElement.Elem()))
+		}
+	}
+
+	if k.t.Kind() == reflect.Array {
+		// Maybe the user meant an array of pointers while we have the array of elements
+		suggestions = append(suggestions, reflect.ArrayOf(k.t.Len(), reflect.PtrTo(k.t.Elem())))
+
+		// Maybe the user meant an array of elements while we have the array of pointers
+		arrayElement := k.t.Elem()
+		if arrayElement.Kind() == reflect.Ptr {
+			suggestions = append(suggestions, reflect.ArrayOf(k.t.Len(), arrayElement.Elem()))
+		}
+	}
+
 	knownTypes := c.knownTypes()
 	if k.t.Kind() == reflect.Interface {
 		// Maybe we have an implementation of the interface.
diff --git a/param.go b/param.go
index d584fc2..ca06897 100644
--- a/param.go
+++ b/param.go
@@ -606,7 +606,7 @@ func (pt paramGroupedSlice) callGroupProviders(c containerStore) (int, error) {
 		providers := c.getGroupProviders(pt.Group, pt.Type.Elem())
 		itemCount += len(providers)
 		for _, n := range providers {
-			if err := n.Call(c); err != nil {
+			if err := n.Call(n.OrigScope()); err != nil {
 				return 0, errParamGroupFailed{
 					CtorID: n.ID(),
 					Key:    key{group: pt.Group, t: pt.Type.Elem()},


Test patch:
diff --git a/dig_test.go b/dig_test.go
index f85241b..321393c 100644
--- a/dig_test.go
+++ b/dig_test.go
@@ -37,6 +37,51 @@ import (
 	"go.uber.org/dig/internal/digtest"
 )
 
+func TestValueGroupValuesGetProperlyDecorated(t *testing.T) {
+	type Foo struct{}
+	type FooResults struct {
+		dig.Out
+
+		Foo Foo `group:"foos"`
+	}
+	type Bar struct{}
+	type UseBarAndFooParams struct {
+		dig.In
+
+		Foos []Foo `group:"foos"`
+		Bar  Bar
+	}
+
+	var FooCtrRun, BarCtrRun bool
+	c := digtest.New(t)
+	c.RequireProvide(func() string { return "base" })
+
+	child := c.Scope("child")
+	child.RequireDecorate(func(s string) string {
+		return s + "-decorated"
+	})
+	child.RequireProvide(func(s string) FooResults {
+		FooCtrRun = true
+		// Previously, this would have just been "base",
+		// because we weren't looking in the constructor's
+		// original scope for decorators to apply
+		// when the value is a value group value.
+		assert.Equal(t, "base-decorated", s)
+		return FooResults{
+			Foo: Foo{},
+		}
+	}, dig.Export(true))
+	child.RequireProvide(func(s string) Bar {
+		BarCtrRun = true
+		assert.Equal(t, "base-decorated", s)
+		return Bar{}
+	}, dig.Export(true))
+	child.RequireInvoke(func(UseBarAndFooParams) {})
+
+	assert.True(t, FooCtrRun)
+	assert.True(t, BarCtrRun)
+}
+
 func TestEndToEndSuccess(t *testing.T) {
 	t.Parallel()
 
@@ -3255,6 +3300,11 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
 
 			A `name:"hello"`
 		}
+		type outPointerA struct {
+			dig.Out
+
+			A *A `name:"hello"`
+		}
 
 		cases := []struct {
 			name        string
@@ -3294,6 +3344,190 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
 					`\*dig_test.A\[name="hello"\] \(did you mean (to use )?dig_test.A\[name="hello"\]\?\)`,
 				},
 			},
+			{
+				name:    "named value missing, pointer present",
+				provide: func() outPointerA { return outPointerA{A: &A{}} },
+				invoke: func(struct {
+					dig.In
+
+					A `name:"hello"`
+				}) {
+				},
+				errContains: []string{
+					`missing type:`,
+					`dig_test.A\[name="hello"\] \(did you mean (to use )?\*dig_test.A\[name="hello"\]\?\)`,
+				},
+			},
+		}
+
+		for _, tc := range cases {
+			c := digtest.New(t, dig.DryRun(dryRun))
+			t.Run(tc.name, func(t *testing.T) {
+				c.RequireProvide(tc.provide)
+
+				err := c.Invoke(tc.invoke)
+				require.Error(t, err)
+
+				lines := append([]string{
+					`dig_test.go:\d+`, // file:line
+				}, tc.errContains...)
+				dig.AssertErrorMatches(t, err,
+					`missing dependencies for function "go.uber.org/dig_test".testInvokeFailures.\S+`,
+					lines...)
+			})
+		}
+	})
+
+	t.Run("requesting a slice of values or pointers when the other is present", func(t *testing.T) {
+		type A struct{}
+		type outA struct {
+			dig.Out
+
+			A []A `name:"hello"`
+		}
+		type outPointerA struct {
+			dig.Out
+
+			A []*A `name:"hello"`
+		}
+
+		cases := []struct {
+			name        string
+			provide     interface{}
+			invoke      interface{}
+			errContains []string
+		}{
+			{
+				name:    "value slice missing, pointer slice present",
+				provide: func() []*A { return []*A{{}} },
+				invoke:  func([]A) {},
+				errContains: []string{
+					`missing type:`,
+					`\[\]dig_test.A \(did you mean (to use )?\[\]\*dig_test.A\?\)`,
+				},
+			},
+			{
+				name:    "pointer slice missing, value slice present",
+				provide: func() []A { return []A{{}} },
+				invoke:  func([]*A) {},
+				errContains: []string{
+					`missing type:`,
+					`\[\]\*dig_test.A \(did you mean (to use )?\[\]dig_test.A\?\)`,
+				},
+			},
+			{
+				name:    "named pointer slice missing, value slice present",
+				provide: func() outA { return outA{A: []A{{}}} },
+				invoke: func(struct {
+					dig.In
+
+					A []*A `name:"hello"`
+				}) {
+				},
+				errContains: []string{
+					`missing type:`,
+					`\[\]\*dig_test.A\[name="hello"\] \(did you mean (to use )?\[\]dig_test.A\[name="hello"\]\?\)`,
+				},
+			},
+			{
+				name:    "named value slice missing, pointer slice present",
+				provide: func() outPointerA { return outPointerA{A: []*A{{}}} },
+				invoke: func(struct {
+					dig.In
+
+					A []A `name:"hello"`
+				}) {
+				},
+				errContains: []string{
+					`missing type:`,
+					`\[\]dig_test.A\[name="hello"\] \(did you mean (to use )?\[\]\*dig_test.A\[name="hello"\]\?\)`,
+				},
+			},
+		}
+
+		for _, tc := range cases {
+			c := digtest.New(t, dig.DryRun(dryRun))
+			t.Run(tc.name, func(t *testing.T) {
+				c.RequireProvide(tc.provide)
+
+				err := c.Invoke(tc.invoke)
+				require.Error(t, err)
+
+				lines := append([]string{
+					`dig_test.go:\d+`, // file:line
+				}, tc.errContains...)
+				dig.AssertErrorMatches(t, err,
+					`missing dependencies for function "go.uber.org/dig_test".testInvokeFailures.\S+`,
+					lines...)
+			})
+		}
+	})
+
+	t.Run("requesting an array of values or pointers when the other is present", func(t *testing.T) {
+		type A struct{}
+		type outA struct {
+			dig.Out
+
+			A [7]A `name:"hello"`
+		}
+		type outPointerA struct {
+			dig.Out
+
+			A [7]*A `name:"hello"`
+		}
+
+		cases := []struct {
+			name        string
+			provide     interface{}
+			invoke      interface{}
+			errContains []string
+		}{
+			{
+				name:    "value slice missing, pointer slice present",
+				provide: func() [7]*A { return [7]*A{{}} },
+				invoke:  func([7]A) {},
+				errContains: []string{
+					`missing type:`,
+					`\[7\]dig_test.A \(did you mean (to use )?\[7\]\*dig_test.A\?\)`,
+				},
+			},
+			{
+				name:    "pointer slice missing, value slice present",
+				provide: func() [7]A { return [7]A{{}} },
+				invoke:  func([7]*A) {},
+				errContains: []string{
+					`missing type:`,
+					`\[7\]\*dig_test.A \(did you mean (to use )?\[7\]dig_test.A\?\)`,
+				},
+			},
+			{
+				name:    "named pointer slice missing, value slice present",
+				provide: func() outA { return outA{A: [7]A{{}}} },
+				invoke: func(struct {
+					dig.In
+
+					A [7]*A `name:"hello"`
+				}) {
+				},
+				errContains: []string{
+					`missing type:`,
+					`\[7\]\*dig_test.A\[name="hello"\] \(did you mean (to use )?\[7\]dig_test.A\[name="hello"\]\?\)`,
+				},
+			},
+			{
+				name:    "named value slice missing, pointer slice present",
+				provide: func() outPointerA { return outPointerA{A: [7]*A{{}}} },
+				invoke: func(struct {
+					dig.In
+
+					A [7]A `name:"hello"`
+				}) {
+				},
+				errContains: []string{
+					`missing type:`,
+					`\[7\]dig_test.A\[name="hello"\] \(did you mean (to use )?\[7\]\*dig_test.A\[name="hello"\]\?\)`,
+				},
+			},
 		}
 
 		for _, tc := range cases {
diff --git a/error_test.go b/error_test.go
index a658f9d..ac9a536 100644
--- a/error_test.go
+++ b/error_test.go
@@ -346,6 +346,50 @@ func TestMissingTypeFormatting(t *testing.T) {
 			wantV:     "dig.type1 (did you mean *dig.type1, or dig.someInterface?)",
 			wantPlusV: "dig.type1 (did you mean to use one of *dig.type1, or dig.someInterface?)",
 		},
+		{
+			desc: "one suggestion for a slice of elements",
+			give: missingType{
+				Key: key{t: reflect.TypeOf([]type1{})},
+				suggestions: []key{
+					{t: reflect.TypeOf([]*type1{})},
+				},
+			},
+			wantV:     "[]dig.type1 (did you mean []*dig.type1?)",
+			wantPlusV: "[]dig.type1 (did you mean to use []*dig.type1?)",
+		},
+		{
+			desc: "one suggestion for an array of elements",
+			give: missingType{
+				Key: key{t: reflect.TypeOf([4]type1{})},
+				suggestions: []key{
+					{t: reflect.TypeOf([4]*type1{})},
+				},
+			},
+			wantV:     "[4]dig.type1 (did you mean [4]*dig.type1?)",
+			wantPlusV: "[4]dig.type1 (did you mean to use [4]*dig.type1?)",
+		},
+		{
+			desc: "one suggestion for a slice of pointers",
+			give: missingType{
+				Key: key{t: reflect.TypeOf([]*type1{})},
+				suggestions: []key{
+					{t: reflect.TypeOf([]type1{})},
+				},
+			},
+			wantV:     "[]*dig.type1 (did you mean []dig.type1?)",
+			wantPlusV: "[]*dig.type1 (did you mean to use []dig.type1?)",
+		},
+		{
+			desc: "one suggestion for an array of pointers",
+			give: missingType{
+				Key: key{t: reflect.TypeOf([4]*type1{})},
+				suggestions: []key{
+					{t: reflect.TypeOf([4]type1{})},
+				},
+			},
+			wantV:     "[4]*dig.type1 (did you mean [4]dig.type1?)",
+			wantPlusV: "[4]*dig.type1 (did you mean to use [4]dig.type1?)",
+		},
 	}
 
 	for _, tt := range tests {