psiphon-inc / configloader-go Goto Github PK

There might be no good reason to have type checking over and above what the codec unmarshaler provides. In theory it would be good if an unmarshaler didn't provide sufficient type checking (and just silently filled in zero values, say), but we haven't yet found an example where our type checking catches something that gets passed encoding/json or BurntSushi/toml. And it's quite a lot of additional complexity (and bug potential -- like catching conflicts that aren't really conflicts).

If removed, it would affect these things:

• Remove the type checking (obvs).
• Remove the "expected type" from the "conf" struct tag.
• Re-consider TextUnmarshaler detection and handling. Probably we should:
  • Add a method to the Codec interface like SupportsTextUnmarshaler() bool. Use that to prevent recursion into the struct (like, into the fields of time.Time or whatever). (Maybe get even fancier and pass in reflect info and let the method do interface satisfaction checking. This might also give us BinaryUnmarshaler support.)
  • Don't "expect" a string type if a TextUnmarshaler field is found. Just don't recurse.
• Probably FieldStruct.ExpectedType should be removed completely.

A structure like this:

struct {
  M map[string]int
}

With config like this:

[M]
a = 1
b = 2

...Will fail the consistency check. The map representation of the config used for checking reports M as map[string]interface{}, which fails the type comparison. Example: https://github.com/ascottAtPsiphon/configloader-example

There are tests for map[string]interface{} fields, not but more strongly typed maps.

For example, for a config like:

type Config struct {
  M map[string]int
}

An environment override like:

[]configloader.EnvOverride{
  {
    EnvVar: "MYVAR",
    Key: configloader.Key{"M", "a"},
  },
}

...Will result in an error about M.a not being found in the struct.

The code should detect that M is a map and relax the consistency check.