Skip to content

Commit

Permalink
return an error when using default= on slices
Browse files Browse the repository at this point in the history
  • Loading branch information
vrischmann committed Aug 29, 2016
1 parent 079767d commit 9dd087a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 13 deletions.
17 changes: 11 additions & 6 deletions envconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ var (
ErrNotAPointer = errors.New("envconfig: value is not a pointer")
// ErrInvalidValueKind is the error returned by the Init* functions when the configuration object is not a struct.
ErrInvalidValueKind = errors.New("envconfig: invalid value kind, only works on structs")
// ErrDefaultUnsupportedOnSlice is the error returned by the Init* functions when there is a default tag on a slice.
// The `default` tag is unsupported on slices because slice parsing uses , as the separator, as does the envconfig tags separator.
ErrDefaultUnsupportedOnSlice = errors.New("envconfig: default tag unsupported on slice")
)

type context struct {
Expand Down Expand Up @@ -211,9 +214,7 @@ func readStruct(value reflect.Value, ctx *context) (nonnil bool, err error) {
var byteSliceType = reflect.TypeOf([]byte(nil))

func setField(value reflect.Value, ctx *context) (ok bool, err error) {
isSliceNotUnmarshaler := value.Kind() == reflect.Slice && !isUnmarshaler(value.Type())

str, err := readValue(ctx, isSliceNotUnmarshaler)
str, err := readValue(ctx)
if err != nil {
return false, err
}
Expand All @@ -222,6 +223,7 @@ func setField(value reflect.Value, ctx *context) (ok bool, err error) {
return false, nil
}

isSliceNotUnmarshaler := value.Kind() == reflect.Slice && !isUnmarshaler(value.Type())
switch {
case isSliceNotUnmarshaler && value.Type() == byteSliceType:
return true, parseBytesValue(value, str)
Expand All @@ -235,6 +237,10 @@ func setField(value reflect.Value, ctx *context) (ok bool, err error) {
}

func setSliceField(value reflect.Value, str string, ctx *context) error {
if ctx.defaultVal != "" {
return ErrDefaultUnsupportedOnSlice
}

elType := value.Type().Elem()
tnz := newSliceTokenizer(str)

Expand Down Expand Up @@ -406,7 +412,7 @@ func combineName(parentName, name string) string {
return parentName + "." + name
}

func readValue(ctx *context, isSlice bool) (string, error) {
func readValue(ctx *context) (string, error) {
keys := makeAllPossibleKeys(ctx)

var str string
Expand All @@ -422,8 +428,7 @@ func readValue(ctx *context, isSlice bool) (string, error) {
return str, nil
}

// only return the default value for non slices since it doesn't work as people expect.
if !isSlice && ctx.defaultVal != "" {
if ctx.defaultVal != "" {
return ctx.defaultVal, nil
}

Expand Down
10 changes: 3 additions & 7 deletions envconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,19 +493,15 @@ func TestDefaultSlice(t *testing.T) {
// The way people think about the following default value, is that the slice will be [a,b]
// However this never worked because we split the entire envconfig tag on , therefore default is just `a` here.
// The proper thing to do is to introduce a new format in the tag that doesn't have this limitation, but we don't have that yet.
// For now, we ignore the default tag and simply treat the rest, here `b` is the name of the environment variable expected.
//
// At least now it won't silently fail by putting only half of the slice in the default.
// For now, we simply return an error indicating default is not unsupported on slices.

var conf struct {
Hosts []string `envconfig:"default=a,b"`
}

os.Setenv("b", "c,d")

err := envconfig.Init(&conf)
require.Nil(t, err)
require.Equal(t, []string{"c", "d"}, conf.Hosts)
require.NotNil(t, err)
require.Equal(t, envconfig.ErrDefaultUnsupportedOnSlice, err)
}

func TestInitNotAPointer(t *testing.T) {
Expand Down

0 comments on commit 9dd087a

Please sign in to comment.