Repository: uber-go/fx
Pull request #809 (2021-11-14 09:21:25)

fx.Annotate: Support variadic functions
Add support to fx.Annotate for variadic functions.
The last parameter of variadic functions is now treated as a slice.
With this users can feed value groups into variadic functions.

Ref https://github.com/uber-go/dig/issues/120

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/uber-go/fx?pullRequest=809) <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/fx?pullRequest=809) 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/fx?pullRequest=809) it.</sub>
codecov[bot]: # [Codecov](https://codecov.io/gh/uber-go/fx/pull/809?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) Report
> Merging [#809](https://codecov.io/gh/uber-go/fx/pull/809?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) (4b7dd31) into [master](https://codecov.io/gh/uber-go/fx/commit/cc87b094398dbab9a6bf89cbe92157e4f5f3ceab?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) (cc87b09) will **decrease** coverage by `0.31%`.
> The diff coverage is `57.14%`.

[![Impacted file tree graph](https://codecov.io/gh/uber-go/fx/pull/809/graphs/tree.svg?width=650&height=150&src=pr&token=PYY9PSfaC6&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go)](https://codecov.io/gh/uber-go/fx/pull/809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go)

```diff
@@            Coverage Diff             @@
##           master     #809      +/-   ##
==========================================
- Coverage   99.25%   98.94%   -0.32%     
==========================================
  Files          24       24              
  Lines         941      945       +4     
==========================================
+ Hits          934      935       +1     
- Misses          6        7       +1     
- Partials        1        3       +2     
```


| [Impacted Files](https://codecov.io/gh/uber-go/fx/pull/809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) | Coverage Δ | |
|---|---|---|
| [annotated.go](https://codecov.io/gh/uber-go/fx/pull/809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-YW5ub3RhdGVkLmdv) | `97.14% <57.14%> (-2.13%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber-go/fx/pull/809?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/fx/pull/809?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). Last update [cc87b09...4b7dd31](https://codecov.io/gh/uber-go/fx/pull/809?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).

xqbumu: > 
> 
> Thanks for this change. Looks good to me

There is something wrong that 'value groups cannot be optional' when use optional tags in ParamTags.
And that message is from dig where we can delve deeper.

Refer: https://github.com/uber-go/dig/issues/120

abhinav: Hey, on a second look, this doesn't match the behavior of dig.
Dig specifically just ignores variadic arguments.
That is, if a function accepts variadic arguments, dig will fill everything except that.
I originally thought that's what this change was doing, but the test indicates otherwise.

If we want to support filling variadic arguments with value groups using this,
we can do that, but we should make that choice explicitly.
CC @sywhang @shirchen 

RE: value groups cannot be optional:
They cannot be optional, in that you cannot tag them optional.
They are already optional, in that if no values are provided to that group, you get an empty slice.
xqbumu: > 
> 
> Hey, on a second look, this doesn't match the behavior of dig. Dig specifically just ignores variadic arguments. That is, if a function accepts variadic arguments, dig will fill everything except that. I originally thought that's what this change was doing, but the test indicates otherwise.
> 
> If we want to support filling variadic arguments with value groups using this, we can do that, but we should make that choice explicitly. CC @sywhang @shirchen
> 
> RE: value groups cannot be optional: They cannot be optional, in that you cannot tag them optional. They are already optional, in that if no values are provided to that group, you get an empty slice.

Oh, I got it, very thanks for you explain. 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1031a42..418203f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
-- No changes yet.
+### Changed
+- `fx.Annotate`: support variadic functions, and feeding value groups into
+  them.
 
 ## [1.15.0] - 2021-11-08
 ### Added
diff --git a/annotated.go b/annotated.go
index b4ebc47..430fb54 100644
--- a/annotated.go
+++ b/annotated.go
@@ -287,7 +287,8 @@ func (ann annotated) String() string {
 // Build builds and returns a constructor based on fx.In/fx.Out params and
 // results wrapping the original constructor passed to fx.Annotate.
 func (ann *annotated) Build() (interface{}, error) {
-	if reflect.TypeOf(ann.Target).Kind() != reflect.Func {
+	ft := reflect.TypeOf(ann.Target)
+	if ft.Kind() != reflect.Func {
 		return nil, fmt.Errorf("must provide constructor function, got %v (%T)", ann.Target, ann.Target)
 	}
 
@@ -301,7 +302,12 @@ func (ann *annotated) Build() (interface{}, error) {
 	origFn := reflect.ValueOf(ann.Target)
 	newFn := reflect.MakeFunc(newFnType, func(args []reflect.Value) []reflect.Value {
 		args = remapParams(args)
-		results := origFn.Call(args)
+		var results []reflect.Value
+		if ft.IsVariadic() {
+			results = origFn.CallSlice(args)
+		} else {
+			results = origFn.Call(args)
+		}
 		results = remapResults(results)
 		return results
 	})
@@ -522,6 +528,42 @@ func (ann *annotated) results() (
 //
 // If more tags are given than the number of parameters/results, only
 // the ones up to the number of parameters/results will be applied.
+//
+// Variadic functions
+//
+// If the provided function is variadic, Annotate treats its parameter as a
+// slice. For example,
+//
+//  fx.Annotate(func(w io.Writer, rs ...io.Reader) {
+//    // ...
+//  }, ...)
+//
+// Is equivalent to,
+//
+//  fx.Annotate(func(w io.Writer, rs []io.Reader) {
+//    // ...
+//  }, ...)
+//
+// You can use variadic parameters with Fx's value groups.
+// For example,
+//
+//  fx.Annotate(func(mux *http.ServeMux, handlers ...http.Handler) {
+//    // ...
+//  }, fx.ParamTags(``, `group:"server"`))
+//
+// If we provide the above to the application,
+// any constructor in the Fx application can inject its HTTP handlers
+// by using fx.Annotate, fx.Annotated, or fx.Out.
+//
+//  fx.Annotate(
+//    func(..) http.Handler { ... },
+//    fx.ResultTags(`group:"server"`),
+//  )
+//
+//  fx.Annotated{
+//    Target: func(..) http.Handler { ... },
+//    Group:  "server",
+//  }
 func Annotate(t interface{}, anns ...Annotation) interface{} {
 	result := annotated{Target: t}
 	for _, ann := range anns {


Test patch:
diff --git a/annotated_test.go b/annotated_test.go
index be7362e..18ecd42 100644
--- a/annotated_test.go
+++ b/annotated_test.go
@@ -430,6 +430,7 @@ func TestAnnotate(t *testing.T) {
 	type a struct{}
 	type b struct{ a *a }
 	type c struct{ b *b }
+	type sliceA struct{ sa []*a }
 	newA := func() *a { return &a{} }
 	newB := func(a *a) *b {
 		return &b{a}
@@ -437,6 +438,9 @@ func TestAnnotate(t *testing.T) {
 	newC := func(b *b) *c {
 		return &c{b}
 	}
+	newSliceA := func(sa ...*a) *sliceA {
+		return &sliceA{sa}
+	}
 
 	t.Run("Provide with optional", func(t *testing.T) {
 		t.Parallel()
@@ -494,6 +498,46 @@ func TestAnnotate(t *testing.T) {
 		assert.Contains(t, err.Error(), `missing type: *fx_test.a[name="a"]`)
 	})
 
+	t.Run("Provide with variadic function", func(t *testing.T) {
+		t.Parallel()
+
+		var got *sliceA
+		app := fxtest.New(t,
+			fx.Provide(
+				fx.Annotated{Group: "as", Target: newA},
+				fx.Annotated{Group: "as", Target: newA},
+				fx.Annotate(newSliceA,
+					fx.ParamTags(`group:"as"`),
+				),
+			),
+			fx.Populate(&got),
+		)
+		defer app.RequireStart().RequireStop()
+		require.NoError(t, app.Err())
+
+		assert.Len(t, got.sa, 2)
+	})
+
+	t.Run("Invoke with variadic function", func(t *testing.T) {
+		t.Parallel()
+
+		type T1 struct{ s string }
+
+		app := fxtest.New(t,
+			fx.Supply(
+				fx.Annotate(T1{"foo"}, fx.ResultTags(`group:"t"`)),
+				fx.Annotate(T1{"bar"}, fx.ResultTags(`group:"t"`)),
+				fx.Annotate(T1{"baz"}, fx.ResultTags(`group:"t"`)),
+			),
+			fx.Invoke(fx.Annotate(func(got ...T1) {
+				assert.ElementsMatch(t, []T1{{"foo"}, {"bar"}, {"baz"}}, got)
+			}, fx.ParamTags(`group:"t"`))),
+		)
+
+		defer app.RequireStart().RequireStop()
+		require.NoError(t, app.Err())
+	})
+
 	t.Run("provide with annotated results", func(t *testing.T) {
 		t.Parallel()