Bazel: struct accepts keys that are not valid identifier names.

[ttsugriy]

Description of the problem / feature request:

I have recently discovered that some of the struct usages in our codebase had keys containing dots (say foo.bar), which means that the only way to access it is via getattr function.

Overall, struct looks similar to Python's namedtuple and its behavior seems more appropriate:

>>> import collections
>>> collections.namedtuple('Foo', ['foo.bar'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/collections.py", line 351, in namedtuple
    'alphanumeric characters and underscores: %r' % name)
ValueError: Type names and field names can only contain alphanumeric characters and underscores: 'foo.bar'

or in Python 3

>>> import collections
>>> collections.namedtuple('Foo', ['foo.bar'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/collections/__init__.py", line 361, in namedtuple
    raise ValueError('Type names and field names must be valid '
ValueError: Type names and field names must be valid identifiers: 'foo.bar'

The only reason I can think of for allowing arbitrary keys is to allow users serialize their dictionaries into json, since struct.to_json is the only way to serialize data structures exposed to users. In such case would it make sense to just have a separate native function to_json that supports all supported data structures and stop accepting dots and possibly other types of keys like keywords?

Feature requests: what underlying problem are you trying to solve with this feature?

• allow only valid identifier names as struct keys
• expose to_json function as a standalone function that supports all supported data types

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

The following code

foo = struct(**{"foo.bar": "foo"})

does not produce any warnings/errors, but the only way to get the value of foo.bar attribute is via getattr(foo, 'foo.bar') and foo.foo.bar will fail.

What operating system are you running Bazel on?

Any

What's the output of bazel info release?

release 0.15.2-homebrew

[laurentlb]

cc @c-parsons, @alandonovan
Shall we roll out a breaking change for this?

[alandonovan]

This seems like a reasonable change. (I've been working with proto a lot recently and it's common to use the string data type, which must be a valid UTF-8 encoding. I worry that some day a user is going to sneak a binary string into a struct field name and effectively break the all proto features.)

Related: https://github.com/bazelbuild/bazel/issues/7801 (and Google-internal b/124504533).

I also agree that json should be decoupled from struct; see other issues about json.