Repository: uber-go/dig
Pull request #274 (2021-02-23 01:27:02)

Add ability to allow unexported fields in dig.In structs
In order for unexported fields to be allowed, an 
"allowUnexported=true" struct tag should be used on the embedded dig.In 
field.

Resolves: https://github.com/uber-go/dig/issues/273

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/uber-go/dig?pullRequest=274) <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/dig?pullRequest=274) 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/dig?pullRequest=274) it.</sub>
codecov[bot]: # [Codecov](https://codecov.io/gh/uber-go/dig/pull/274?src=pr&el=h1) Report
> Merging [#274](https://codecov.io/gh/uber-go/dig/pull/274?src=pr&el=desc) (1063373) into [master](https://codecov.io/gh/uber-go/dig/commit/4ea249947c78543420fc8c36620326fb811a0094?el=desc) (4ea2499) will **decrease** coverage by `0.18%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/uber-go/dig/pull/274/graphs/tree.svg?width=650&height=150&src=pr&token=ARuH7Frwgw)](https://codecov.io/gh/uber-go/dig/pull/274?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
- Coverage   98.14%   97.96%   -0.19%     
==========================================
  Files          13       13              
  Lines        1297     1079     -218     
==========================================
- Hits         1273     1057     -216     
+ Misses         18       16       -2     
  Partials        6        6              
```


| [Impacted Files](https://codecov.io/gh/uber-go/dig/pull/274?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [dig.go](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree#diff-ZGlnLmdv) | `100.00% <100.00%> (ø)` | |
| [param.go](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree#diff-cGFyYW0uZ28=) | `94.94% <100.00%> (-0.49%)` | :arrow_down: |
| [graph.go](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree#diff-Z3JhcGguZ28=) | `89.74% <0.00%> (-2.26%)` | :arrow_down: |
| [result.go](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree#diff-cmVzdWx0Lmdv) | `96.85% <0.00%> (-0.43%)` | :arrow_down: |
| [internal/dot/graph.go](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZG90L2dyYXBoLmdv) | `98.58% <0.00%> (-0.33%)` | :arrow_down: |
| [cycle.go](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree#diff-Y3ljbGUuZ28=) | `100.00% <0.00%> (ø)` | |
| [error.go](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree#diff-ZXJyb3IuZ28=) | `100.00% <0.00%> (ø)` | |
| [group.go](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree#diff-Z3JvdXAuZ28=) | `100.00% <0.00%> (ø)` | |
| ... and [5 more](https://codecov.io/gh/uber-go/dig/pull/274/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber-go/dig/pull/274?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/uber-go/dig/pull/274?src=pr&el=footer). Last update [4ea2499...1063373](https://codecov.io/gh/uber-go/dig/pull/274?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/dig.go b/dig.go
index 9e81923..8af25a8 100644
--- a/dig.go
+++ b/dig.go
@@ -35,8 +35,9 @@ import (
 )
 
 const (
-	_optionalTag = "optional"
-	_nameTag     = "name"
+	_optionalTag         = "optional"
+	_nameTag             = "name"
+	_ignoreUnexportedTag = "ignore-unexported"
 )
 
 // Unique identification of an object in the graph.
@@ -735,6 +736,24 @@ func isFieldOptional(f reflect.StructField) (bool, error) {
 	return optional, err
 }
 
+// Checks if ignoring unexported files in an In struct is allowed.
+// The struct field MUST be an _inType.
+func isIgnoreUnexportedSet(f reflect.StructField) (bool, error) {
+	tag := f.Tag.Get(_ignoreUnexportedTag)
+	if tag == "" {
+		return false, nil
+	}
+
+	allowed, err := strconv.ParseBool(tag)
+	if err != nil {
+		err = errf(
+			"invalid value %q for %q tag on field %v",
+			tag, _ignoreUnexportedTag, f.Name, err)
+	}
+
+	return allowed, err
+}
+
 // Checks that all direct dependencies of the provided param are present in
 // the container. Returns an error if not.
 func shallowCheckDependencies(c containerStore, p param) error {
diff --git a/param.go b/param.go
index 37b2343..0979228 100644
--- a/param.go
+++ b/param.go
@@ -287,13 +287,30 @@ func (po paramObject) DotParam() []*dot.Param {
 func newParamObject(t reflect.Type) (paramObject, error) {
 	po := paramObject{Type: t}
 
+	// Check if the In type supports ignoring unexported fields.
+	var ignoreUnexported bool
+	for i := 0; i < t.NumField(); i++ {
+		f := t.Field(i)
+		if f.Type == _inType {
+			var err error
+			ignoreUnexported, err = isIgnoreUnexportedSet(f)
+			if err != nil {
+				return po, err
+			}
+			break
+		}
+	}
+
 	for i := 0; i < t.NumField(); i++ {
 		f := t.Field(i)
 		if f.Type == _inType {
 			// Skip over the dig.In embed.
 			continue
 		}
-
+		if f.PkgPath != "" && ignoreUnexported {
+			// Skip over an unexported field if it is allowed.
+			continue
+		}
 		pof, err := newParamObjectField(i, f)
 		if err != nil {
 			return po, errf("bad field %q of %v", f.Name, t, err)


Test patch:
diff --git a/dig_test.go b/dig_test.go
index 44e2f1c..d991496 100644
--- a/dig_test.go
+++ b/dig_test.go
@@ -344,6 +344,30 @@ func TestEndToEndSuccess(t *testing.T) {
 		}))
 	})
 
+	t.Run("ignore unexported fields", func(t *testing.T) {
+		type type1 struct{}
+		type type2 struct{}
+		type type3 struct{}
+		constructor := func() (*type1, *type2, *type3) {
+			return &type1{}, &type2{}, &type3{}
+		}
+
+		c := New()
+		type param struct {
+			In `ignore-unexported:"true"`
+
+			T1 *type1 // regular 'ol type
+			T2 *type2 `optional:"true"` // optional type present in the graph
+			t3 *type3
+		}
+		require.NoError(t, c.Provide(constructor))
+		require.NoError(t, c.Invoke(func(p param) {
+			require.NotNil(t, p.T1, "whole param struct should not be nil")
+			assert.NotNil(t, p.T2, "optional type in the graph should not return nil")
+			assert.Nil(t, p.t3, "unexported field should not be set")
+		}))
+	})
+
 	t.Run("out type inserts multiple objects into the graph", func(t *testing.T) {
 		type A struct{ name string }
 		type B struct{ name string }
@@ -2912,3 +2936,57 @@ func BenchmarkProvideCycleDetection(b *testing.B) {
 		c.Provide(newA)
 	}
 }
+
+func TestUnexportedFieldsFailures(t *testing.T) {
+	t.Run("empty tag value", func(t *testing.T) {
+		type type1 struct{}
+		type type2 struct{}
+		type type3 struct{}
+		constructor := func() (*type1, *type2) {
+			return &type1{}, &type2{}
+		}
+
+		c := New()
+		type param struct {
+			In `ignore-unexported:""`
+
+			T1 *type1 // regular 'ol type
+			T2 *type2 `optional:"true"` // optional type present in the graph
+			t3 *type3
+		}
+		require.NoError(t, c.Provide(constructor))
+		err := c.Invoke(func(p param) {
+			require.NotNil(t, p.T1, "whole param struct should not be nil")
+			assert.NotNil(t, p.T2, "optional type in the graph should not return nil")
+		})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(),
+			`bad argument 1: bad field "t3" of dig.param: unexported fields not allowed in dig.In, did you mean to export "t3" (*dig.type3)`)
+	})
+
+	t.Run("invalid tag value", func(t *testing.T) {
+		type type1 struct{}
+		type type2 struct{}
+		type type3 struct{}
+		constructor := func() (*type1, *type2) {
+			return &type1{}, &type2{}
+		}
+
+		c := New()
+		type param struct {
+			In `ignore-unexported:"foo"`
+
+			T1 *type1 // regular 'ol type
+			T2 *type2 `optional:"true"` // optional type present in the graph
+			t3 *type3
+		}
+		require.NoError(t, c.Provide(constructor))
+		err := c.Invoke(func(p param) {
+			require.NotNil(t, p.T1, "whole param struct should not be nil")
+			assert.NotNil(t, p.T2, "optional type in the graph should not return nil")
+		})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(),
+			`bad argument 1: invalid value "foo" for "ignore-unexported" tag on field In: strconv.ParseBool: parsing "foo": invalid syntax`)
+	})
+}
diff --git a/param_test.go b/param_test.go
index 22e7dbd..68f0cde 100644
--- a/param_test.go
+++ b/param_test.go
@@ -101,6 +101,29 @@ func TestParamObjectSuccess(t *testing.T) {
 	})
 }
 
+func TestParamObjectWithUnexportedFieldsSuccess(t *testing.T) {
+	type type1 struct{}
+	type type2 struct{}
+
+	type in struct {
+		In `ignore-unexported:"true"`
+
+		T1 type1
+		t2 type2
+	}
+
+	po, err := newParamObject(reflect.TypeOf(in{}))
+	require.NoError(t, err)
+
+	require.Len(t, po.Fields, 1)
+
+	require.Equal(t, "T1", po.Fields[0].FieldName)
+	t1, ok := po.Fields[0].Param.(paramSingle)
+	require.True(t, ok, "T1 must be a paramSingle")
+	assert.Empty(t, t1.Name)
+	assert.False(t, t1.Optional)
+}
+
 func TestParamObjectFailure(t *testing.T) {
 	t.Run("unexported field gets an error", func(t *testing.T) {
 		type A struct{}
@@ -116,6 +139,36 @@ func TestParamObjectFailure(t *testing.T) {
 		assert.Contains(t, err.Error(),
 			`bad field "a2" of dig.in: unexported fields not allowed in dig.In, did you mean to export "a2" (dig.A)`)
 	})
+
+	t.Run("unexported field with empty tag value gets an error", func(t *testing.T) {
+		type A struct{}
+		type in struct {
+			In `ignore-unexported:""`
+
+			A1 A
+			a2 A
+		}
+
+		_, err := newParamObject(reflect.TypeOf(in{}))
+		require.Error(t, err)
+		assert.Contains(t, err.Error(),
+			`bad field "a2" of dig.in: unexported fields not allowed in dig.In, did you mean to export "a2" (dig.A)`)
+	})
+
+	t.Run("unexported field with invalid tag value gets an error", func(t *testing.T) {
+		type A struct{}
+		type in struct {
+			In `ignore-unexported:"foo"`
+
+			A1 A
+			a2 A
+		}
+
+		_, err := newParamObject(reflect.TypeOf(in{}))
+		require.Error(t, err)
+		assert.Contains(t, err.Error(),
+			`invalid value "foo" for "ignore-unexported" tag on field In: strconv.ParseBool: parsing "foo": invalid syntax`)
+	})
 }
 
 func TestParamGroupSliceErrors(t *testing.T) {