If set, default values should always imply type

[akutz]

I will be submitting a pull request in accompaniment with this issue. The gist of it is this - I believe a key's default value, if set, should imply the value's type no matter from where else the value is set. This is particularly important when working with environment variables. For example:

package main

import (
    "fmt"
    "os"

    "github.com/spf13/viper"
)

func print(name string, val interface{}) {
    fmt.Printf("%-15[1]s%-15[2]T%[2]v\n", name, val)
}

func main() {
    viper.BindEnv("mykey", "MYPREFIX_MYKEY")
    viper.SetDefault("mykey", []string{})
    os.Setenv("MYPREFIX_MYKEY", "a b c")

    v1 := viper.GetStringSlice("mykey")
    v2 := viper.Get("mykey")

    print("v1", v1)
    print("v2", v2)
}

When this program is executed the following is emitted:

[0]akutz@pax:ex$ ./ex1 
v1             []string       [a b c]
v2             string         a b c
[0]akutz@pax:ex$

You may wonder, why is this important? Just use the GetStringSlice function. Well, it becomes important when dealing with marshaling. If we update the above program to this:

package main

import (
    "fmt"
    "os"

    "github.com/spf13/viper"
)

type Data struct {
    MyKey []string
}

func print(name string, val interface{}) {
    fmt.Printf("%-15[1]s%-15[2]T%[2]v\n", name, val)
}

func main() {
    viper.BindEnv("mykey", "MYPREFIX_MYKEY")
    viper.SetDefault("mykey", []string{})
    os.Setenv("MYPREFIX_MYKEY", "a b c")

    v1 := viper.GetStringSlice("mykey")
    v2 := viper.Get("mykey")

    print("v1", v1)
    print("v2", v2)

    d := &Data{}
    viper.Marshal(d)
    print("d.MyKey", d.MyKey)
}

Now we can see the issue when we execute the updated program:

[0]akutz@pax:ex$ ./ex2
v1             []string       [a b c]
v2             string         a b c
d.MyKey        []string       []
[0]akutz@pax:ex$

The marshalled data structure's field MyKey is empty when in fact it should have a string slice equal to, in value, []string {"a", "b", "c"}.

The problem is that viper's Marshal function calls AllSettings which ultimately uses the Get function. The Get function does try to infer the value's type, but it does so using the type of the value retrieved using this logic:

Get has the behavior of returning the value associated with the first place from where it is set. Viper will check in the following order:

• override
• flag
• env
• config file
• key/value store
• default

While the above order is the one we want when retrieving the values, I posit it is not the desired behavior when understanding the type of the value. I'm suggesting that the type of a key's default value be used in the Get function when switching the type of the value to return, regardless of the origin of the value.

I will be issuing a pull request to demonstrate the fix.

[akutz]

Pull Request #111 has been opened for this issue.

[s7v7nislands]

should close this issue?