Skip to content

Commit

Permalink
Extend noinit to all pointer fields (#55)
Browse files Browse the repository at this point in the history
The initial implementation of noinit only applied to struct pointer fields. This extends the behavior to apply to any pointer fields.
  • Loading branch information
sethvargo committed May 27, 2022
1 parent 82b1ad8 commit e94cab6
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 28 deletions.
1 change: 1 addition & 0 deletions Makefile
Expand Up @@ -25,6 +25,7 @@ test-acc:
@go test \
-count=1 \
-race \
-shuffle=on \
-timeout=10m \
-vet="${VETTERS}" \
./...
Expand Down
51 changes: 34 additions & 17 deletions README.md
Expand Up @@ -260,23 +260,8 @@ Envconfig walks the entire struct, including nested structs, so deeply-nested
fields are also supported.
If a nested struct is a pointer type, it will automatically be instantianted to
the non-nil value. To disable this behavior, use the tag `noinit`. E.g.
```go
type ParentCfg struct {
// Without `noinit` tag, `Child` would be set to `&ChildCfg{}` whether
// or not `FIELD` is set in the env var.
// With `noinit`, `Child` would stay nil if `FIELD` is not set in the env var.
Child *ChildCfg `env:",noinit"`
}

type ChildCfg struct {
Field string `env:"FIELD"`
}
```
The `noinit` tag is only application for struct pointer fields. Put the tag on
non-struct-pointer will return an error.
the non-nil value. To change this behavior, see
(Initialization)[#Initialization].
### Custom
Expand Down Expand Up @@ -307,6 +292,38 @@ if err := envconfig.ProcessWith(ctx, &c, l); err != nil {
export APP_MYVAR="foo"
```
## Initialization
By default, all pointer fields are initialized (allocated) so they are not
`nil`. To disable this behavior, use the tag the field as `noinit`:
```go
type MyStruct struct {
// Without `noinit`, DeleteUser would be initialized to the default boolean
// value. With `noinit`, if the environment variable is not given, the value
// is kept as uninitialized (nil).
DeleteUser *bool `env:"DELETE_USER, noinit"`
}
```
This also applies to nested fields in a struct:
```go
type ParentConfig struct {
// Without `noinit` tag, `Child` would be set to `&ChildConfig{}` whether
// or not `FIELD` is set in the env var.
// With `noinit`, `Child` would stay nil if `FIELD` is not set in the env var.
Child *ChildConfig `env:",noinit"`
}

type ChildConfig struct {
Field string `env:"FIELD"`
}
```
The `noinit` tag is only applicable for pointer fields. Putting the tag on a
non-struct-pointer will return an error.
## Extension
Expand Down
18 changes: 12 additions & 6 deletions envconfig.go
Expand Up @@ -290,7 +290,8 @@ func ProcessWith(ctx context.Context, i interface{}, l Lookuper, fns ...MutatorF
// it means nothing was changed in any sub-fields.
// With the noinit opt, we skip setting the empty value
// to the original struct pointer (aka. keep it nil).
if !reflect.DeepEqual(v.Interface(), empty) || !opts.NoInit {
// if !reflect.DeepEqual(v.Interface(), empty) || !opts.NoInit {
if !reflect.DeepEqual(v.Interface(), empty) {
origin.Set(v)
}
}
Expand Down Expand Up @@ -394,7 +395,7 @@ func ProcessWith(ctx context.Context, i interface{}, l Lookuper, fns ...MutatorF
}

// Set value.
if err := processField(val, ef, opts.Delimiter, opts.Separator); err != nil {
if err := processField(val, ef, opts.Delimiter, opts.Separator, opts.NoInit); err != nil {
return fmt.Errorf("%s(%q): %w", tf.Name, val, err)
}
}
Expand Down Expand Up @@ -544,7 +545,12 @@ func processAsDecoder(v string, ef reflect.Value) (bool, error) {
return imp, err
}

func processField(v string, ef reflect.Value, delimiter, separator string) error {
func processField(v string, ef reflect.Value, delimiter, separator string, noInit bool) error {
// If the input value is empty and initialization is skipped, do nothing.
if v == "" && noInit {
return nil
}

// Handle pointers and uninitialized pointers.
for ef.Type().Kind() == reflect.Ptr {
if ef.IsNil() {
Expand Down Expand Up @@ -627,12 +633,12 @@ func processField(v string, ef reflect.Value, delimiter, separator string) error
mKey, mVal := strings.TrimSpace(pair[0]), strings.TrimSpace(pair[1])

k := reflect.New(tf.Key()).Elem()
if err := processField(mKey, k, delimiter, separator); err != nil {
if err := processField(mKey, k, delimiter, separator, noInit); err != nil {
return fmt.Errorf("%s: %w", mKey, err)
}

v := reflect.New(tf.Elem()).Elem()
if err := processField(mVal, v, delimiter, separator); err != nil {
if err := processField(mVal, v, delimiter, separator, noInit); err != nil {
return fmt.Errorf("%s: %w", mVal, err)
}

Expand All @@ -650,7 +656,7 @@ func processField(v string, ef reflect.Value, delimiter, separator string) error
s := reflect.MakeSlice(tf, len(vals), len(vals))
for i, val := range vals {
val = strings.TrimSpace(val)
if err := processField(val, s.Index(i), delimiter, separator); err != nil {
if err := processField(val, s.Index(i), delimiter, separator, noInit); err != nil {
return fmt.Errorf("%s: %w", val, err)
}
}
Expand Down
68 changes: 63 additions & 5 deletions envconfig_test.go
Expand Up @@ -1656,7 +1656,7 @@ func TestProcessWith(t *testing.T) {

// No init
{
name: "noinit/no_init_when_sub_fields_set",
name: "noinit/no_init_when_sub_fields_unset",
input: &struct {
Sub *struct {
Field string `env:"FIELD"`
Expand All @@ -1667,10 +1667,23 @@ func TestProcessWith(t *testing.T) {
Field string `env:"FIELD"`
} `env:",noinit"`
}{
// Sub struct ptr shouldn't be initiated because the 'Field' is not set
// in the lookuper.
Sub: nil,
},
// 'Sub' struct ptr shouldn't be initiated
// because the 'Field' is not set in the lookuper.
lookuper: MapLookuper(map[string]string{}),
},
{
name: "noinit/init_when_sub_sub_fields_unset",
input: &struct {
Lepton *Lepton `env:",noinit"`
}{},
exp: &struct {
Lepton *Lepton `env:",noinit"`
}{
// Sub-sub fields should not be initiaized when no value is given.
Lepton: nil,
},
lookuper: MapLookuper(map[string]string{}),
},
{
Expand All @@ -1685,18 +1698,63 @@ func TestProcessWith(t *testing.T) {
Field string `env:"FIELD"`
} `env:",noinit"`
}{
// Sub struct ptr should be initiated because the 'Field' is set in the
// lookuper.
Sub: &struct {
Field string `env:"FIELD"`
}{
Field: "banana",
},
},
// 'Sub' struct ptr should be initiated
// because the 'Field' is set in the lookuper.
lookuper: MapLookuper(map[string]string{
"FIELD": "banana",
}),
},
{
name: "noinit/init_when_sub_sub_fields_set",
input: &struct {
Lepton *Lepton `env:",noinit"`
}{},
exp: &struct {
Lepton *Lepton `env:",noinit"`
}{
// Sub-sub fields should be initiaized when a value is given.
Lepton: &Lepton{
Quark: &Quark{
Value: 5,
},
},
},
lookuper: MapLookuper(map[string]string{
"QUARK_VALUE": "5",
}),
},
{
name: "noinit/non_struct_ptr",
input: &struct {
Field1 *string `env:"FIELD1, noinit"`
Field2 *int `env:"FIELD2, noinit"`
Field3 *float64 `env:"FIELD3, noinit"`
Field4 *bool `env:"FIELD4"`
}{},
exp: &struct {
Field1 *string `env:"FIELD1, noinit"`
Field2 *int `env:"FIELD2, noinit"`
Field3 *float64 `env:"FIELD3, noinit"`
Field4 *bool `env:"FIELD4"`
}{
// The pointer fields that had a value should initialize, but the unset
// values should remain nil, iff they are set to noinit.
Field1: func() *string { x := "banana"; return &x }(),
Field2: func() *int { x := 5; return &x }(),
Field3: nil,
Field4: func() *bool { x := false; return &x }(),
},
lookuper: MapLookuper(map[string]string{
"FIELD1": "banana",
"FIELD2": "5",
}),
},
{
name: "noinit/error_not_ptr",
input: &struct {
Expand Down

0 comments on commit e94cab6

Please sign in to comment.