From 7bb5b404dfed59b9ebc97e5e8bfaf37f6b1affce Mon Sep 17 00:00:00 2001 From: josephinedotlee <84732210+josephinedotlee@users.noreply.github.com> Date: Tue, 22 Feb 2022 10:49:07 -0800 Subject: [PATCH] Make provide's errors more expressive for fx.Annotate-ed functions (#844) * Make provide's errors more expressive for fx.Annotate-ed functions Because of the use of reflection in fx.Annotate, the error messages for provide tend to refer to the annotated function as `"reflect".makeFuncStub` which is not helpful for debugging. This change updates error messages that previously looked like: ``` Failed: cannot provide function "reflect".makeFuncStub (/usr/local/go/src/reflect/asm_arm64.s:12): cannot provide *fx_test.Logger from [0].Field0: already provided by "reflect".makeFuncStub (/usr/local/go/src/reflect/asm_arm64.s:12) ``` To looking like this: ``` Failed: cannot provide function "reflect".makeFuncStub (/usr/local/go/src/reflect/asm_amd64.s:12): cannot provide *fx_test.Logger from [0].Field0: already provided by "go.uber.org/fx_test".TestAnnotate.func1 (/gocode/src/github.com/uber-go/fx/annotated_test.go:515) ``` WIP: Planning on making a future fix that gives a more readable name to the first function which still is referred to as "reflect".makeFuncStub after this change. * Switch to storing ptr value instead of ProvideOption Co-authored-by: Moises Vega * Switch to storing ptr value instead of ProvideOption Co-authored-by: Moises Vega * Switch to storing ptr value instead of ProvideOption Co-authored-by: Moises Vega * Update annotated_test.go test naming Co-authored-by: Moises Vega * Fix lint issue Co-authored-by: Moises Vega Co-authored-by: Sung Yoon Whang --- annotated.go | 2 ++ annotated_test.go | 16 ++++++++++++++++ provide.go | 1 + 3 files changed, 19 insertions(+) diff --git a/annotated.go b/annotated.go index 06a15fff2..85a8ec0e6 100644 --- a/annotated.go +++ b/annotated.go @@ -264,6 +264,7 @@ type annotated struct { ParamTags []string ResultTags []string As [][]reflect.Type + FuncPtr uintptr } func (ann annotated) String() string { @@ -302,6 +303,7 @@ func (ann *annotated) Build() (interface{}, error) { newFnType := reflect.FuncOf(paramTypes, resultTypes, false) origFn := reflect.ValueOf(ann.Target) + ann.FuncPtr = origFn.Pointer() newFn := reflect.MakeFunc(newFnType, func(args []reflect.Value) []reflect.Value { args = remapParams(args) var results []reflect.Value diff --git a/annotated_test.go b/annotated_test.go index 19bed2caf..f8ba3b9e1 100644 --- a/annotated_test.go +++ b/annotated_test.go @@ -787,6 +787,22 @@ func TestAnnotate(t *testing.T) { defer app.RequireStart().RequireStop() }) + t.Run("provide an already provided function using Annotate", func(t *testing.T) { + t.Parallel() + + app := NewForTest(t, + fx.Provide(fx.Annotate(newA, fx.ResultTags(`name:"a"`))), + fx.Provide(fx.Annotate(newA, fx.ResultTags(`name:"a"`))), + fx.Invoke( + fx.Annotate(newB, fx.ParamTags(`name:"a"`)), + ), + ) + err := app.Err() + require.Error(t, err) + assert.Contains(t, err.Error(), "already provided") + assert.Contains(t, err.Error(), "\"go.uber.org/fx_test\".TestAnnotate.func1") + }) + t.Run("specify more ParamTags than Params", func(t *testing.T) { t.Parallel() diff --git a/provide.go b/provide.go index 52b01e317..c37b8c0bc 100644 --- a/provide.go +++ b/provide.go @@ -109,6 +109,7 @@ func runProvide(c container, p provide, opts ...dig.ProvideOption) error { return fmt.Errorf("fx.Provide(%v) from:\n%+vFailed: %v", constructor, p.Stack, err) } + opts = append(opts, dig.LocationForPC(constructor.FuncPtr)) if err := c.Provide(ctor, opts...); err != nil { return fmt.Errorf("fx.Provide(%v) from:\n%+vFailed: %v", constructor, p.Stack, err) }