json_schema_compiler: any, additionalProperties, functions on types

tools/json_schema_compiler/model.py

Index: tools/json_schema_compiler/model.py
diff --git a/tools/json_schema_compiler/model.py b/tools/json_schema_compiler/model.py
index d839ab977be318601d9a79232b1903271fc3ad2e..3249d97814a2228c68f5dc455413f1dbde431ab0 100644
--- a/tools/json_schema_compiler/model.py
+++ b/tools/json_schema_compiler/model.py
@@ -40,13 +40,16 @@ class Namespace(object):
   """
   def __init__(self, json, source_file):
     self.name = json['namespace']
-    self.unix_name = _UnixName(self.name)
+    self.unix_name = UnixName(self.name)
     self.source_file = source_file
     self.source_file_dir, self.source_file_filename = os.path.split(source_file)
     self.types = {}
     self.functions = {}
+    # TODO(calamity): Implement this business.

[not at google - send to devlin, 2012/02/28 22:41:17]

What is "this business"?  Implementation of properties on namespaces?

Yeah, I can't think of when it'd be useful, at the moment, to support this. 

[calamity, 2012/03/01 04:47:09]

Leaving this TODO as discussed (for shared structures or constants across a
namespace).

+    for property_json in json.get('properties', []):
+      pass
     for type_json in json.get('types', []):
-      type_ = Type(type_json)
+      type_ = Type(type_json['id'], type_json)
       self.types[type_.name] = type_
     for function_json in json.get('functions', []):
       if not function_json.get('nocompile', False):
@@ -66,16 +69,31 @@ class Type(object):
     JSON (as described by the schema), such as top-level types and function
     parameters
   """
-  def __init__(self, json):
-    self.name = json['id']
+  def __init__(self, name, json):
+    assert ('properties' in json or 'additionalProperties' in json or
+        'functions' in json), name + " has no properties or functions"

[Yoyo Zhou, 2012/02/28 22:23:41]

nit: indent to paren

[not at google - send to devlin, 2012/02/28 22:41:17]

nit... I like arranging multiple linked conditionals like this by lining them up
like:

assert('properties' in json or
       'additionalProperties' in json or
       'functions' in json, name + ' has no properties or functions')

I don't think an assert is right here though.  Asserts should be for sanity
checking the correctness of your own code (that is, json_schema_compiler) but
this is validating the correctness of input to the program.  It should raise
some appropriate error instead.

Oh, and can types really have additionalProperties?  That seems odd.  You'd
expect types to be... well, entirely typed.

[calamity, 2012/03/01 04:47:09]

Types as in OBJECT properties. Should I have a flag to disallow
additionalProperties on a root type or just test this case?

Added a ParseException class that prints the hierarchy to the model object.

[not at google - send to devlin, 2012/03/01 07:09:37]

Eh, the more you support the better I suppose. Might as well test it too since
it's cheap.

[calamity, 2012/03/01 23:58:48]

Done.

+    self.name = name
     self.description = json.get('description')
     self.from_json = True
     self.from_client = True
     self.properties = {}
-    for prop_name, prop_json in json['properties'].items():
+    self.functions = {}
+    for function_json in json.get('functions', []):
+      if not function_json.get('nocompile', False):
+        function = Function(function_json)
+        self.functions[function.name] = function

[Yoyo Zhou, 2012/02/28 22:23:41]

This could fit on one line (same with 56-57).

[calamity, 2012/03/01 04:47:09]

like
self.functions[function_json['name']] = Function(function_json)?

[Yoyo Zhou, 2012/03/01 19:02:38]

Yes, exactly.

[calamity, 2012/03/01 23:58:48]

Done.

+    for prop_name, prop_json in json.get('properties', {}).items():
+      # Ignore callbacks in objects because they're handled renderer-side

[not at google - send to devlin, 2012/02/28 22:41:17]

Reference to "renderer-side" doesn't make much sense out of context in the
compiler.  You might want to say like

TODO(calamity): support functions (callbacks) on properties.  The model doesn't
support it yet because to h/cc generators don't -- this is because we'd need to
hook it into a base::Callback or something.

However, pragmatically it's not necessary to support them anyway, since the
instances of functions-on-properties in the extension APIs are all handled in
pure Javascript on the render process (and .: never reach C++ let alone the
browser).

[calamity, 2012/03/01 04:47:09]

Done.

+      if prop_json.get('type') == 'function':
+        continue
       self.properties[prop_name] = Property(prop_name, prop_json,
           from_json=True,
           from_client=True)
+    additional_properties = json.get('additionalProperties')
+    if additional_properties:
+      prop = Property('additionalProperties', additional_properties)
+      prop.type_ = PropertyType.ADDITIONAL_PROPERTIES

[Yoyo Zhou, 2012/02/28 22:23:41]

Seems like an abstraction violation to have to set this here and not in
Property.__init__.

[calamity, 2012/03/01 04:47:09]

Made a flag in Property.__init__.

+      self.properties['additionalProperties'] = prop

[not at google - send to devlin, 2012/02/28 22:41:17]

So... additionalProperties is stored as a property called
'additionalProperties'?  I guess this might make code generation easier, but it
could cause problems if there actually is a property called
'additionalProperties' I assume.

[calamity, 2012/03/01 04:47:09]

Raised an exception if any 2 properties have the same unix_name.

 
 class Callback(object):
   """A callback parameter to a Function.
@@ -109,7 +127,7 @@ class Function(object):
   def __init__(self, json):
     self.name = json['name']
     self.params = []
-    self.description = json['description']
+    self.description = json.get('description')
     self.callback = None
     for param in json['parameters']:
       if param.get('type') == 'function':
@@ -147,7 +165,7 @@ class Property(object):
       users of generated code, such as top-level types and function results
     """
     self.name = name
-    self._unix_name = _UnixName(self.name)
+    self._unix_name = UnixName(self.name)
     self._unix_name_used = False
     self.optional = json.get('optional', False)
     self.description = json.get('description')
@@ -182,10 +200,11 @@ class Property(object):
         self.properties = {}
         self.from_json = from_json
         self.from_client = from_client
-        for key, val in json.get('properties', {}).items():
-          self.properties[key] = Property(key, val,
-              from_json,
-              from_client)
+        type_ = Type(self.name, json)
+        self.properties = type_.properties
+        self.functions = type_.functions
+        self.from_client = from_client
+        self.from_json = from_json

[not at google - send to devlin, 2012/02/28 22:41:17]

you have repeated assignments here.

[calamity, 2012/03/01 04:47:09]

Done.

       else:
         raise NotImplementedError(json_type)
     elif 'choices' in json:
@@ -257,8 +276,9 @@ class PropertyType(object):
   CHOICES = _Info(False, "CHOICES")
   OBJECT = _Info(False, "OBJECT")
   ANY = _Info(False, "ANY")
+  ADDITIONAL_PROPERTIES = _Info(False, "ADDITIONAL_PROPERTIES")
 
-def _UnixName(name):
+def UnixName(name):
   """Returns the unix_style name for a given lowerCamelCase string.
   """
   return '_'.join([x.lower()