Fix behavior when typed JS interop getters are called as functions.

pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart

 // Copyright (c) 2014, 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.
 
 library dart2js.js_emitter.program_builder;
 
 import '../js_emitter.dart' show
     ClassStubGenerator,
     CodeEmitterTask,
     computeMixinClass,
@@ skipping 12 matching lines @@
     Names,
     Selectors;
 import '../../compiler.dart' show
     Compiler;
 import '../../constants/values.dart' show
     ConstantValue,
     InterceptorConstantValue;
 import '../../core_types.dart' show
     CoreClasses;
 import '../../dart_types.dart' show
-    DartType;
+    DartType,
+    FunctionType,
+    TypedefType;
 import '../../elements/elements.dart' show
     ClassElement,
     Element,
     Elements,
     FieldElement,
     FunctionElement,
     FunctionSignature,
+    GetterElement,
     LibraryElement,
     MethodElement,
     Name,
     ParameterElement,
     TypedefElement,
     VariableElement;
 import '../../js/js.dart' as js;
 import '../../js_backend/backend_helpers.dart' show
     BackendHelpers;
 import '../../js_backend/js_backend.dart' show
@@ skipping 349 matching lines @@
                 if (stubNames.add(stubName.key)) {
                   interceptorClass.callStubs.add(_buildStubMethod(
                       stubName,
                       js.js('function(obj, v) { return obj.# = v }',
                           [member.name]),
                       element: member));
                 }
               }
             }
 
+            bool isFunctionLike = false;

[sra1, 2015/10/30 20:54:59]

Explain (comment) that this is generating stubs for direct calls and stubs for
call-through. 

[Jacob, 2015/10/30 22:31:53]

Done.

+            FunctionType functionType;

[sra1, 2015/10/30 20:54:59]

I like to add = null if it is not set one some path.

[Jacob, 2015/10/30 22:31:53]

Done.

+
             if (member.isFunction) {
+              FunctionElement fn = member;
+              functionType = fn.type;
+            } else if (member.isGetter) {
+              GetterElement getter = member;
+              DartType returnType = getter.type.returnType;
+              if (returnType.isFunctionType) {
+                functionType = returnType;
+              } else if (_compiler.types.isSubtype(returnType,
+                      backend.coreTypes.functionType) ||
+                  returnType.treatAsDynamic) {

[sra1, 2015/10/30 20:54:59]

Horrible indentation.  If it is the formatter, can it be fixed by switching ||
operands?

[Jacob, 2015/10/30 22:31:53]

done. shame line length isn't 100.

+                if (returnType.isTypedef) {
+                  TypedefType typedef = returnType;
+                  functionType = typedef.element.functionSignature.type;
+                } else {
+                  // Other misc function type such as coreTypes.Function.
+                  // Allow any number of arguments.
+                  isFunctionLike = true;
+                }
+              }
+            } // TODO(jacobr): handle field elements.
+
+            if (isFunctionLike || functionType != null) {
+              int minArgs;
+              int maxArgs;
+              if (functionType != null) {
+                minArgs = functionType.parameterTypes.length;
+                maxArgs = minArgs + functionType.optionalParameterTypes.length;
+              } else {
+                minArgs = 0;
+                maxArgs = 32767;
+              }
               var selectors =
                   _compiler.codegenWorld.invocationsByName(member.name);
               FunctionElement fn = member;

[sra1, 2015/10/30 20:54:59]

'fn' unused below.  Will fail if you TODO handle field elements

[Jacob, 2015/10/30 22:31:53]

done. forgot to remove this.

               // Named arguments are not yet supported. In the future we
               // may want to map named arguments to an object literal containing
               // all named arguments.
               if (selectors != null && !selectors.isEmpty) {
                 for (var selector in selectors.keys) {
                   // Check whether the arity matches this member.
                   var argumentCount = selector.argumentCount;
-                  if (argumentCount > fn.parameters.length) break;
-                  if (argumentCount < fn.parameters.length &&
-                      !fn.parameters[argumentCount].isOptional) break;
+                  // JS interop does not support named arguments.
+                  if (selector.namedArgumentCount > 0) break;
+                  if (argumentCount < minArgs) break;
+                  if (argumentCount > maxArgs) break;
                   var stubName = namer.invocationName(selector);
                   if (!stubNames.add(stubName.key)) break;
-                  var candidateParameterNames =
-                      'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLOMOPQRSTUVWXYZ';
                   var parameters = new List<String>.generate(argumentCount,
-                      (i) => candidateParameterNames[i]);
+                      (i) => 'p$i');

[Siggi Cherem (dart-lang), 2015/10/30 19:54:17]

you could also keep the old naming scheme to optimize for functions with few
arguments. For example:

var candidateNames = 'abc....';
(i) => i < 52 ? candidateNames[i] : '${candidateNames[i % 52]}${i ~/ 52}'

[Jacob, 2015/10/30 22:31:53]

not needed. these names will be minified anyway so the previous code was a
premature optimization.

 
                   interceptorClass.callStubs.add(_buildStubMethod(
                       stubName,

[sra1, 2015/10/30 20:54:59]

Do you get duplicate stubs?

[Jacob, 2015/10/30 22:31:53]

no because of 
if (!stubNames.add(stubName.key)) break

siggi complained about my deep nesting so it is written this way which makes it
a little harder to spot this line.

                       js.js('function(receiver, #) { return receiver.#(#) }',
                           [parameters, member.name, parameters]),

[sra1, 2015/10/30 20:54:59]

This is correct for function stubs, but is it what you want for call-through
stubs? I guess they have to be the same.

[Jacob, 2015/10/30 22:31:53]

this is awesome for call through stubs. It means it doesn't have to be a compile
time error if a JS interop subclass defines a method as a getter returning a
Function while the base class has a method... which frequently happens in d.ts
files as JavaScript devs don't really care about distinguishing the two.
Added a comment describing that this is the intended effect.

                       element: member));
                 }
               }
             }
           });
         }
       }
     });
   }
 
@@ skipping 519 matching lines @@
       Constant constant = new Constant(name, holder, constantValue);
       _constants[constantValue] = constant;
     }
   }
 
   Holder _registerStaticStateHolder() {
     return _registry.registerHolder(
         namer.staticStateHolder, isStaticStateHolder: true);
   }
 }