JSON Schema Compiler now supports conversion from Choice to base::Value.

tools/json_schema_compiler/cc_generator.py

Index: tools/json_schema_compiler/cc_generator.py
diff --git a/tools/json_schema_compiler/cc_generator.py b/tools/json_schema_compiler/cc_generator.py
index d21e160e6266a8362bb31ef99fab2794fef4e31a..520c41422c094615e2b20c042a9e25826f62e67a 100644
--- a/tools/json_schema_compiler/cc_generator.py
+++ b/tools/json_schema_compiler/cc_generator.py
@@ -255,6 +255,10 @@ class CCGenerator(object):
             c.Sblock('if (%s != %s)' %
                 (prop.unix_name,
                     self._cpp_type_generator.GetEnumNoneValue(prop)))

[not at google - send to devlin, 2012/07/19 01:48:22]

nit: these should vertically line up, so unindent this 4 characters

[chebert, 2012/07/19 21:06:17]

Done.

+          elif prop.type_ == PropertyType.CHOICES:
+            c.Sblock('if (%s_type != %s)' %
+                (prop.unix_name,
+                    self._cpp_type_generator.GetEnumNoneValue(prop)))

[not at google - send to devlin, 2012/07/19 01:48:22]

ditto

oh it was already like this. incremental cleanup!

[chebert, 2012/07/19 21:06:17]

Done.

           else:
             c.Sblock('if (%s.get())' % prop.unix_name)
         c.Append('value->SetWithoutPathExpansion("%s", %s);' % (
@@ -293,41 +297,6 @@ class CCGenerator(object):
 
     return c
 
-  def _GenerateCreateEnumValue(self, cpp_namespace, prop):
-    """Generates CreateEnumValue() that returns the |base::StringValue|
-    representation of an enum.
-    """
-    c = Code()
-    c.Append('// static')
-    c.Sblock('scoped_ptr<base::Value> %(cpp_namespace)s::CreateEnumValue('
-             '%(arg)s) {')
-    c.Sblock('switch (%s) {' % prop.unix_name)
-    if prop.optional:
-      (c.Append('case %s: {' % self._cpp_type_generator.GetEnumNoneValue(prop))
-        .Append('  return scoped_ptr<base::Value>();')
-        .Append('}')
-      )
-    for enum_value in prop.enum_values:
-      (c.Append('case %s: {' %
-          self._cpp_type_generator.GetEnumValue(prop, enum_value))
-        .Append('  return scoped_ptr<base::Value>('
-                'base::Value::CreateStringValue("%s"));' %
-                enum_value)
-        .Append('}')
-      )
-    (c.Append('default: {')
-      .Append('  return scoped_ptr<base::Value>();')
-      .Append('}')
-    )
-    c.Eblock('}')
-    c.Eblock('}')
-    c.Substitute({
-        'cpp_namespace': cpp_namespace,
-        'arg': cpp_util.GetParameterDeclaration(
-            prop, self._cpp_type_generator.GetType(prop))
-    })
-    return c
-
   def _CreateValueFromProperty(self, prop, var):
     """Creates a base::Value given a property. Generated code passes ownership
     to caller.
@@ -337,13 +306,8 @@ class CCGenerator(object):
     E.g for std::string, generate base::Value::CreateStringValue(var)
     """
     if prop.type_ == PropertyType.CHOICES:
-      # CHOICES conversion not implemented. If needed, write something to
-      # generate a function that returns a scoped_ptr<base::Value> and put it in
-      # _GeneratePropertyFunctions, then use it here. Look at CreateEnumValue()
-      # for reference.
-      raise NotImplementedError(
-          'Conversion of CHOICES to base::Value not implemented')
-    if self._IsObjectOrObjectRef(prop):
+      return 'GetChoiceValue(%s_type).release()' % var
+    elif self._IsObjectOrObjectRef(prop):
       if prop.optional:
         return '%s->ToValue().release()' % var
       else:
@@ -615,6 +579,7 @@ class CCGenerator(object):
   def _GeneratePropertyFunctions(self, param_namespace, params):
     """Generate the functions for structures generated by a property such as
     CreateEnumValue for ENUMs and Populate/ToValue for Params/Results objects.
+    If from_function is True if these properties belong to an API Function.

[not at google - send to devlin, 2012/07/19 01:48:22]

revert this comment change

[chebert, 2012/07/19 21:06:17]

Done.

     """
     c = Code()
     for param in params:
@@ -629,11 +594,76 @@ class CCGenerator(object):
       elif param.type_ == PropertyType.CHOICES:
         c.Concat(self._GeneratePropertyFunctions(
             param_namespace, param.choices.values()))
+        if param.from_client and param.from_json:

[not at google - send to devlin, 2012/07/19 01:48:22]

why does it need to be both from the JSON? i would have though the correct
condition was just "param.from_client"?

[chebert, 2012/07/19 21:06:17]

Done.

+          c.Concat(self._GenerateGetChoiceValue(param_namespace, param))
       elif param.type_ == PropertyType.ENUM:
         c.Concat(self._GenerateCreateEnumValue(param_namespace, param))
         c.Append()
     return c
 
+  def _GenerateGetChoiceValue(self, cpp_namespace, prop):

[not at google - send to devlin, 2012/07/19 01:48:22]

this code is so similar to _GenerateCreateEnumValue. However, it may be a bit of
a contrivance to try and share code between the two. Oh well.

[chebert, 2012/07/19 21:06:17]

I took a shot at this. It is a bit contrived though...

On 2012/07/19 01:48:22, kalman wrote:

+    """Generates GetChoiceValue() that returns a scoped_ptr<base::Value>
+    representing the choice value.
+    """
+    c = Code()
+    (c.Sblock('scoped_ptr<base::Value> '

[not at google - send to devlin, 2012/07/19 01:48:22]

static? (const would also be unnecessary)

[chebert, 2012/07/19 21:06:17]

non-static, because we return member data.
const, because the "this" reference is const (from the ToValue() method).
On 2012/07/19 01:48:22, kalman wrote:

+              '%(cpp_namespace)s::GetChoiceValue(%(arg)s) const {')
+      .Sblock('switch(%s) {' % prop.unix_name)
+    )
+    for choice in self._cpp_type_generator.ExpandParams([prop]):
+      (c.Append('case %s:' %
+            self._cpp_type_generator.GetEnumValue(prop, choice.type_.name))
+        .Append('  return make_scoped_ptr<base::Value>(%s);' %
+            self._CreateValueFromProperty(choice, choice.unix_name))
+      )
+    (c.Append('default:')

[not at google - send to devlin, 2012/07/19 01:48:22]

we are generating a branch for every enum value, so it shouldn't be possible to
hit this unless there's a bug somewhere (which involves casting a non-enum value
to one of these enums; and *that* shouldn't be possible because the only thing
calling this code is our own code).

So rather than

switch (...) {
  case FOO:
    return ...;
  case BAR:
    return ...;
  default:
    return scoped_ptr<base::Value>();
}

let's generate

switch (...) {
  case FOO:
    return ...;
  case BAR:
    return ...;
}

NOTREACHED();
return scoped_ptr<base::Value>();

(make sure this file #includes base/logging.h)

that would also require generating an explicit case for "none" if it's
optional...

[chebert, 2012/07/19 21:06:17]

Done.

+      .Append('  return scoped_ptr<base::Value>();')
+      .Eblock('}')
+      .Eblock('}')
+      .Append()
+      .Substitute({
+        'cpp_namespace': cpp_namespace,
+        'arg': cpp_util.GetParameterDeclaration(
+            prop, self._cpp_type_generator.GetChoicesEnumType(prop))
+      })
+    )
+    return c
+
+  def _GenerateCreateEnumValue(self, cpp_namespace, prop):
+    """Generates CreateEnumValue() that returns the |base::StringValue|
+    representation of an enum.
+    """
+    c = Code()
+    c.Append('// static')
+    c.Sblock('scoped_ptr<base::Value> %(cpp_namespace)s::CreateEnumValue('
+             '%(arg)s) {')
+    c.Sblock('switch (%s) {' % prop.unix_name)
+    if prop.optional:
+      (c.Append('case %s: {' % self._cpp_type_generator.GetEnumNoneValue(prop))
+        .Append('  return scoped_ptr<base::Value>();')
+        .Append('}')
+      )
+    for enum_value in prop.enum_values:
+      (c.Append('case %s: {' %
+          self._cpp_type_generator.GetEnumValue(prop, enum_value))
+        .Append('  return scoped_ptr<base::Value>('
+                'base::Value::CreateStringValue("%s"));' %
+                enum_value)
+        .Append('}')
+      )
+    (c.Append('default: {')
+      .Append('  return scoped_ptr<base::Value>();')
+      .Append('}')

[not at google - send to devlin, 2012/07/19 01:48:22]

ditto default thing.

[chebert, 2012/07/19 21:06:17]

Done.

+    )
+    c.Eblock('}')
+    c.Eblock('}')
+    c.Substitute({
+        'cpp_namespace': cpp_namespace,
+        'arg': cpp_util.GetParameterDeclaration(
+            prop, self._cpp_type_generator.GetType(prop))
+    })
+    return c
+
   def _GenerateCreateCallbackArguments(self, function_scope, callback):
     """Generate all functions to create Value parameters for a callback.