Generate Mojom Types for Dart

mojo/public/tools/bindings/generators/mojom_dart_generator.py

Index: mojo/public/tools/bindings/generators/mojom_dart_generator.py
diff --git a/mojo/public/tools/bindings/generators/mojom_dart_generator.py b/mojo/public/tools/bindings/generators/mojom_dart_generator.py
index 034588680ec55e6a4a00a4cedc2f2b8a4e1cfe89..c95cb9d5b76719ded6e44d9b89ae03127e5341ee 100644
--- a/mojo/public/tools/bindings/generators/mojom_dart_generator.py
+++ b/mojo/public/tools/bindings/generators/mojom_dart_generator.py
@@ -70,6 +70,32 @@ _kind_to_dart_decl_type = {
   mojom.NULLABLE_STRING:       "String"
 }
 
+_kind_to_mojom_type = {
+  mojom.BOOL:                  "BOOL",
+  mojom.INT8:                  "INT8",
+  mojom.UINT8:                 "UINT8",
+  mojom.INT16:                 "INT16",
+  mojom.UINT16:                "UINT16",
+  mojom.INT32:                 "INT32",
+  mojom.UINT32:                "UINT32",
+  mojom.FLOAT:                 "FLOAT",
+  mojom.HANDLE:                "UNSPECIFIED",
+  mojom.DCPIPE:                "DATA_PIPE_CONSUMER",
+  mojom.DPPIPE:                "DATA_PIPE_PRODUCER",
+  mojom.MSGPIPE:               "MESSAGE_PIPE",
+  mojom.SHAREDBUFFER:          "SHARED_BUFFER",
+  mojom.NULLABLE_HANDLE:       "UNSPECIFIED",
+  mojom.NULLABLE_DCPIPE:       "DATA_PIPE_CONSUMER",
+  mojom.NULLABLE_DPPIPE:       "DATA_PIPE_PRODUCER",
+  mojom.NULLABLE_MSGPIPE:      "MESSAGE_PIPE",
+  mojom.NULLABLE_SHAREDBUFFER: "SHARED_BUFFER",
+  mojom.INT64:                 "INT64",
+  mojom.UINT64:                "UINT64",
+  mojom.DOUBLE:                "DOUBLE",
+  mojom.STRING:                "STRING",
+  mojom.NULLABLE_STRING:       "STRING"
+}
+
 _spec_to_decode_method = {
   mojom.BOOL.spec:                  'decodeBool',
   mojom.DCPIPE.spec:                'decodeConsumerHandle',
@@ -122,6 +148,15 @@ _spec_to_encode_method = {
   mojom.UINT8.spec:                 'encodeUint8',
 }
 
+# The mojom_types.mojom and service_describer.mojom files are special because
+# they are used to generate mojom Type's and ServiceDescription implementations.
+_service_describer_pkg_short = "service_describer"
+_service_describer_pkg = "package:mojo/mojo/%s.mojom.dart" % \
+  _service_describer_pkg_short
+_mojom_types_pkg_short = "mojom_types"
+_mojom_types_pkg = "package:mojo/mojo/%s.mojom.dart" % \
+  _mojom_types_pkg_short
+
 def GetDartType(kind):
   if kind.imported_from:
     return kind.imported_from["unique_name"] + "." + GetNameForElement(kind)
@@ -172,6 +207,27 @@ def DartDeclType(kind):
   if mojom.IsEnumKind(kind):
     return GetDartType(kind)
 
+def GetMojomTypeValue(kind, typepkg=''):
+  if not kind in _kind_to_mojom_type:
+    return ''
+
+  nullable = 'true' if mojom.IsNullableKind(kind) else 'false'
+  w = _kind_to_mojom_type[kind]
+  if kind == mojom.BOOL or kind == mojom.FLOAT or kind == mojom.DOUBLE or \
+    mojom.IsIntegralKind(kind):
+
+    return 'new %sType()..simpleType = %sSimpleType.%s' % (typepkg, typepkg, w)
+  elif mojom.IsAnyHandleKind(kind):
+    return ('new %sType()\n..handleType = (new %sHandleType()' +
+      '\n..kind = %sHandleTypeKind.%s' +
+      '\n..nullable = %s)') % \
+      (typepkg, typepkg, typepkg, w, nullable)
+  elif mojom.IsStringKind(kind):
+    return 'new %sType()\n..stringType = (new %sStringType()..nullable = %s)' \
+      % (typepkg, typepkg, nullable)
+  else:
+    raise Exception('Missing case for kind: %s' % kind)
+
 def NameToComponent(name):
   # insert '_' between anything and a Title name (e.g, HTTPEntry2FooBar ->
   # HTTP_Entry2_FooBar)
@@ -382,6 +438,9 @@ def IsPointerArrayKind(kind):
 def IsEnumArrayKind(kind):
   return mojom.IsArrayKind(kind) and mojom.IsEnumKind(kind.kind)
 
+def IsImportedKind(kind):
+  return hasattr(kind, 'imported_from') and kind.imported_from is not None

[azani, 2015/11/20 20:17:58]

unless you need to differentiate between an empty dict and None, the convention
is to omit "is not None".

[alexfandrianto, 2015/11/21 03:58:34]

I didn't know that, thanks. (I like being very specific in what I'm comparing.)

+
 def ParseStringAttribute(attribute):
   assert isinstance(attribute, basestring)
   return attribute
@@ -392,12 +451,48 @@ def GetPackage(module):
   # Default package.
   return 'mojom'
 
+def GetPackageName(module):
+  return module.name.split('.')[0]
+
 def GetImportUri(module):
   package = GetPackage(module);
   elements = module.namespace.split('.')
   elements.append("%s" % module.name)
   return os.path.join(package, *elements)
 
+identifier_cache = {}

[azani, 2015/11/20 20:17:59]

I don't think you're actually using this as a cache. Can you maybe use a more
appropriate name.

[alexfandrianto, 2015/11/21 03:58:34]

How about identifier_store?

+def GetIdentifier(kind):
+  # Use the kind's module to determine the package name.
+  if hasattr(kind, 'module'):
+    package = GetPackageName(kind.module)
+  elif mojom.IsInterfaceRequestKind(kind):
+    package = GetPackageName(kind.kind.module)
+  else:
+    return ''

[azani, 2015/11/20 20:17:58]

This looks like an error condition. Can you raise an exception here?

[alexfandrianto, 2015/11/21 03:58:34]

I call GetIdentifier with every kind, including simple kinds, so I prefer
returning '' instead of throwing.

+
+  # Most kinds have a name, but those that don't should rely on their spec.

[azani, 2015/11/20 20:17:58]

In which case does a kind not have a name? You should not rely on spec as it may
go away in the future.

[alexfandrianto, 2015/11/21 03:58:34]

I moved the name computation earlier, so I have no more reliance on spec.
Thanks for pointing that out. (I also changed Go to mirror these changes.)

+  # Since spec can have : and ? characters, these must be replaced. Since ? is
+  # replaced with '', the caller must keep track of optionality on its own.
+  name_or_spec = (kind.name if hasattr(kind, 'name') else kind.spec)
+  package_unique = name_or_spec.replace(':', '_').replace('?', '')
+  return '%s_%s' % (package, package_unique)
+
+def StoreIdentifier(identifier, cache_name):
+  if not cache_name in identifier_cache:
+    identifier_cache[cache_name] = {}

[azani, 2015/11/20 20:17:58]

You could use a defaultdict instead.

[alexfandrianto, 2015/11/21 03:58:34]

I'll use defaultdict(lambda : defaultdict(bool))

+  identifier_cache[cache_name][identifier] = True
+  return ''
+
+def CheckIdentifier(identifier, cache_name):
+  if not cache_name in identifier_cache:
+    identifier_cache[cache_name] = {}
+  return identifier in identifier_cache[cache_name]
+
+# Get the mojom type's identifier suffix.

[azani, 2015/11/20 20:17:59]

Put this comment in the function's =docstring.

[alexfandrianto, 2015/11/21 03:58:34]

Done.

+def GetMojomTypeIdentifier(kind):
+  # Since this should be unique, it is based on the type's identifier.
+  return "_%s__" % GetIdentifier(kind)
+
 class Generator(generator.Generator):
 
   dart_filters = {
@@ -407,6 +502,13 @@ class Generator(generator.Generator):
     'default_value': DartDefaultValue,
     'encode_method': EncodeMethod,
     'expression_to_text': ExpressionToText,
+    'identifier': GetIdentifier,
+    'identifier_check': CheckIdentifier,

[azani, 2015/11/20 20:17:59]

Can you explain what the purpose of identifier_store and identifier_check is?
I'm really uncomfortable with running functions with side-effects as part of the
template generation.

[alexfandrianto, 2015/11/21 03:58:34]

The purpose is to detect if we have already generated information for a type
before. It breaks cycles and repeats.

+    'identifier_store': StoreIdentifier,
+    'mojom_type_value': GetMojomTypeValue,
+    'mojom_type_identifier': GetMojomTypeIdentifier,
+    'is_imported_kind': IsImportedKind,
+    'is_array_kind': mojom.IsArrayKind,
     'is_map_kind': mojom.IsMapKind,
     'is_nullable_kind': mojom.IsNullableKind,
     'is_pointer_array_kind': IsPointerArrayKind,
@@ -414,6 +516,8 @@ class Generator(generator.Generator):
     'is_struct_kind': mojom.IsStructKind,
     'is_union_kind': mojom.IsUnionKind,
     'is_enum_kind': mojom.IsEnumKind,
+    'is_interface_kind': mojom.IsInterfaceKind,
+    'is_interface_request_kind': mojom.IsInterfaceRequestKind,
     'dart_true_false': GetDartTrueFalse,
     'dart_type': DartDeclType,
     'name': GetNameForElement,
@@ -421,9 +525,17 @@ class Generator(generator.Generator):
     'interface_response_name': GetInterfaceResponseName,
     'dot_to_underscore': DotToUnderscore,
     'is_cloneable_kind': mojom.IsCloneableKind,
+    'upper_camel': UpperCamelCase,
   }
 
+  # TODO: This value should be settable via arguments. If False, then mojom type
+  # information will not be generated.
+  should_gen_mojom_types = True

[azani, 2015/11/20 20:17:59]

Given that most people likely won't want to generate the type information, maybe
we should implement that in that CL.

Also, I think TODOs should generally have a username associated with them.

[alexfandrianto, 2015/11/21 03:58:34]

I tried to plumb this most of the way through. This needs to be on for the tests
to pass.
I've modified public/tools/bindings/mojom_bindings_generator_v1.py, but I don't
know who calls this one, so I left a TODO there.
https://github.com/domokit/mojo/issues/558

+
   def GetParameters(self, args):
+    package = GetPackageName(self.module)
+    defInterface = len(self.module.interfaces) > 0
+
     return {
       "namespace": self.module.namespace,
       "imports": self.GetImports(args),
@@ -435,6 +547,21 @@ class Generator(generator.Generator):
       "interfaces": self.GetInterfaces(),
       "imported_interfaces": self.GetImportedInterfaces(),
       "imported_from": self.ImportedFrom(),
+      "typepkg": '%s.' % _mojom_types_pkg_short \

[azani, 2015/11/20 20:17:59]

In order to enhance readability, you could just set all those values in the
dictionary for the common case, and then remove them later in the function if
need be.

[alexfandrianto, 2015/11/21 03:58:34]

Done.

+        if package != _mojom_types_pkg_short else '',
+      "descpkg": '%s.' % _service_describer_pkg_short \
+        if package != _service_describer_pkg_short else '',
+      "mojom_types_import": 'import \'%s\' as %s;' % \
+        (_mojom_types_pkg, _mojom_types_pkg_short) \
+        if package != _mojom_types_pkg_short else '',
+      "service_describer_import": 'import \'%s\' as %s;' % \
+        (_service_describer_pkg, _service_describer_pkg_short) \
+        if defInterface and package != _service_describer_pkg_short else '',
+    }
+
+  def GetGlobals(self):
+    return {
+      'should_gen_mojom_types': self.should_gen_mojom_types,
     }
 
   @UseJinja("dart_templates/module.lib.tmpl", filters=dart_filters)