Repository: uber-go/fx
Pull request #831 (2022-02-08 00:05:49)

fx.Annotate: make variadic params optional by default
If annotate is passed a variadic function, the dependency
listed as the variadic parameter should be optional
unless otherwise specified with the `optional:false`
parameter tag. This commit addresses that bug.

Resolves GO-1169

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

[![Impacted file tree graph](https://codecov.io/gh/uber-go/fx/pull/831/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/831?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     #831      +/-   ##
==========================================
- Coverage   99.27%   99.07%   -0.21%     
==========================================
  Files          25       25              
  Lines         972      977       +5     
==========================================
+ Hits          965      968       +3     
- Misses          6        7       +1     
- Partials        1        2       +1     
```


| [Impacted Files](https://codecov.io/gh/uber-go/fx/pull/831?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/831/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-YW5ub3RhdGVkLmdv) | `99.40% <100.00%> (+0.01%)` | :arrow_up: |
| [app.go](https://codecov.io/gh/uber-go/fx/pull/831/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-YXBwLmdv) | `99.00% <0.00%> (-0.67%)` | :arrow_down: |

------

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

abhinav: oops i broke it

Patch:
diff --git a/annotated.go b/annotated.go
index a3f1aa5..06a15ff 100644
--- a/annotated.go
+++ b/annotated.go
@@ -360,7 +360,7 @@ func (ann *annotated) parameters() (
 
 	// No parameter annotations. Return the original types
 	// and an identity function.
-	if len(ann.ParamTags) == 0 {
+	if len(ann.ParamTags) == 0 && !ft.IsVariadic() {
 		return types, func(args []reflect.Value) []reflect.Value {
 			return args
 		}
@@ -383,6 +383,11 @@ func (ann *annotated) parameters() (
 
 		if i < len(ann.ParamTags) {
 			field.Tag = reflect.StructTag(ann.ParamTags[i])
+		} else if i == ft.NumIn()-1 && ft.IsVariadic() {
+			// If a variadic argument is unannotated, mark it optional,
+			// so that just wrapping a function in fx.Annotate does not
+			// suddenly introduce a required []arg dependency.
+			field.Tag = reflect.StructTag(`optional:"true"`)
 		}
 
 		inFields = append(inFields, field)


Test patch:
diff --git a/annotated_test.go b/annotated_test.go
index 583c890..19bed2c 100644
--- a/annotated_test.go
+++ b/annotated_test.go
@@ -522,6 +522,10 @@ func TestAnnotate(t *testing.T) {
 	newSliceA := func(sa ...*a) *sliceA {
 		return &sliceA{sa}
 	}
+	newSliceAWithB := func(b *b, sa ...*a) *sliceA {
+		total := append(sa, b.a)
+		return &sliceA{total}
+	}
 
 	t.Run("Provide with optional", func(t *testing.T) {
 		t.Parallel()
@@ -599,6 +603,72 @@ func TestAnnotate(t *testing.T) {
 		assert.Len(t, got.sa, 2)
 	})
 
+	t.Run("Provide variadic function with no optional params", func(t *testing.T) {
+		t.Parallel()
+
+		var got struct {
+			fx.In
+
+			Result *sliceA `name:"as"`
+		}
+		app := fxtest.New(t,
+			fx.Supply([]*a{{}, {}, {}}),
+			fx.Provide(
+				fx.Annotate(newSliceA,
+					fx.ResultTags(`name:"as"`),
+				),
+			),
+			fx.Populate(&got),
+		)
+		defer app.RequireStart().RequireStop()
+		require.NoError(t, app.Err())
+		assert.Len(t, got.Result.sa, 3)
+	})
+
+	t.Run("Provide variadic function named with no given params", func(t *testing.T) {
+		t.Parallel()
+
+		var got *sliceA
+		app := NewForTest(t,
+			fx.Provide(
+				fx.Annotate(newSliceA, fx.ParamTags(`name:"a"`)),
+			),
+			fx.Populate(&got),
+		)
+		err := app.Err()
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), `missing dependencies`)
+		assert.Contains(t, err.Error(), `missing type: []*fx_test.a[name="a"]`)
+	})
+
+	t.Run("Invoke variadic function with multiple params", func(t *testing.T) {
+		t.Parallel()
+
+		app := fxtest.New(t,
+			fx.Supply(
+				fx.Annotate(newB(newA()), fx.ResultTags(`name:"b"`)),
+			),
+			fx.Invoke(fx.Annotate(newSliceAWithB, fx.ParamTags(`name:"b"`))),
+		)
+
+		defer app.RequireStart().RequireStop()
+		require.NoError(t, app.Err())
+	})
+
+	t.Run("Invoke non-optional variadic function with a missing dependency", func(t *testing.T) {
+		t.Parallel()
+
+		app := NewForTest(t,
+			fx.Invoke(
+				fx.Annotate(newSliceA, fx.ParamTags(`optional:"false"`)),
+			),
+		)
+		err := app.Err()
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), `missing dependencies`)
+		assert.Contains(t, err.Error(), `missing type: []*fx_test.a`)
+	})
+
 	t.Run("Invoke with variadic function", func(t *testing.T) {
 		t.Parallel()