Repository: uber-go/dig
Pull request #304 (2021-12-06 17:41:23)

Options: Implement String()
Change option implementations from function pointers
to objects implementing `String()`.

This makes it easier to debug options by printing them
and aligns with the recommended pattern for functional options
in [our style guide].

  [our style guide]: https://github.com/uber-go/guide/blob/master/style.md#functional-options


codecov[bot]: # [Codecov](https://codecov.io/gh/uber-go/dig/pull/304?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) Report
> Merging [#304](https://codecov.io/gh/uber-go/dig/pull/304?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) (382f95d) into [master](https://codecov.io/gh/uber-go/dig/commit/d83e15908afa3f383f1d03a61d23fe453e86129b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) (d83e159) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/uber-go/dig/pull/304/graphs/tree.svg?width=650&height=150&src=pr&token=ARuH7Frwgw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go)](https://codecov.io/gh/uber-go/dig/pull/304?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     #304   +/-   ##
=======================================
  Coverage   98.54%   98.55%           
=======================================
  Files          18       18           
  Lines        1172     1176    +4     
=======================================
+ Hits         1155     1159    +4     
  Misses         10       10           
  Partials        7        7           
```


| [Impacted Files](https://codecov.io/gh/uber-go/dig/pull/304?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) | Coverage Δ | |
|---|---|---|
| [container.go](https://codecov.io/gh/uber-go/dig/pull/304/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-Y29udGFpbmVyLmdv) | `100.00% <100.00%> (ø)` | |
| [provide.go](https://codecov.io/gh/uber-go/dig/pull/304/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-cHJvdmlkZS5nbw==) | `100.00% <100.00%> (ø)` | |
| [visualize.go](https://codecov.io/gh/uber-go/dig/pull/304/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-dmlzdWFsaXplLmdv) | `89.47% <100.00%> (-0.27%)` | :arrow_down: |

------

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


Patch:
diff --git a/container.go b/container.go
index f4f5955..4b2045f 100644
--- a/container.go
+++ b/container.go
@@ -62,10 +62,6 @@ type Option interface {
 	applyOption(*Container)
 }
 
-type optionFunc func(*Container)
-
-func (f optionFunc) applyOption(c *Container) { f(c) }
-
 // Container is a directed acyclic graph of types and their dependencies.
 type Container struct {
 	// Mapping from key to all the constructor node that can provide a value for that
@@ -166,30 +162,54 @@ func New(opts ...Option) *Container {
 // Applications adding providers to a container in a tight loop may experience
 // performance improvements by initializing the container with this option.
 func DeferAcyclicVerification() Option {
-	return optionFunc(func(c *Container) {
-		c.deferAcyclicVerification = true
-	})
+	return deferAcyclicVerificationOption{}
+}
+
+type deferAcyclicVerificationOption struct{}
+
+func (deferAcyclicVerificationOption) String() string {
+	return "DeferAcyclicVerification()"
+}
+
+func (deferAcyclicVerificationOption) applyOption(c *Container) {
+	c.deferAcyclicVerification = true
 }
 
 // Changes the source of randomness for the container.
 //
 // This will help provide determinism during tests.
 func setRand(r *rand.Rand) Option {
-	return optionFunc(func(c *Container) {
-		c.rand = r
-	})
+	return setRandOption{r: r}
+}
+
+type setRandOption struct{ r *rand.Rand }
+
+func (o setRandOption) String() string {
+	return fmt.Sprintf("setRand(%p)", o.r)
+}
+
+func (o setRandOption) applyOption(c *Container) {
+	c.rand = o.r
 }
 
 // DryRun is an Option which, when set to true, disables invocation of functions supplied to
 // Provide and Invoke. Use this to build no-op containers.
 func DryRun(dry bool) Option {
-	return optionFunc(func(c *Container) {
-		if dry {
-			c.invokerFn = dryInvoker
-		} else {
-			c.invokerFn = defaultInvoker
-		}
-	})
+	return dryRunOption(dry)
+}
+
+type dryRunOption bool
+
+func (o dryRunOption) String() string {
+	return fmt.Sprintf("DryRun(%v)", bool(o))
+}
+
+func (o dryRunOption) applyOption(c *Container) {
+	if o {
+		c.invokerFn = dryInvoker
+	} else {
+		c.invokerFn = defaultInvoker
+	}
 }
 
 // invokerFn specifies how the container calls user-supplied functions.
diff --git a/provide.go b/provide.go
index ec52c7b..40d3d86 100644
--- a/provide.go
+++ b/provide.go
@@ -21,6 +21,7 @@
 package dig
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"reflect"
@@ -86,10 +87,6 @@ func (o *provideOptions) Validate() error {
 	return nil
 }
 
-type provideOptionFunc func(*provideOptions)
-
-func (f provideOptionFunc) applyProvideOption(opts *provideOptions) { f(opts) }
-
 // Name is a ProvideOption that specifies that all values produced by a
 // constructor should have the given name. See also the package documentation
 // about Named Values.
@@ -108,9 +105,17 @@ func (f provideOptionFunc) applyProvideOption(opts *provideOptions) { f(opts) }
 // This option cannot be provided for constructors which produce result
 // objects.
 func Name(name string) ProvideOption {
-	return provideOptionFunc(func(opts *provideOptions) {
-		opts.Name = name
-	})
+	return provideNameOption(name)
+}
+
+type provideNameOption string
+
+func (o provideNameOption) String() string {
+	return fmt.Sprintf("Name(%q)", string(o))
+}
+
+func (o provideNameOption) applyProvideOption(opt *provideOptions) {
+	opt.Name = string(o)
 }
 
 // Group is a ProvideOption that specifies that all values produced by a
@@ -120,9 +125,17 @@ func Name(name string) ProvideOption {
 // This option cannot be provided for constructors which produce result
 // objects.
 func Group(group string) ProvideOption {
-	return provideOptionFunc(func(opts *provideOptions) {
-		opts.Group = group
-	})
+	return provideGroupOption(group)
+}
+
+type provideGroupOption string
+
+func (o provideGroupOption) String() string {
+	return fmt.Sprintf("Group(%q)", string(o))
+}
+
+func (o provideGroupOption) applyProvideOption(opt *provideOptions) {
+	opt.Group = string(o)
 }
 
 // ID is a unique integer representing the constructor node in the dependency graph.
@@ -189,9 +202,17 @@ func (o *Output) String() string {
 // FillProvideInfo is a ProvideOption that writes info on what Dig was able to get out
 // out of the provided constructor into the provided ProvideInfo.
 func FillProvideInfo(info *ProvideInfo) ProvideOption {
-	return provideOptionFunc(func(opts *provideOptions) {
-		opts.Info = info
-	})
+	return fillProvideInfoOption{info: info}
+}
+
+type fillProvideInfoOption struct{ info *ProvideInfo }
+
+func (o fillProvideInfoOption) String() string {
+	return fmt.Sprintf("FillProvideInfo(%p)", o.info)
+}
+
+func (o fillProvideInfoOption) applyProvideOption(opts *provideOptions) {
+	opts.Info = o.info
 }
 
 // As is a ProvideOption that specifies that the value produced by the
@@ -237,9 +258,25 @@ func FillProvideInfo(info *ProvideInfo) ProvideOption {
 // This option cannot be provided for constructors which produce result
 // objects.
 func As(i ...interface{}) ProvideOption {
-	return provideOptionFunc(func(opts *provideOptions) {
-		opts.As = append(opts.As, i...)
-	})
+	return provideAsOption(i)
+}
+
+type provideAsOption []interface{}
+
+func (o provideAsOption) String() string {
+	buf := bytes.NewBufferString("As(")
+	for i, iface := range o {
+		if i > 0 {
+			buf.WriteString(", ")
+		}
+		buf.WriteString(reflect.TypeOf(iface).Elem().String())
+	}
+	buf.WriteString(")")
+	return buf.String()
+}
+
+func (o provideAsOption) applyProvideOption(opts *provideOptions) {
+	opts.As = append(opts.As, o...)
 }
 
 // LocationForPC is a ProvideOption which specifies an alternate function program
@@ -249,9 +286,19 @@ func As(i ...interface{}) ProvideOption {
 // with the reflect.MakeFunc method whose error messages are otherwise hard to
 // understand
 func LocationForPC(pc uintptr) ProvideOption {
-	return provideOptionFunc(func(opts *provideOptions) {
-		opts.Location = digreflect.InspectFuncPC(pc)
-	})
+	return provideLocationOption{
+		loc: digreflect.InspectFuncPC(pc),
+	}
+}
+
+type provideLocationOption struct{ loc *digreflect.Func }
+
+func (o provideLocationOption) String() string {
+	return fmt.Sprintf("LocationForPC(%v)", o.loc)
+}
+
+func (o provideLocationOption) applyProvideOption(opts *provideOptions) {
+	opts.Location = o.loc
 }
 
 // provider encapsulates a user-provided constructor.
diff --git a/visualize.go b/visualize.go
index 8086158..372712f 100644
--- a/visualize.go
+++ b/visualize.go
@@ -21,6 +21,7 @@
 package dig
 
 import (
+	"fmt"
 	"io"
 	"strconv"
 	"text/template"
@@ -37,10 +38,6 @@ type visualizeOptions struct {
 	VisualizeError error
 }
 
-type visualizeOptionFunc func(*visualizeOptions)
-
-func (f visualizeOptionFunc) applyVisualizeOption(opts *visualizeOptions) { f(opts) }
-
 // VisualizeError includes a visualization of the given error in the output of
 // Visualize if an error was returned by Invoke or Provide.
 //
@@ -51,9 +48,17 @@ func (f visualizeOptionFunc) applyVisualizeOption(opts *visualizeOptions) { f(op
 // This option has no effect if the error was nil or if it didn't contain any
 // information to visualize.
 func VisualizeError(err error) VisualizeOption {
-	return visualizeOptionFunc(func(opts *visualizeOptions) {
-		opts.VisualizeError = err
-	})
+	return visualizeErrorOption{err}
+}
+
+type visualizeErrorOption struct{ err error }
+
+func (o visualizeErrorOption) String() string {
+	return fmt.Sprintf("VisualizeError(%v)", o.err)
+}
+
+func (o visualizeErrorOption) applyVisualizeOption(opt *visualizeOptions) {
+	opt.VisualizeError = o.err
 }
 
 func updateGraph(dg *dot.Graph, err error) error {


Test patch:
diff --git a/container_test.go b/container_test.go
new file mode 100644
index 0000000..612172f
--- /dev/null
+++ b/container_test.go
@@ -0,0 +1,64 @@
+// Copyright (c) 2021 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package dig
+
+import (
+	"fmt"
+	"math/rand"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestOptionStrings(t *testing.T) {
+	t.Parallel()
+
+	t.Run("DeferAcyclicVerification", func(t *testing.T) {
+		t.Parallel()
+
+		assert.Equal(t, "DeferAcyclicVerification()", fmt.Sprint(DeferAcyclicVerification()))
+	})
+
+	t.Run("setRand", func(t *testing.T) {
+		t.Parallel()
+
+		t.Run("nil", func(t *testing.T) {
+			t.Parallel()
+
+			assert.Equal(t, "setRand(0x0)", fmt.Sprint(setRand(nil)))
+		})
+
+		t.Run("non nil", func(t *testing.T) {
+			t.Parallel()
+
+			opt := setRand(rand.New(rand.NewSource(42)))
+			assert.NotEqual(t, "setRand(0x0)", fmt.Sprint(opt))
+			assert.Contains(t, fmt.Sprint(opt), "setRand(0x")
+		})
+	})
+
+	t.Run("DryRun", func(t *testing.T) {
+		t.Parallel()
+
+		assert.Equal(t, "DryRun(true)", fmt.Sprint(DryRun(true)))
+		assert.Equal(t, "DryRun(false)", fmt.Sprint(DryRun(false)))
+	})
+}
diff --git a/provide_test.go b/provide_test.go
new file mode 100644
index 0000000..9cc317a
--- /dev/null
+++ b/provide_test.go
@@ -0,0 +1,88 @@
+// Copyright (c) 2021 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package dig
+
+import (
+	"fmt"
+	"io"
+	"reflect"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestProvideOptionStrings(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		desc string
+		give ProvideOption
+		want string
+	}{
+		{
+			desc: "Name",
+			give: Name("foo"),
+			want: `Name("foo")`,
+		},
+		{
+			desc: "Group",
+			give: Group("bar"),
+			want: `Group("bar")`,
+		},
+		{
+			desc: "As",
+			give: As(new(io.Reader), new(io.Writer)),
+			want: `As(io.Reader, io.Writer)`,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.desc, func(t *testing.T) {
+			t.Parallel()
+
+			assert.Equal(t, tt.want, fmt.Sprint(tt.give))
+		})
+	}
+}
+
+func TestFillProvideInfoString(t *testing.T) {
+	t.Parallel()
+
+	t.Run("nil", func(t *testing.T) {
+		t.Parallel()
+
+		assert.Equal(t, "FillProvideInfo(0x0)", fmt.Sprint(FillProvideInfo(nil)))
+	})
+
+	t.Run("not nil", func(t *testing.T) {
+		t.Parallel()
+
+		opt := FillProvideInfo(new(ProvideInfo))
+		assert.NotEqual(t, fmt.Sprint(opt), "FillProvideInfo(0x0)")
+		assert.Contains(t, fmt.Sprint(opt), "FillProvideInfo(0x")
+	})
+}
+
+func TestLocationForPCString(t *testing.T) {
+	opt := LocationForPC(reflect.ValueOf(func() {}).Pointer())
+	assert.Contains(t, fmt.Sprint(opt), `LocationForPC("go.uber.org/dig".TestLocationForPCString.func1 `)
+}
diff --git a/visualize_test.go b/visualize_test.go
index d344db6..d87380f 100644
--- a/visualize_test.go
+++ b/visualize_test.go
@@ -1,4 +1,4 @@
-// Copyright (c) 2019 Uber Technologies, Inc.
+// Copyright (c) 2021 Uber Technologies, Inc.
 //
 // Permission is hereby granted, free of charge, to any person obtaining a copy
 // of this software and associated documentation files (the "Software"), to deal
@@ -22,6 +22,7 @@ package dig
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"reflect"
@@ -643,3 +644,21 @@ func TestCanVisualizeError(t *testing.T) {
 		})
 	}
 }
+
+func TestVisualizeErrorString(t *testing.T) {
+	t.Parallel()
+
+	t.Run("nil", func(t *testing.T) {
+		t.Parallel()
+
+		opt := VisualizeError(nil)
+		assert.Equal(t, "VisualizeError(<nil>)", fmt.Sprint(opt))
+	})
+
+	t.Run("not nil", func(t *testing.T) {
+		t.Parallel()
+
+		opt := VisualizeError(errors.New("great sadness"))
+		assert.Equal(t, "VisualizeError(great sadness)", fmt.Sprint(opt))
+	})
+}