Convert legacy 'native code' to JS-forms in dart:html

lib/html/scripts/systemhtml.py

 #!/usr/bin/python
 # Copyright (c) 2012, 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.
 
 """This module provides shared functionality for the system to generate
 Dart:html APIs from the IDL database."""
 
 import emitter
 import os
@@ skipping 627 matching lines @@
   def FinishInterface(self):
     pass
 
   def EmitFactoryProvider(self, constructor_info, factory_provider, emitter):
     template_file = ('factoryprovider_%s.darttemplate' %
                      self._html_interface_name)
     template = self._template_loader.TryLoad(template_file)
     if not template:
       template = self._template_loader.Load('factoryprovider.darttemplate')
 
+    arguments = constructor_info.ParametersAsArgumentList()
+    comma = ',' if arguments else ''
     emitter.Emit(
         template,
         FACTORYPROVIDER=factory_provider,
         CONSTRUCTOR=self._html_interface_name,
         PARAMETERS=constructor_info.ParametersImplementationDeclaration(self._DartType),
         NAMED_CONSTRUCTOR=constructor_info.name or self._html_interface_name,
-        ARGUMENTS=constructor_info.ParametersAsArgumentList())
+        ARGUMENTS=arguments,
+        PRE_ARGUMENTS_COMMA=comma,
+        ARGUMENTS_PATTERN=','.join(['#'] * len(constructor_info.param_infos)))
 
   def SecondaryContext(self, interface):
     if interface is not self._current_secondary_parent:
       self._current_secondary_parent = interface
       self._members_emitter.Emit('\n  // From $WHERE\n', WHERE=interface.id)
 
   def AddIndexer(self, element_type):
     """Adds all the methods required to complete implementation of List."""
     # We would like to simply inherit the implementation of everything except
     # length, [], and maybe []=.  It is possible to extend from a base
     # array implementation class only when there is no other implementation
     # inheritance.  There might be no implementation inheritance other than
     # DOMBaseWrapper for many classes, but there might be some where the
     # array-ness is introduced by a non-root interface:
     #
     #   interface Y extends X, List<T> ...
     #
     # In the non-root case we have to choose between:
     #
     #   class YImpl extends XImpl { add List<T> methods; }
     #
     # and
     #
     #   class YImpl extends ListBase<T> { copies of transitive XImpl methods; }
     #
     self._members_emitter.Emit(
         '\n'
-        '  $TYPE operator[](int index) native "return this[index];";\n',
+        '  $TYPE operator[](int index) => JS("$TYPE", "#[#]", this, index);\n',
         TYPE=self._NarrowOutputType(element_type))
 
     if 'CustomIndexedSetter' in self._interface.ext_attrs:
       self._members_emitter.Emit(
           '\n'
-          '  void operator[]=(int index, $TYPE value) native "this[index] = value";\n',
+          '  void operator[]=(int index, $TYPE value) =>'
+          ' JS("void", "#[#] = #", this, index, value);\n',
           TYPE=self._NarrowInputType(element_type))
     else:
       # The HTML library implementation of NodeList has a custom indexed setter
       # implementation that uses the parent node the NodeList is associated
       # with if one is available.
       if self._interface.id != 'NodeList':
         self._members_emitter.Emit(
             '\n'
             '  void operator[]=(int index, $TYPE value) {\n'
             '    throw new UnsupportedOperationException("Cannot assign element of immutable List.");\n'
@@ skipping 64 matching lines @@
     self._AddRenamingGetter(attribute, html_name)
     if not read_only:
       self._AddRenamingSetter(attribute, html_name)
 
   def _AddRenamingGetter(self, attr, html_name):
     conversion = self._OutputConversion(attr.type.id, attr.id)
     if conversion:
       return self._AddConvertingGetter(attr, html_name, conversion)
     return_type = self._NarrowOutputType(attr.type.id)
     self._members_emitter.Emit(
-        '\n  $TYPE get $HTML_NAME() native "return this.$NAME;";\n',
+        # TODO(sra): Use metadata to provide native name.
+        '\n  $TYPE get $HTML_NAME() => JS("$TYPE", "#.$NAME", this);\n',
         HTML_NAME=html_name,
         NAME=attr.id,
         TYPE=return_type)
 
   def _AddRenamingSetter(self, attr, html_name):
     conversion = self._InputConversion(attr.type.id, attr.id)
     if conversion:
       return self._AddConvertingSetter(attr, html_name, conversion)
     self._members_emitter.Emit(
-        '\n  void set $HTML_NAME($TYPE value)'
-        ' native "this.$NAME = value;";\n',
+        # TODO(sra): Use metadata to provide native name.
+        '\n  void set $HTML_NAME($TYPE value) {'
+        ' JS("void", "#.$NAME = #", this, value); }\n',

[blois, 2012/10/12 16:55:19]

Nit- should this be expanded to multiple lines? Not sure what the style guide is
for this and how close we want to try to follow it.

[sra1, 2012/10/12 21:39:47]

Done.

         HTML_NAME=html_name,
         NAME=attr.id,
         TYPE=self._NarrowInputType(attr.type.id))
 
   def _AddConvertingGetter(self, attr, html_name, conversion):
     self._members_emitter.Emit(
+        # TODO(sra): Use metadata to provide native name.
         '\n  $RETURN_TYPE get $HTML_NAME => $CONVERT(this._$(HTML_NAME));'
-        '\n  $NATIVE_TYPE get _$HTML_NAME() native "return this.$NAME;";'
+        '\n  $NATIVE_TYPE get _$HTML_NAME() =>'
+        ' JS("$NATIVE_TYPE", "#.$NAME", this);\n'
         '\n',
         CONVERT=conversion.function_name,
         HTML_NAME=html_name,
         NAME=attr.id,
         RETURN_TYPE=conversion.output_type,
         NATIVE_TYPE=conversion.input_type)
 
   def _AddConvertingSetter(self, attr, html_name, conversion):
     self._members_emitter.Emit(
+        # TODO(sra): Use metadata to provide native name.
         '\n  void set $HTML_NAME($INPUT_TYPE value) {'
         ' this._$HTML_NAME = $CONVERT(value); }'
-        '\n  void set _$HTML_NAME(/*$NATIVE_TYPE*/ value)'
-        ' native "this.$NAME = value;";'
+        '\n  void set _$HTML_NAME(/*$NATIVE_TYPE*/ value) {'
+        ' JS("void", "#.$NAME = #", this, value); }\n'
         '\n',
         CONVERT=conversion.function_name,
         HTML_NAME=html_name,
         NAME=attr.id,
         INPUT_TYPE=conversion.input_type,
         NATIVE_TYPE=conversion.output_type)
 
   def AmendIndexer(self, element_type):
     pass
 
@@ skipping 20 matching lines @@
           MODIFIERS='static ' if info.IsStatic() else '',
           TYPE=return_type,
           HTML_NAME=html_name,
           NAME=info.declared_name,
           PARAMS=info.ParametersImplementationDeclaration(
               lambda type_name: self._NarrowInputType(type_name)))
 
       operation_emitter.Emit(
           '\n'
           #'  // @native("$NAME")\n;'
           '  $MODIFIERS$TYPE $(HTML_NAME)($PARAMS) native "$NAME";\n')

[Anton Muhin, 2012/10/12 11:40:38]

just for my education, any reasons why it still uses native?  If it is switched
to JS helper, we can remove some cruft.

[sra1, 2012/10/12 21:39:47]

Some form where there is a simple renaming will continued to be supported.  We
will transition to using metadata at some point instead of the keyword.

There are two thing here that can't be achieved with JS(...).

1. The compiler needs to know there is already a member called $NAME to avoid
problems in choosing field names and minification.

2. The compiler can't inline the code since it does not know that the blob of
JavaScript generated by JS has the correct properties.
In general, you can't inline here, even if you have resolved the foo method
precisely:

   x.foo()

You can only inline if you know that the body of foo will throw a
NullPointerException before any other  effect.  JS(...) generated code is opaque
to the compiler, so has no guarantee and will prevent inlining.

It is also faster to directly generate the inlined code than inline an
expression.

[Anton Muhin, 2012/10/15 10:27:34]

I see, thanks, Stephen.  May main concern is this form is not supported for
statics, so either we should fork it and resort to JS for statics or keep
statics unsupported.

On 2012/10/12 21:39:47, sra1 wrote:

     else:
       self._members_emitter.Emit(
           '\n'
           '  $MODIFIERS$TYPE $NAME($PARAMS) native;\n',
           MODIFIERS='static ' if info.IsStatic() else '',
           TYPE=self._NarrowOutputType(info.type_name),
           NAME=info.name,
           PARAMS=info.ParametersImplementationDeclaration(
               lambda type_name: self._NarrowInputType(type_name)))
 
@@ skipping 250 matching lines @@
 
     library_emitter = self._multiemitter.FileEmitter(library_file_path)
     library_file_dir = os.path.dirname(library_file_path)
     auxiliary_dir = os.path.relpath(auxiliary_dir, library_file_dir)
     imports_emitter = library_emitter.Emit(
         self._template, AUXILIARY_DIR=massage_path(auxiliary_dir))
     for path in sorted(self._path_to_emitter.keys()):
       relpath = os.path.relpath(path, library_file_dir)
       imports_emitter.Emit(
           "#source('$PATH');\n", PATH=massage_path(relpath))