Simplify Go's Mojom Type Generator

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

 # Copyright 2015 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.
 
 '''Generates Go source files from a mojom.Module.'''
 
 from itertools import chain
 import os
 import re
 
 from mojom.generate.template_expander import UseJinja
 
 import mojom.generate.generator as generator
 import mojom.generate.module as mojom
 import mojom.generate.pack as pack
 

[azani, 2015/12/18 18:12:58]

I think you can remove the GENERATOR_PREFIX now.

[alexfandrianto, 2015/12/18 19:08:31]

Done.

+GENERATOR_PREFIX = 'go'
+
 class KindInfo(object):
   def __init__(self, go_type, encode_suffix, decode_suffix, bit_size):
     self.go_type = go_type
     self.encode_suffix = encode_suffix
     self.decode_suffix = decode_suffix
     self.bit_size = bit_size
 
 _kind_infos = {
   mojom.BOOL:                  KindInfo('bool', 'Bool', 'Bool', 1),
   mojom.INT8:                  KindInfo('int8', 'Int8', 'Int8', 8),
@@ skipping 273 matching lines @@
   path = GetPackagePath(imported['module'])
   if path in imports:
     return
   name = GetPackageName(imported['module'])
   while name in imports.values(): # This avoids repeated names.
     name += '_'
   imported['go_name'] = name
   imports[path] = name
   mojom_imports[path] = name
 
-# The identifier cache is used by the Type generator to determine if a type has
-# already been generated or not. This prevents over-generation of the same type
-# when it is referred to in multiple ways.
-identifier_cache = {}
 def GetIdentifier(kind):
-  # Use the kind's module to determine the package name.
+  # Use the kind's module to determine the package and name.

[azani, 2015/12/18 18:12:58]

Can you put this comment in the fuction's docstring?

def GetIdentifier(kind):
  """Use the kind's module to determine the package and name."""

[alexfandrianto, 2015/12/18 19:08:31]

Done.

+  # Note: InterfaceRequest's should use the Interface inside them.
   if hasattr(kind, 'module'):
     package = GetPackageName(kind.module)
+    name = kind.name
   elif mojom.IsInterfaceRequestKind(kind):
     package = GetPackageName(kind.kind.module)
+    name = kind.kind.name
   else:
+    # This is for a type that should not be stored. Return an empty identifier.

[azani, 2015/12/18 18:12:58]

I'm a little uncomfortable with this function promising to return the identifier
of a kind but occasionally returning an empty string. Could you document when
this will happen? (and maybe rename the function to something a bit more
descriptive) It seems to me we should raise an error here.

[alexfandrianto, 2015/12/18 19:08:31]

Yeah, we should raise an error.

[azani, 2015/12/18 23:19:11]

Maybe you forgot to commit. It's still returning an empty string.

     return ''
 
-  # Most kinds have a name, but those that don't should rely on their spec.
-  # 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] = {}
-  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]
+  return '%s_%s' % (package, name)
 
 # Get the mojom type's identifier suffix.
 def GetMojomTypeIdentifier(kind):
   # Since this should be unique, it is based on the type's identifier.
   return "%s__" % GetIdentifier(kind)
 
 class Generator(generator.Generator):
   go_filters = {
     'array': lambda kind: mojom.Array(kind),
     'bit_size': GetBitSize,
     'decode_suffix': DecodeSuffix,
     'encode_suffix': EncodeSuffix,
     'go_type': GetGoType,
     'expression_to_text': ExpressionToText,
     'has_response': lambda method: method.response_parameters is not None,
     'identifier': GetIdentifier,
-    'identifier_check': CheckIdentifier,
-    'identifier_store': StoreIdentifier,
     'is_array': mojom.IsArrayKind,
     'is_enum': mojom.IsEnumKind,
     'is_handle': mojom.IsAnyHandleKind,
     'is_interface': mojom.IsInterfaceKind,
     'is_interface_request': mojom.IsInterfaceRequestKind,
     'is_map': mojom.IsMapKind,
     'is_none_or_empty': lambda array: array is None or len(array) == 0,
     'is_nullable': mojom.IsNullableKind,
     'is_pointer': IsPointer,
     'is_object': mojom.IsObjectKind,
     'is_struct': mojom.IsStructKind,
     'is_union': mojom.IsUnionKind,
     'qualified': GetQualifiedName,
     'fullidentifier': GetFullIdentifier,
     'mojom_type': GetMojomTypeValue,
     'mojom_type_identifier': GetMojomTypeIdentifier,
     'name': GetNameForElement,
     'unqualified_name': GetUnqualifiedNameForElement,
     'package': GetPackageNameForElement,
     'tab_indent': lambda s, size = 1: ('\n' + '\t' * size).join(s.splitlines())
   }
 
-  # TODO: This value should be settable via arguments. If False, then mojom type
-  # information will not be generated.
-  should_gen_mojom_types = True
+  # If set to True, then mojom type information will be generated.
+  should_gen_mojom_types = False
 
   def GetParameters(self):
     package = GetPackageName(self.module)
     imports, mojom_imports = self.GetImports()
     return {
       'enums': GetAllEnums(self.module),
       'imports': imports,
       'interfaces': self.GetInterfaces(),
       'mojom_imports': mojom_imports,
       'package': package,
       'structs': self.GetStructs(),
       'descpkg': '%s.' % _service_describer_pkg_short \
         if package != _service_describer_pkg_short else '',
       'typepkg': '%s.' % _mojom_types_pkg_short \
         if package != _mojom_types_pkg_short else '',
       'unions': self.GetUnions()
     }
 
   @UseJinja('go_templates/source.tmpl', filters=go_filters)
   def GenerateSource(self):
     return self.GetParameters()
 
   def GenerateFiles(self, args):
+    if "--generate_type_info" in args:

[azani, 2015/12/18 18:12:58]

self.should_gen_mojom_types = "--generate_type_info" in args

[alexfandrianto, 2015/12/18 19:08:31]

Done.

+        self.should_gen_mojom_types = True
+
     self.Write(self.GenerateSource(), os.path.join("go", "src",
         GetPackagePath(self.module), "%s.go" % self.module.name))
 
   def GetJinjaParameters(self):
     return {
       'lstrip_blocks': True,
       'trim_blocks': True,
     }
 
   def GetGlobals(self):
@@ skipping 88 matching lines @@
   # Overrides the implementation from the base class in order to customize the
   # struct and field names.
   def _GetResponseStructFromMethod(self, method):
     params_class = "%s_%s_ResponseParams" % (
         GetNameForElement(method.interface), GetNameForElement(method))
     struct = mojom.Struct(params_class, module=method.interface.module)
     for param in method.response_parameters:
       struct.AddField("out%s" % GetNameForElement(param),
           param.kind, param.ordinal, attributes=param.attributes)
     return self._AddStructComputedData(False, struct)