map with no values does not return error

[adamplansky]

If there are no map values, package does not return any error.

https://play.golang.org/p/hbIQe_nTST0

[jan-dubsky]

I'm not sure whether this can be considered a bug. Thinking of string, it's valid and desired behavior of the package to parse environment variable set to empty string as empty string. Using the same logic here, we should allow empty map parsing, as empty map is also a valid map.
Maybe it would make sense to point out that there are also more known issues with map parsing. Your problem is just an instance of much bigger problem - parsing map from env cannot be safe, as we don't know how many keys are desired in a map. Let's imagine you have two env vars EXAMPLE_foo and EXAMLPE_bar. If foo and bar were string variables, env package would correctly return an error that bar is not specified because of a typo in variable name. But if EXAMPLE_ were a map, env package wouldn't know that there are two keys to parse. It would simply parse foo and ignore bar. So your issue is just a single instance of more generic problem.
I'd say that with map parsing, we are already hitting the line where env parsing still makes sense compared to config file loading. Consequently, I cannot see any simple way how to resolve the aforementioned issue. Do you have any proposals?

[adamplansky]

I agree that dealing with map is tricky but I think this is indeed bug.

If env is not provided at all I would expect it fails, at least this is described in package motivation that all envs are considered as required.

I was not looking deep in the code so I dont have any proposal yet.

[jan-dubsky]

Well yeah, but how would you then parse an empty map? Also how does this empty map problem differs from the typo issue described above?

[Hrubon]

would expect it fails, at least this is described in package motivation that all envs are considered as required.

Yes, you're kinda right. But As @jan-dubsky said, I don't see any elegant way how to parse it, since the keys are baked in the env var name. A solution might be to specify the map as "required" by some extra annotation, and in such case, it would override the default behavior and would check if the map contains at least one key. But I am not sure if this would improve the package in general (we want to keep it simple). This would be a little bit hairy.

To add an example: Imagine you have a map representing a black-list. If you don't want to blacklist anything, empty map is completely valid input. So you definitely don't want to enforce it as default behavior. Then you have the only option to make the whole thing configurable as described above. Once you do that, there is no way back. So we should really think about it, if it's really the way to go.

Do you have any concrete use-case in mind which has put you in trouble?

[jan-dubsky]

Moreover even the required tag wouldn't save from typos as described above (1 var parsed instead of 2). The only solution to that would be to introduce some special key indicating map element count. So you would have EXAMPLE_foo=something, EXAMPLE_bar=aloha and for example EXAMPLE_##COUNT##=2. Only then env package could check that the count of elements matches and also the ##COUNT## env var could be required in every case. But again, this is extremely hairy and contradicts the package simplicity.

On the other hand you are right that the README presents whole env package as safe and the map behavior doesn't actually fulfill this promise. I will try to write a section of README explaining mostly what I wrote here and warning the user that maps are slightly more error prone than the rest of the package.

[adamplansky]

For black-list example I would add go tag, something like: env:"XXX_",optional for allowing empty blacklist or I would use similar syntax as for slice (not sure if it has some other problems):

https://play.golang.org/p/eFZbvlA0qrF

I get it now why it behaves differently but still It was big surprise for me.