Infer parameter types on overrides

lib/src/checker/resolver.dart

Index: lib/src/checker/resolver.dart
diff --git a/lib/src/checker/resolver.dart b/lib/src/checker/resolver.dart
index b4954453288a42725160dd232a8e7a4d27e01c5c..698e2f836cabfc3000a93c00ea9e136a671c90a2 100644
--- a/lib/src/checker/resolver.dart
+++ b/lib/src/checker/resolver.dart
@@ -168,10 +168,10 @@ class LibraryResolverWithInference extends LibraryResolver {
             .forEach((f) => _inferFieldTypeFromOverride(f, pending));
         if (pending.isNotEmpty) _inferVariableFromInitializer(pending);
 
-        // Infer return-types from overrides
+        // Infer return-types and param-types from overrides
         cls.members
             .where((m) => m is MethodDeclaration && !m.isStatic)
-            .forEach(_inferMethodReturnTypeFromOverride);
+            .forEach(_inferMethodTypesFromOverride);
       } else {
         _inferVariableFromInitializer(cls.members
             .where(_isInstanceField)
@@ -231,18 +231,38 @@ class LibraryResolverWithInference extends LibraryResolver {
     }
   }
 
-  void _inferMethodReturnTypeFromOverride(MethodDeclaration method) {
+  void _inferMethodTypesFromOverride(MethodDeclaration method) {
     var methodElement = method.element;
-    if ((methodElement is MethodElement ||
-            methodElement is PropertyAccessorElement) &&
-        methodElement.returnType.isDynamic &&
-        method.returnType == null) {
-      var enclosingElement = methodElement.enclosingElement as ClassElement;
-      var type = searchTypeFor(enclosingElement.type, methodElement);
+    if (methodElement is! MethodElement &&
+        methodElement is! PropertyAccessorElement) return;
+
+    var enclosingElement = methodElement.enclosingElement as ClassElement;
+    FunctionType type =
+        null; // searchTypeFor(enclosingElement.type, methodElement);
+
+    // Infer the return type if omitted
+    if (methodElement.returnType.isDynamic && method.returnType == null) {
+      type = searchTypeFor(enclosingElement.type, methodElement);
       if (type != null && !type.returnType.isDynamic) {
         methodElement.returnType = type.returnType;
       }
     }
+
+    // Infer parameter types if omitted

[Brian Wilkerson, 2015/08/25 14:06:29]

In the ported version of the code we get all of the overridden methods, not just
one, and only infer a return type if they are equal. I'm guessing that for
parameter types we would want to pick the least upper bound of the corresponding
(non-dynamic) parameter types, possibly discarding Object.

[vsm, 2015/08/25 15:43:47]

Perhaps we should do equality for params as well then?

[Brian Wilkerson, 2015/08/25 15:54:51]

For the return type we'd need a greatest lower bound computation, which we don't
have. I decided that it wasn't worth writing it (mostly because I think most of
the time it wouldn't come up with anything useful).

For the parameter types we'd need a least upper bound, and I don't like the
implementation we have, so yes, we might want to leave it as equals until we can
get a good implementation of LUB.

[Leaf, 2015/08/25 17:03:50]

I think equality is fine for now.  For return types, we could eventually at
least handle the case that one is a subtype of the others.

+    if (method.parameters == null) return;
+    var parameters = method.parameters.parameters;
+    var length = parameters.length;
+    for (int i = 0; i < length; ++i) {
+      var parameter = parameters[i];
+      if (parameter is SimpleFormalParameter && parameter.type == null) {
+        type = type ?? searchTypeFor(enclosingElement.type, methodElement);

[Brian Wilkerson, 2015/08/25 14:06:29]

I can't say I'm fond of the lazy initialization technique of using '??', but
that aside, it might be better to hoist this line outside the for loop,
especially if searchTypeFor can ever return null, even though we might
needlessly perform the search one time.

[vsm, 2015/08/25 15:43:47]

Yes, it looks like a fairly expensive operation though & I was trying to
preserve that property of the original code - i.e., don't search unless there is
an untyped variable to infer.

Good catch on the null return.  Added a short-circuiting check for that.

+        if (type != null &&
+            type.parameters.length > i &&
+            !type.parameters[i].type.isDynamic) {
+          parameter.element.type = type.parameters[i].type;
+        }
+      }
+    }
   }
 
   void _inferVariableFromInitializer(Iterable<VariableDeclaration> variables) {