Add enum class support to json_schema_compiler (idl files)

tools/json_schema_compiler/cpp_type_generator.py

Index: tools/json_schema_compiler/cpp_type_generator.py
diff --git a/tools/json_schema_compiler/cpp_type_generator.py b/tools/json_schema_compiler/cpp_type_generator.py
index 269a99572c1453a789597dbbcf6b0b5dae01b79b..d27a97ec11a43771fca8a81a6289a941b5c6e3da 100644
--- a/tools/json_schema_compiler/cpp_type_generator.py
+++ b/tools/json_schema_compiler/cpp_type_generator.py
@@ -35,27 +35,43 @@ class CppTypeGenerator(object):
       self._default_namespace = model.namespaces.values()[0]
     self._schema_loader = schema_loader
 
-  def GetEnumNoneValue(self, type_):
+  def _AddEnumScope(self, type_, value, is_in_declaration, use_override=True):
+    """Decorates the given value with a FOO_ or Foo:: prefix depending whether
+    type_ is an enum class. If in a declaration, Foo:: prefixes are never added,
+    but FOO_ prefixes are.
+    """
+    prefix_override = use_override and type_.cpp_enum_prefix_override
+    name_prefix = ''
+    if not type_.is_class or prefix_override:
+      name_prefix = (prefix_override or self.FollowRef(type_).unix_name) + '_'
+
+    name_prefix = name_prefix.upper();
+    if not type_.is_class or is_in_declaration:
+      return '%s%s' % (name_prefix, value)
+
+    return '%s::%s%s' % (self.FollowRef(type_).simple_name, name_prefix, value)
+
+  def GetEnumNoneValue(self, type_, is_in_declaration=False):
     """Gets the enum value in the given model.Property indicating no value has
     been set.
     """
-    return '%s_NONE' % self.FollowRef(type_).unix_name.upper()
+    return self._AddEnumScope(type_, 'NONE', is_in_declaration, False)

[Devlin, 2016/05/17 19:04:10]

The fact that optional enums are NONEs instead of just an empty unique_ptr is
pretty annoying, and there's even a TODO lying around to fix it. [1]  Since
everywhere we change to an enum class will have to be updated, maybe this is a
good time to change that?  WDYT?

[tapted, 2016/05/18 09:35:54]

[1] -> null pointer? :)

But I think it would be tricky to map a unique_ptr into the use cases for things
like AXEvent and other things from ax_enums.idl. The same enums are used
extensively in browser-only UI, without touching json at all (no base::*Value
etc.). And, e.g., ui::AX_EVENT_NONE is used often as a sentinel value. So I
think we would still need `NONE` at least for the stuff in ax_enums.idl, but we
could perhaps phase it out of some other generated types that only ever operate
using base::Value and `NONE` is an error state rather than a ~"don't care"
state.

[Devlin, 2016/05/19 00:33:12]

Whoops, [1] ->
https://code.google.com/p/chromium/codesearch#chromium/src/tools/json_schema_...

Is there any case outside of ax events that actually uses NONE for something
other than optional values?  It's really inconsistent, so I'd like to fix it at
some point, and, given that this is a massive enum overhauling, it seemed
appropriate.  But... ah well.

[tapted, 2016/05/19 12:22:32]

`git grep ui::AX.*NONE` shows up a lot of other ax examples that would be tricky
to migrate from `none` (e.g. AX_ROLE, and others apart from AX_EVENT). And while
`git grep -C10 'api::.*_NONE'` under extensions/browser/api shows up mostly
error-state handling (DCHECK, NOTREACHED, and so forth), there are a lot of
situations where an enum is returned, and there's some logic that requires it to
be initialized to "some" value beforehand.

The change to make these methods return, e.g., a scoped_ptr<base::Value> instead
of an enum wouldn't be mechanical. Whereas changing from an enum to an enum
class is almost purely mechanical -- a simple sed script and `git cl format` do
most of the job. All that was left in the AXEvent change in
http://crrev.com/1988613002 was adding a static_cast<int> to an stream output.
So, while it is an overhaul, it's a straightforward one.

 
-  def GetEnumLastValue(self, type_):
+  def GetEnumLastValue(self, type_, is_in_declaration=False):
     """Gets the enum value in the given model.Property indicating the last value
     for the type.
     """
-    return '%s_LAST' % self.FollowRef(type_).unix_name.upper()
+    return self._AddEnumScope(type_, 'LAST', is_in_declaration, False)
 
-  def GetEnumValue(self, type_, enum_value):
+  def GetEnumValue(self, type_, enum_value, is_in_declaration=False):
     """Gets the enum value of the given model.Property of the given type.
 
     e.g VAR_STRING
     """
-    value = cpp_util.Classname(enum_value.name.upper())
-    prefix = (type_.cpp_enum_prefix_override or
-              self.FollowRef(type_).unix_name)
-    value = '%s_%s' % (prefix.upper(), value)
+    value = self._AddEnumScope(type_,
+                               cpp_util.Classname(enum_value.name.upper()),
+                               is_in_declaration)
+
     # To avoid collisions with built-in OS_* preprocessor definitions, we add a
     # trailing slash to enum names that start with OS_.
     if value.startswith("OS_"):