Skip to content

Commit

Permalink
Bubble location from LocationForPC's ProvideOption up to errors (#332)
Browse files Browse the repository at this point in the history
Previously, if a location was passed to Provide via the
LocationForPC ProvideOption, the location would not be taken
into account in the case of an error where a constructor could
not be Provided.

This change updates the error returned, making it more readable by
basing the error message on alternate function provided by
LocationForPC rather than the constructor.

For example, the following error:
```
Failed: cannot provide function "reflect".makeFuncStub (/usr/local/Cellar/go/1.17.6/libexec/src/reflect/asm_amd64.s:30):
cannot provide *fx_test.a[name="a"] from [0].Field0: already provided by
"go.uber.org/fx_test".TestAnnotate.func1 (/gocode/src/github.com/uber-go/fx/annotated_test.go:515)
```

Becomes:
```
Failed: cannot provide function "go.uber.org/fx_test".TestAnnotate.func1 (/gocode/src/github.com/uber-go/fx/annotated_test.go:515):
cannot provide *fx_test.a[name="a"] from [0].Field0: already provided by
"go.uber.org/fx_test".TestAnnotate.func1 (/gocode/src/github.com/uber-go/fx/annotated_test.go:515)
```

Co-authored-by: Sung Yoon Whang <sungyoon@uber.com>
  • Loading branch information
josephinedotlee and sywhang authored Mar 15, 2022
1 parent 8dcf217 commit 42c4022
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
38 changes: 38 additions & 0 deletions dig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2417,6 +2417,30 @@ func testProvideFailures(t *testing.T, dryRun bool) {
assert.Contains(t, err.Error(), "cannot provide io.Reader")
assert.Contains(t, err.Error(), "already provided")
})

t.Run("error should refer to location given by LocationForPC ProvideOption", func(t *testing.T) {
c := digtest.New(t)
type A struct{ idx int }
type ret struct {
dig.Out

A1 A // same type A provided twice
A2 A
}

locationFn := func() {}

err := c.Provide(func() ret {
return ret{
A1: A{idx: 1},
A2: A{idx: 2},
}
}, dig.LocationForPC(reflect.ValueOf(locationFn).Pointer()))
require.Error(t, err, "provide must return error")
dig.AssertErrorMatches(t, err,
`cannot provide function "go.uber.org/dig_test".testProvideFailures.func\d+.1`,
)
})
}

func TestInvokeFailures(t *testing.T) {
Expand Down Expand Up @@ -3515,6 +3539,20 @@ func TestEndToEndSuccessWithAliases(t *testing.T) {
)
})

t.Run("duplicate provide with LocationForPC", func(t *testing.T) {
c := digtest.New(t)
c.RequireProvide(func(x int) float64 {
return testStruct{}.TestMethod(x)
}, dig.LocationForPC(reflect.TypeOf(testStruct{}).Method(0).Func.Pointer()))
err := c.Provide(func(x int) float64 {
return testStruct{}.TestMethod(x)
}, dig.LocationForPC(reflect.TypeOf(testStruct{}).Method(0).Func.Pointer()))

require.Error(t, err)
require.Contains(t, err.Error(), `cannot provide function "go.uber.org/dig_test".testStruct.TestMethod`)
require.Contains(t, err.Error(), `already provided by "go.uber.org/dig_test".testStruct.TestMethod`)
})

t.Run("named instances", func(t *testing.T) {
c := digtest.New(t)
type A1 struct{ s string }
Expand Down
9 changes: 8 additions & 1 deletion provide.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,15 @@ func (s *Scope) Provide(constructor interface{}, opts ...ProvideOption) error {
}

if err := s.provide(constructor, options); err != nil {
var errFunc *digreflect.Func
if options.Location == nil {
errFunc = digreflect.InspectFunc(constructor)
} else {
errFunc = options.Location
}

return errProvide{
Func: digreflect.InspectFunc(constructor),
Func: errFunc,
Reason: err,
}
}
Expand Down

0 comments on commit 42c4022

Please sign in to comment.