Panics can happen in a data-dependent way

[eli-darkly]

As noted in the error handling documentation, it is reasonable for a programmer error— code that is trying to use gcfg in a way that cannot possibly work, like passing something other than a struct pointer to ReadInto— to trigger a panic. And the documentation also specifies that the top-level config struct must have fields that are either structs or maps of strings to struct pointers, so declaring some other kind of field there is a programmer error.

However, the way the latter requirement is currently enforced makes the panic data-dependent: that is, you could easily write incorrect code that will be undetected in some cases, and panic in other cases, depending on what's in the config file.

For instance:

type Config struct {
	Bar Bar
	Baz int
}

type Foo struct {
	Field string
}

In this example, the Baz field is something I should not have put there— at least, it's not valid as far as gcfg is concerned. Perhaps I meant for it to be private and ignored by gcfg, but I accidentally exported it. Or maybe I didn't read the documentation. Either way, assuming the Baz was not really supposed to be part of the configuration schema, I might normally be running the program with a config file like this—

[Bar]
Field = x

—and it would work just fine. Then one day someone makes a typo in the config file—

[Baz]
Field = x

—and the program panics.

While this is still the programmer's fault, I think gcfg's current behavior— only checking that the type is valid if the section name is actually used— makes it too easy to miss errors like this, and end up with code that is vulnerable to panicking only for certain erroneous inputs.

If it is an absolute rule that all exported fields in the target struct must be either structs or map[string]*struct, then I think it would be reasonable and desirable for gcfg to check the fields ahead of time and immediately panic if they're wrong. I'd like to be able to assume that if I've written my code correctly in this regard, it won't panic for any input data, but if I've made this kind of mistake, it won't not panic just because I happened to test with valid data.

[speter]

Thank you for the feedback,. I agree that eager checking of the config struct would be a desirable feature, but I think the actual possibility of an error is rather hypothetical, making it low priority. Another thing to consider is that some people may be making use of the fact that struct checking is not eager, and include ancillary fields in their config structs, which currently works without issue (even though this is not the originally intended usage of gcfg). Based on this, rather than changing the behavior of the existing gcfg.Read* APIs to panic more eagerly, it seems more pragmatic to add a new optional and explicit API, such as gcfg.MustStruct, that checks a config struct and panics if invalid.

[eli-darkly]

Regarding adding new APIs to support new behavior, have you considered using a variadic options pattern as described here so that you wouldn't need to add new variants of ReadInto, ReadStringInto, and ReadFileInto every time there's a new option? Either that, or a single new API variant of each that takes an options struct.