Initial commit of the Dart Chrome Extension APIs generators

tools/json_schema_compiler/dart_generator.py

+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+"""
+Generator language component for compiler.py that adds Dart language support.
+
+Pass 'dart' with the -l flag to compiler.py to activate the use of this library.
+"""
+
+from collections import defaultdict

[not at google - send to devlin, 2013/01/29 16:37:08]

unused

[sashab, 2013/01/30 05:26:03]

Done.

+
+from code import Code
+from model import *
+
+import os
+from datetime import datetime
+
+LICENSE = ("""
+// Copyright (c) %s, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.""" %
+datetime.now().year)

[not at google - send to devlin, 2013/01/29 16:37:08]

indent this line?

[sashab, 2013/01/30 05:26:03]

Done.

+
+class DartGenerator(object):
+  """A .dart generator for a namespace.
+  """
+
+  def __init__(self, namespace, custom_dart_folder):

[not at google - send to devlin, 2013/01/29 16:37:08]

IMO like you said in the doc, "overrides" would be a nice name.

also, please use "dir" not "folder".

[sashab, 2013/01/30 05:26:03]

Done.

+    self._namespace = namespace
+    self._types = namespace.types
+
+    # Build a dictionary of Type Name --> Custom Dart code.
+    self._custom_dart = {}
+    if custom_dart_folder:

[not at google - send to devlin, 2013/01/29 16:37:08]

If custom_dart_folder is optional then make the argument custom_dart_folder=None
to indicate it. Also, use "if custom_dart_folder is not None" since "if
custom_dart_folder" will also fail on empty-string, and that's not something
that we need to handle.

[sashab, 2013/01/30 05:26:03]

Done.

+      for filename in os.listdir(custom_dart_folder):
+        if filename.startswith(namespace.unix_name):
+          with open(os.path.join(custom_dart_folder, filename)) as f:
+            # Split off the namespace and file extension, leaving just the type.
+            type_path = '.'.join(filename.split('.')[1:-1])
+            self._custom_dart[type_path] = f.read()
+    print self._custom_dart
+    global x
+    x= self._namespace

[not at google - send to devlin, 2013/01/29 16:37:08]

delete 3 lines above here?

[sashab, 2013/01/30 05:26:03]

Done.

+
+  def Generate(self):
+    """Generates a Code object with the .dart for the entire namespace.
+    """
+    c = Code()
+    (c.Append(LICENSE)
+      .Append()
+      .Append('part of chrome;'))
+
+    # Add all types.

[not at google - send to devlin, 2013/01/29 16:37:08]

comment not useful

[sashab, 2013/01/30 05:26:03]

Done.

+    if self._types:
+      (c.Append()
+        .Append('/**')
+        .Append(' * Types')
+        .Append(' */')
+      )
+    for type_name in self._types:
+      c.Concat(self._GenerateType(self._types[type_name]))
+
+    # Add all events.

[not at google - send to devlin, 2013/01/29 16:37:08]

ditto

[sashab, 2013/01/30 05:26:03]

Done.

+    if self._namespace.events:
+      (c.Append()
+        .Append('/**')
+        .Append(' * Events')
+        .Append(' */')
+      )
+    for event_name in self._namespace.events:
+      c.Concat(self._GenerateEvent(self._namespace.events[event_name]))
+
+    # Add main class for this file.

[not at google - send to devlin, 2013/01/29 16:37:08]

ditto

[sashab, 2013/01/30 05:26:03]

Done.

+    (c.Append()
+      .Append('/**')
+      .Append(' * Functions')
+      .Append(' */')
+    )
+    c.Concat(self._GenerateMainClass())
+
+    return c
+
+  def _GenerateType(self, type_):
+    """Given a Type object, returns the Code with the .dart for this
+    type's definition.
+
+    Assumes this type is a Parameter Type (creatable by user), and creates an
+    object that extends ChromeObject. All parameters are specifiable as named
+    arguments in the constructor, and all methods are wrapped with getters and
+    setters that hide the JS() implementation.
+    """
+    c = Code()
+    (c.Append()
+      .Concat(self._GenerateDocumentation(type_))
+      .Sblock('class %(type_name)s extends ChromeObject {')
+    )
+
+    # Check whether this type has function members. If it does, don't allow
+    # public construction.
+    add_public_constructor = True

[not at google - send to devlin, 2013/01/29 16:37:08]

add_public_constructor = any(self._IsFunction(p)
                             for p in type_.properties.values())

[sashab, 2013/01/30 05:26:03]

Done.

+    for prop_name in type_.properties:
+      if self._IsFunction(type_.properties[prop_name]):
+        add_public_constructor = False
+        break
+
+    constructor_fields = []

[not at google - send to devlin, 2013/01/29 16:37:08]

constructor_fields = [self._GeneratePropertySignature(p)
                      for p in type_.properties.values()]

[sashab, 2013/01/30 05:26:03]

Done.

+    for prop_name in type_.properties:
+      constructor_fields.append(
+          self._GeneratePropertySignature(type_.properties[prop_name],
+                                        prepend_this = False,
+                                        omitBasicTypes = False))

[not at google - send to devlin, 2013/01/29 16:37:08]

prepend_this = False -> prepend_this=False
omitBasicTypes = False -> omit_basic_types=False

however, since they're False by default, you can leave it out.

[sashab, 2013/01/30 05:26:03]

Done.

+
+    # Add the public constructor.

[not at google - send to devlin, 2013/01/29 16:37:08]

I think you can leave this comment out, the variable is called
add_public_constructor and there's a dart comment anyway.

[sashab, 2013/01/30 05:26:03]

Done.

+    if add_public_constructor:
+      (c.Append('/*')
+        .Append(' * Public constructor')
+        .Append(' */')
+        .Sblock('%(type_name)s({%(constructor_fields)s}) {')
+      )
+
+      for prop_name in type_.properties:
+        c.Append('this.%s = %s;' % (prop_name, prop_name))
+      (c.Eblock('}')
+        .Append()
+      )
+
+    # Add the private constructor.

[not at google - send to devlin, 2013/01/29 16:37:08]

ditto

[sashab, 2013/01/30 05:26:03]

Done.

+    (c.Append('/*')
+      .Append(' * Private constructor')
+      .Append(' */')
+      .Append('%(type_name)s._proxy(_jsObject) : super._proxy(_jsObject);')
+    )
+
+    # Add an accessor (getter & setter) for each property.
+    properties = [t for t in type_.properties.values()
+                  if not self._IsFunction(t)]

[not at google - send to devlin, 2013/01/29 16:37:08]

nit: p would be a slighty more appropriate temporary variable than t

[sashab, 2013/01/30 05:26:03]

Done.

+    if properties:
+      (c.Append()
+        .Append('/*')
+        .Append(' * Public accessors')
+        .Append(' */')
+      )
+    for prop in properties:
+      type_name = self._GetPropertyType(prop)
+      prop_is_base_type = self._IsBaseType(prop)
+
+      # Check for custom dart for this whole property.
+      overwrite_key = '%s.%s' % (type_.name, prop.name)

[not at google - send to devlin, 2013/01/29 16:37:08]

overwrite -> override?

[sashab, 2013/01/30 05:26:03]

Done.

+      if overwrite_key in self._custom_dart:

[not at google - send to devlin, 2013/01/29 16:37:08]

A common idiom is like:

override = self._custom_dart.get('%s.%s' % (type_.name, prop.name))
if override is not None:
  ...
   

[sashab, 2013/01/30 05:26:03]

Done.

+        contents = self._custom_dart[overwrite_key]
+        if contents.strip():

[not at google - send to devlin, 2013/01/29 16:37:08]

Why is this guard necessary? If it is for some reason possible  for empty
strings to end up in custom_dart, that check should be done while populating the
map.

[sashab, 2013/01/30 05:26:03]

If you want to "hide" a function completely, you can make an empty file with the
name of the function. In this case, we don't want to print the function's
documentation.

I'm also not sure the best place to put this; moving it to when the tree is
generated wouldn't really help, as you could be overwriting the getter for the
class, but not the setter, in which case you still want the documentation (and
you can't just remove it from the tree, since you need the information for the
setter).

The code logic for this actually isn't quite right - it should really be:

if override is not blank:
  write docs
  do override
else:
  if override_get is not blank:
    write docs
    do override for get
  if override_set is not blank:
    if override_get is blank:
      write docs
    do override for set

Can you think of a better way to do this?

[not at google - send to devlin, 2013/01/31 02:08:48]

So is this an alternative to specifying nodart?

[sashab, 2013/01/31 04:41:40]

Exactly. The good thing about this is its all in the one repo. That is, if we
want to add "nodart" to a function, we just have to worry about maintaining that
in Dart's repo, not in Chromium's.

+          (c.Append()
+            .Concat(self._GenerateDocumentation(prop))
+          )
+        for line in contents.split('\n'):
+          c.Append(line)
+      else:
+        # Add the getter.
+        getter_key = '%s.%s.get' % (type_.name, prop.name)
+        if getter_key in self._custom_dart:
+          contents = self._custom_dart[getter_key]
+          if contents.strip():
+            # Add the documentation for this property.
+            (c.Append()
+              .Concat(self._GenerateDocumentation(prop))
+            )
+          for line in contents.split('\n'):
+            c.Append(line)
+        else:
+          # Add the documentation for this property.
+          (c.Append()
+            .Concat(self._GenerateDocumentation(prop))
+          )
+
+          # TODO(sashab): Serialize generic Dart objects differently.
+          if prop_is_base_type or self._IsObjectType(prop):
+            c.Append("%s get %s => JS('%s', '#.%s', this._jsObject);" %
+                (type_name, prop.name, type_name, prop.name))
+          elif prop.type_.property_type == PropertyType.REF:
+            c.Append("%s get %s => new %s._proxy(JS('', '#.%s', "
+                     "this._jsObject));"
+              % (type_name, prop.name, type_name, prop.name))
+          else:
+            # TODO(sashab): What to do in this situation? Unserializable type.

[not at google - send to devlin, 2013/01/29 16:37:08]

throw an exception?

[sashab, 2013/01/30 05:26:03]

Done.

+            c.Append("%s get %s => JS('%s', '#.%s', this._jsObject);" %
+                (type_name, prop.name, type_name, prop.name))
+
+        # Add the setter.
+        setter_key = '%s.%s.set' % (type_.name, prop.name)
+        if setter_key in self._custom_dart:
+          contents = self._custom_dart[setter_key]
+          if contents.strip():
+            c.Append()
+          for line in contents.split('\n'):
+            c.Append(line)

[not at google - send to devlin, 2013/01/29 16:37:08]

This pattern (reading from custom dart and then appending it) is repeated 3
times, make it a function.

[sashab, 2013/01/30 05:26:03]

Done.

+        else:
+          wrapped_name = prop.name
+          if not prop_is_base_type:
+            wrapped_name = 'convertArgument(%s)' % prop.name
+
+          (c.Append()
+            .Sblock("void set %s(%s %s) {" % (prop.name, type_name, prop.name))
+            .Append("JS('void', '#.%s = #', this._jsObject, %s);" %
+              (prop.name, wrapped_name))
+            .Eblock("}")
+          )
+
+    # Now add all the function properties.

[not at google - send to devlin, 2013/01/29 16:37:08]

s/function properties/methods/ ?

[sashab, 2013/01/30 05:26:03]

Done.

+    function_properties = [t for t in type_.properties.values()
+                           if self._IsFunction(t)]
+    if function_properties:
+      (c.Append()
+        .Append('/*')
+        .Append(' * Methods')
+        .Append(' */')
+      )
+    for prop in function_properties:
+      c.Concat(self._GenerateFunction(prop.type_.function))
+
+    (c.Eblock('}')
+      .Substitute({
+        'type_name': type_.simple_name,
+        'constructor_fields': ', '.join(constructor_fields)
+      })
+    )
+
+    return c
+
+  def _GenerateDocumentation(self, prop):
+    """Given an object, generates the documentation for this object (as a
+    code string) and returns the Code object.
+
+    Returns an empty code object if the object has no documentation.
+
+    Uses triple-quotes for the string.
+    """
+    c = Code()
+    if not hasattr(prop, 'description'):

[not at google - send to devlin, 2013/01/29 16:37:08]

"hasattr" should no longer be necessary (and we should avoid reflection if
possible). props should always have at least a None description, so check "is
not None"

[sashab, 2013/01/30 05:26:03]

Done.

+      return c
+
+    if prop.description:

[not at google - send to devlin, 2013/01/29 16:37:08]

oh, there's a check here anyway. I prefer the check above

[sashab, 2013/01/30 05:26:03]

Hopefully I've made it a little clearer now.

+      for line in prop.description.split('\n'):
+        c.Comment(line, comment_prefix='/// ')
+    return c
+
+
+  def _GenerateFunction(self, f):
+    """Returns the Code object for the given function.
+    """
+    return (Code()
+        .Append()
+        .Concat(self._GenerateDocumentation(f))
+        .Append("%s => %s;" % (self._GenerateFunctionSignature(f),
+                               self._GenerateProxyCall(f)))
+        )
+
+  def _GenerateProxyCall(self, function, callTarget='this._jsObject'):

[not at google - send to devlin, 2013/01/29 16:37:08]

call_target

[sashab, 2013/01/30 05:26:03]

Done.

+    """Given a function, generates the code to call that function via JS().
+    Returns a string.
+
+    |callTarget| is the name of the object to call the function on. The default
+    is this._jsObject.
+
+    e.g.
+        JS('void', '#.resizeTo(#, #)', this._jsObject, width, height)
+        JS('void', '#.setBounds(#)', this._jsObject, convertArgument(bounds))
+    """
+
+    format = ("JS('%(return_type)s', "
+              "'#.%(name)s(%(param_hashes)s)', "
+              "%(target)s%(params)s)")
+
+    params = ""
+    if function.params:
+      params_wrapped = []
+      for param in function.params:
+        if not self._IsBaseType(param):
+          params_wrapped.append('convertArgument(%s)' % param.name)
+        else:
+          params_wrapped.append(param.name)
+      params = ', ' + ', '.join(params_wrapped)

[not at google - send to devlin, 2013/01/29 16:37:08]

the adding comma stuff is a bit icky, could you just add all the comma-separated
things above into a list and then ', '.join them at once, rather than separating
out the components?

[sashab, 2013/01/30 05:26:03]

Done.

+
+    return format % {
+      'return_type': self._GetPropertyType(function.returns),
+      'name': function.name,
+      'param_hashes': ', '.join('#' for p in function.params),
+      'target': callTarget,
+      'params': params
+    }
+
+  def _GenerateEvent(self, event):
+    """Given a Function object, returns the Code with the .dart for this event,
+    represented by the function.
+
+    All events extend the Event base type.
+    """
+    c = Code()
+
+    # Add documentation for this event.
+    (c.Append()
+      .Concat(self._GenerateDocumentation(event))
+      .Sblock('class Event_%(event_name)s extends Event {')
+    )
+
+    # Override Event callback type definitions.
+    for ret_type, event_func in (('void', 'addListener'),
+                                 ('void', 'removeListener'),
+                                 ('bool', 'hasListener')):
+
+      param_list = self._GenerateParameterList(event.params, event.callback,
+                                               allow_optional = False)
+
+      c.Append('%s %s(void callback(%s)) => super.%s(callback);' %
+          (ret_type, event_func, param_list, event_func))
+
+    # Generate the constructor.
+    (c.Append()
+      .Append('Event_%(event_name)s(jsObject) : '
+              'super(jsObject, %(param_num)d);')
+    )
+
+    (c.Eblock('}')
+      .Substitute({
+        'event_name': self._namespace.unix_name + '_' + event.name,
+        'param_num': len(event.params)
+      })
+    )
+
+    return c
+
+  def _GenerateMainClass(self):
+    """Generates the main class for this file, which links to all functions
+    and events.
+
+    Includes a ChromeApi member variable to represent the connection.
+
+    Returns a code object.
+    """
+    c = Code()
+    (c.Append()
+      .Sblock('class API_%(namespace_name)s {')
+      .Append('/*')
+      .Append(' * API connection')
+      .Append(' */')
+      .Append('Object _jsObject;')
+    )
+
+    # Add events.
+    if self._namespace.events:
+      (c.Append()
+        .Append('/*')
+        .Append(' * Events')
+        .Append(' */')
+      )
+    for event_name in self._namespace.events:
+      c.Append('Event_%s_%s %s;' % (self._namespace.unix_name, event_name,
+                                    event_name))
+
+    # Add functions.
+    if self._namespace.functions:
+      (c.Append()
+        .Append('/*')
+        .Append(' * Functions')
+        .Append(' */')
+      )
+    for function in self._namespace.functions.values():
+      c.Concat(self._GenerateFunction(function))
+
+    # Add the constructor.
+    (c.Append()
+      .Sblock('API_%(namespace_name)s(this._jsObject) {')
+    )
+
+    # Add events to constructor.
+    for event_name in self._namespace.events:
+      c.Append("%s = new Event_%s_%s(JS('', '#.%s', this._jsObject));" %
+        (event_name, self._namespace.unix_name, event_name, event_name))
+    c.Eblock('}')
+
+    (c.Eblock('}')
+      .Substitute({
+        'namespace_name': self._namespace.unix_name

[not at google - send to devlin, 2013/01/29 16:37:08]

substitution only used once, just %s it above?

[sashab, 2013/01/30 05:26:03]

Done.

+      })
+    )
+
+    return c
+
+  def _GeneratePropertySignature(self, prop, prepend_this=False,
+                                 omitBasicTypes=False,
+                                 functionsAsObjects=False,
+                                 withGetKeyword=False):

[not at google - send to devlin, 2013/01/29 16:37:08]

functionsAsObjects -> functions_as_objects
withGetKeyword -> with_get_keyword

That said I can't find anywhere where these parameters are used.

[sashab, 2013/01/30 05:26:03]

Fixed the camelcase problems. Removed the unused parameters (they were needed in
a previous version).

+    """Given a property, returns a signature for that property.
+    Recursively generates the signature for callbacks.
+    Returns a String for the given property.
+
+    * If |prepend_this| is True, prepends "this." to all variable names.
+    * If |omitBasicTypes| is True, only adds type names for function types.
+    * If |functionsAsObjects| is True, treats callbacks as basic types and
+      prepends the type 'Function'.
+    * If |withGetKeyword| is True, adds the word 'get' between the property's
+      type and name. Used for getters in dart.
+
+    e.g.
+      bool x
+      void onClosed()
+      void doSomething(bool x, void callback([String x]))
+
+    e.g. If prepend_this is True:
+      bool this.x
+      void this.onClosed()
+      void this.doSomething(bool x, void callback([String x]))
+
+    e.g. If omitBasicTypes is True:
+      this.x
+      void onClosed()
+      void doSomething(bool x, void callback([String x]))
+
+    e.g. If functionsAsObjects is True:
+      bool x
+      Function onClosed
+      Function doSomething
+
+    e.g. If withGetKeyword and functionsAsObjects is True:
+      bool get x
+      Function get onClosed
+      Function get doSomething
+    """
+    if self._IsFunction(prop) and not functionsAsObjects:
+      return self._GenerateFunctionSignature(prop.type_.function, prepend_this)

[not at google - send to devlin, 2013/01/29 16:37:08]

for optional arguments, be explicit, it's easier to navigate code (and since
optional arguments aren't ordered, it's easier to understand what's going on).
So specify prepend_this=prepend_this, silly as it looks.

[sashab, 2013/01/30 05:26:03]

Removed this parameter pass anyway, but note taken.

+    else:
+      name_parts = [prop.simple_name]
+      if prepend_this:
+        name_parts[0] = 'this.' + name_parts[0]

[not at google - send to devlin, 2013/01/29 16:37:08]

might be a bit more concise as

name = '%s%s' % ('this.' if prepend_this else '', prop.simple_name)

much as Python's ternary ifs confuse me a little bit each time I see one.

[sashab, 2013/01/30 05:26:03]

Not used anymore anyway.

+      if withGetKeyword:
+        name_parts = ['get'] + name_parts

[not at google - send to devlin, 2013/01/29 16:37:08]

somewhat likewise, like

if with_get_keyword:
  name = 'get %s' % name

[sashab, 2013/01/30 05:26:03]

Not used anymore anyway.

+
+      name = ' '.join(name_parts)
+      type_ = (self._GetPropertyType(prop) + ' ') if not omitBasicTypes else ''
+
+      return '%(type)s%(name)s' % {
+                 'type': type_,
+                 'name': name
+             }
+
+  def _GenerateFunctionSignature(self, function, prepend_this=False):
+    """Given a function object, returns the signature for that function.
+    Recursively generates the signature for callbacks.
+    Returns a String for the given function.
+
+    If prepend_this is True, adds "this." to the function's name.
+
+    e.g.
+      void onClosed()
+      bool isOpen([String type])
+      void doSomething(bool x, void callback([String x]))
+
+    e.g. If prepend_this is True:
+      void this.onClosed()
+      bool this.isOpen([String type])
+      void this.doSomething(bool x, void callback([String x]))
+    """
+    sig = '%(return_type)s %(name)s(%(params)s)'
+
+    # Get function return type.
+    if function.returns:
+      return_type = self._GetPropertyType(function.returns)
+    else:
+      return_type = 'void'
+
+    # Get function name.

[not at google - send to devlin, 2013/01/29 16:37:08]

comment here and above seems unnecessary

[sashab, 2013/01/30 05:26:03]

Done.

+    function_name = function.simple_name
+    if prepend_this:
+      function_name = 'this.' + function_name

[not at google - send to devlin, 2013/01/29 16:37:08]

use 'this.%s' % function_name rather than string concatenation

[sashab, 2013/01/30 05:26:03]

No longer used, but note taken.

+
+    return sig % {
+        'return_type': return_type,
+        'name': function_name,
+        'params': self._GenerateParameterList(function.params,
+            getattr(function, 'callback', None))

[not at google - send to devlin, 2013/01/29 16:37:08]

getattr/reflection shouldn't be necessary, if there's no callback then it should
be None, so just function.callback.

[sashab, 2013/01/30 05:26:03]

Done.

+    }
+
+  def _GenerateParameterList(self, params, callback=None,
+                             allow_optional=True):
+    """Given a list of function parameters, generates their signature (as a
+    string).
+
+    e.g.
+      [String type]
+      bool x, void callback([String x])
+
+    If allow_optional is False, ignores optional parameters. Useful for
+    callbacks, where optional parameters are not used.
+    """
+    # params lists (required & optional), to be joined with ,'s
+    # FIXME(sashab): assuming all optional params come after required ones
+    params_req = []
+    params_opt = []
+    for param in params:
+      p_sig = self._GeneratePropertySignature(param)
+      if allow_optional and param.optional:
+        params_opt.append(p_sig)
+      else:
+        params_req.append(p_sig)
+
+    # Add the callback, if it exists.
+    if callback:
+      c_sig = self._GenerateFunctionSignature(callback)
+      if callback.optional:
+        params_opt.append(c_sig)
+      else:
+        params_req.append(c_sig)
+
+    # join params

[not at google - send to devlin, 2013/01/29 16:37:08]

comment doesn't look right, also, can this all be just

return ', '.join(params_req + params_opt) ?

[sashab, 2013/01/30 05:26:03]

Fixed comment.

Optional parameters have to be in square brackets. This logic is quite messy.

no params: ''
2 required params: 'x, y'
2 optional params: '[a, b]'
2 req & 2 opt params: 'x, y, [a, b]'

Tried to clean this up a bit. Let me know what you think.

+    params = ''
+    if params_req:
+      params += ', '.join(params_req)
+      if params_opt:
+        params += ', '
+    if params_opt:
+      params += '[' + ', '.join(params_opt) + ']'
+
+    return params
+
+  def _GetNamespace(self, name):
+    """Given a name a.b.c, returns the namespace (in this case, a.b).
+    """
+    return name.rsplit('.', 1)[0]
+
+  def _GetBaseName(self, name):
+    """Given a name a.b.c, returns the base name of the path (in this case, c).
+    """
+    return name.rsplit('.', 1)[1]

[not at google - send to devlin, 2013/01/29 16:37:08]

I think there are utils for this nand GetNamespace already in schema_util.py.

[sashab, 2013/01/30 05:26:03]

Removed; replaced usages with the ones from schema_util.

+
+  def _IsFunction(self, prop):
+    """Given a model.Property, returns whether this type is a function.
+    """
+    return prop.type_.property_type == PropertyType.FUNCTION
+
+  def _IsObjectType(self, prop):
+    """Given a model.Property, returns whether this type is an object.
+    """
+    return (prop.type_.property_type == PropertyType.OBJECT or
+        prop.type_.property_type == PropertyType.ANY)

[not at google - send to devlin, 2013/01/29 16:37:08]

return prop.type_.property_name in [PropertyType.OBJECT, PropertyType.ANY]

[sashab, 2013/01/30 05:26:03]

Done.

+
+  def _IsBaseType(self, prop):
+    """Given a model.Property, returns whether this type is a base type
+    (string, number or boolean).
+    """
+    base_type = self._GetPropertyType(prop)
+    if base_type in ['bool', 'num', 'int', 'double', 'String']:
+      return True
+    return False

[not at google - send to devlin, 2013/01/29 16:37:08]

return self._GetPropertyType(prop) in ['bool', 'num', ...]

[sashab, 2013/01/30 05:26:03]

Done.

+
+  def _GetReferencedType(self, name):
+    """Given the name of a referenced type, returns the type object for that

[not at google - send to devlin, 2013/01/29 16:37:08]

unused?

[sashab, 2013/01/30 05:26:03]

Removed.

+    reference.
+
+    Returns None if the type is not found.
+    """
+    if name in self._namespace.types:
+      return self._namespace.types[name]
+    return None
+
+  def _GetPropertyType(self, prop):
+    """Given a model.Property object, returns its type as a Dart string.
+    """
+    if prop == None:

[not at google - send to devlin, 2013/01/29 16:37:08]

is None

[sashab, 2013/01/30 05:26:03]

Done.

+      return 'void'
+
+    dart_type = None
+    type_ = prop.type_.property_type
+
+    if type_ == None:

[not at google - send to devlin, 2013/01/29 16:37:08]

for all comparisons here, use "is" not ==

[sashab, 2013/01/30 05:26:03]

Done.

+      dart_type = 'void'
+    elif type_ == PropertyType.REF:
+      if self._GetNamespace(prop.type_.ref_type) == self._namespace.name:
+        # This type is in this namespace; just use its base name.
+        dart_type = self._GetBaseName(prop.type_.ref_type)
+      else:
+        # TODO(sashab): Work out how to import this foreign type.
+        dart_type = prop.type_.ref_type
+    elif type_ == PropertyType.BOOLEAN:
+      dart_type = 'bool'

[not at google - send to devlin, 2013/01/29 16:37:08]

return 'bool'

[sashab, 2013/01/30 05:26:03]

Done.

+    elif type_ == PropertyType.INTEGER:
+      dart_type = 'int'

[not at google - send to devlin, 2013/01/29 16:37:08]

return 'int'

(etc)

[sashab, 2013/01/30 05:26:03]

Done.

+    elif type_ == PropertyType.INT64:
+      dart_type = 'num'
+    elif type_ == PropertyType.DOUBLE:
+      dart_type = 'double'
+    elif type_ == PropertyType.STRING:
+      dart_type = 'String'
+    elif type_ == PropertyType.ENUM:
+      dart_type = 'String'
+    elif type_ == PropertyType.CHOICES:
+      # TODO(sashab): What is Choices? Is it closer to a Map?

[not at google - send to devlin, 2013/01/29 16:37:08]

who are you asking?

[sashab, 2013/01/30 05:26:03]

Anyone who knows? ^^

What is the Dart equivalent of a Choices property type? Is it closer to a Map,
or an object?

[not at google - send to devlin, 2013/01/31 02:08:48]

Heh. I was just making sure you weren't asking me :) I'd suggest chatting to
calamity who knows a lot about these things (he was original author of the
compiler) and... somebody who knows dart.

Though for some background: JS isn't typesafe so you can just pass any old value
in obviously. C++ is typesafe so we generate a struct like (.. if there was a
field "foo" with choice between int and bool):

struct FooType {
  int as_int;
  bool as_bool;
}

So... yep, those are at least a couple of options. Another would be to try to
generate a different version of the function for each possible parameter list,
but I think this would get really complicated and not work in many cases (in
fact, I recently rewrote a bunch of the schema compiler to not do this since it
prevented us from supporting a few things).

I suspect your best bet is to think of a syntactically sugary way of supporting
the struct variant, which my knowledge of dart is insufficient to help with.

[sashab, 2013/01/31 04:41:40]

Object will support all types, but obviously stricter is better.

By "struct", it sounds like you really mean "union". I'll investigate a Dart
equivalent of this, otherwise I think the best option is to just use "Map", and
hope they have the right keys/values etc.

Maybe another option is to create a new class that specialises Map, and allows
setting of just one of these variables. I'll ask vsm.

+      dart_type = 'Object'
+    elif type_ == PropertyType.ANY:
+      dart_type = 'Object'
+    elif type_ == PropertyType.OBJECT:
+      # TODO(sashab): Work out a mapped type name?

[not at google - send to devlin, 2013/01/29 16:37:08]

who are you asking?

[sashab, 2013/01/30 05:26:03]

That was directed at myself; not sure what to do here - in the case of a generic
object, it may not be serializable to JS from Dart.

[not at google - send to devlin, 2013/01/31 02:08:48]

uninformed idea: could you just not generate type information here, since dart
does optional typing?

[sashab, 2013/01/31 04:41:40]

Yes, that's why its "Object" if it can't figure out what type to use (all
objects in dart extend Object).

The problem is security, and stability - consider this generated code:

function Foo(Object bar) {
   serialize bar;
   do something in js with bar;
   return stuff;
}

Now, if the user calls this function like this:

Foo(1);

It'll be fine, since 1 is serializable.

But, if they call it like this:

Foo(new MyObj());

It will fail, since MyObj is not serializable (Dart has little introspection and
cannot serialize arbitrary objects).

So, in the case where the IDL doesn't have a type definition, it may be worth
either adding one or writing a custom override for it, just to get type safety.

[not at google - send to devlin, 2013/02/02 00:45:47]

Yep, I get that - but I presume ultimately that this will be passed to the JS
bindings (right?) which does runtime type checking. Runtime type check is
obviously not ideal but it's only in rare situations and it's certainly no worse
than what devs already need to deal with.

^^^ that all said, is there a misunderstanding happening here? Objects are
objects in the JS sense not the dart sense, and should generate structs. It's a
very similar problem to the Choices one, except here I really do mean structs
not union (though above I actually *did* mean struct since I presumed dart
didn't support union).

So something like this, borrowing from what little dart I know: say there is an
object (JS object!) with properties "chocolate" which is a bool and
"blueberries" which is an int. In JS you'd write this like

// Not enough chocolate or blueberries.
chrome.muffins.bake({ chocolate: false, blueberries: 20 });

and the runtime type checking would make sure that when you call bake()
chocolate is a bool and blueberries is an int.

In C++ there is no equivalent of a JS object, so it would generate

struct BakeType {
  bool chocolate;
  int blueberries;
};

(and if you were actually able to make these calls from C++ it would look
something like...)

BakeType args;
args.chocolate = false;
args.blueberries = 20;
chrome::muffins::bake(args);

So what I was suggesting here is either do the JS thing and let the runttime
type stuff figure it out (seems like a reasonable way to start) or be clever and
do the C++ thing. From what I just read about dart 2 minutes ago, this might
look like

class BakeType {
  bool chocolate
  int blueberries
}

chrome.muffins.bake(new BakeType()
  ..chocolate = false
  ..blueberries = 20
)

...

Now choices remains a problem, and if dart doesn't support that explicitly then
I'd just be tempted to make it object and let JS runtime type checking kick in.

[sashab, 2013/02/04 05:09:27]

Ohhhh right, I understand what you're saying now.

Yup, this is all happening already - every type gets a generated "container"
which has all the properties it needs. It also has runtime type-checking like
you suggested.

The comment (# TODO(sashab): Work out a mapped type name?) is now out-of-date.

+      dart_type = prop.type_.instance_of or 'Object'

[not at google - send to devlin, 2013/02/02 00:45:47]

^^^

That said you could also generate for choices (if bake took an argument
"temperature" that was either a bool for hot/cold or a number):

class TemperatureType {
  static TemperatureType bool_choice(bool value) {
    // No idea if this is valid.
    return new TemperatureType()..as_bool = value
  }

  static TemperatureType int_choice(int value) {
    return new TemperatureType()..as_int = value
  }

  bool as_bool
  int as_int
}

chrome.muffins.bake(new BakeType()
  ..chocolate = true
  ..blueberries = 100
  ..temperature = TemperatureType.int_choice(200)
)

since I presume you couldn't have

class TemperatureType {
  TemperatureType(bool value) {
    as_bool = value
  }
  TemperatureType(int value) {
    as_int = value
  }
  bool as_bool
  int as_int
}

dart seems too dynamic-y to support that... don't quote me on it.

[sashab, 2013/02/04 05:09:27]

This is a great idea. Let me talk this over with the dart guys.

+    elif type_ == PropertyType.FUNCTION:
+      dart_type = 'Function'
+    elif type_ == PropertyType.ARRAY:
+      if hasattr(prop, 'item_type'):

[not at google - send to devlin, 2013/01/29 16:37:08]

if it's an ARRAY it will always have an item_type, don't need hasattr.

[sashab, 2013/01/30 05:26:03]

Done.

+        container_type = self._GetPropertyType(prop.item_type)
+        dart_type = 'List<%s>' % container_type
+      else:
+        dart_type = 'List'
+    elif type_ == PropertyType.BINARY:
+      dart_type = 'String'
+    else:
+      raise NotImplementedError(type_)
+
+    return dart_type.strip()

[not at google - send to devlin, 2013/01/29 16:37:08]

why do you need to .strip()?

[sashab, 2013/01/30 05:26:03]

Old fix; removed.