Don't call dinvoke on Object methods

lib/src/checker/resolver.dart

 // Copyright (c) 2015, 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.
 
 /// Encapsulates how to invoke the analyzer resolver and overrides how it
 /// computes types on expressions to use our restricted set of types.
 library dev_compiler.src.checker.resolver;
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/src/generated/ast.dart';
@@ skipping 646 matching lines @@
     var element = node.element as LocalVariableElementImpl;
     var exprType = expr.staticType;
     if (exprType is InterfaceType) {
       var iteratedType = _findIteratedType(exprType);
       if (iteratedType != null) {
         element.type = iteratedType;
       }
     }
   }
 
+  Map<String, DartType> _objectMemberMap = null;
+
+  Map<String, DartType> _getObjectMemberMap() {
+    if (_objectMemberMap == null) {
+      _objectMemberMap = new Map<String, DartType>();
+      var objectType = _typeProvider.objectType;
+      var element = objectType.element;
+      // Only record methods (including getters) with no parameters.  As parameters are contravariant wrt
+      // type, using Object's version may be too strict.
+      // Add instance methods.
+      element.methods
+          .where((method) => !method.isStatic && method.parameters.isEmpty)
+          .forEach((method) {
+        _objectMemberMap[method.name] = method.type;
+      });
+      // Add getters.
+      element.accessors
+          .where((member) => !member.isStatic && member.isGetter)
+          .forEach((member) {
+        _objectMemberMap[member.name] = member.type.returnType;
+      });
+    }
+    return _objectMemberMap;
+  }
+
+  List<DartType> _nonExtendableTypes = null;
+
+  bool _isExtendable(DartType t) {
+    if (_nonExtendableTypes == null) {
+      // TODO(vsm): Use the analyzer's list - see dartbug.com/23125.
+      _nonExtendableTypes = <DartType>[
+        _typeProvider.nullType,
+        _typeProvider.numType,
+        _typeProvider.intType,
+        _typeProvider.doubleType,
+        _typeProvider.boolType,
+        _typeProvider.stringType
+      ];
+    }
+    return !_nonExtendableTypes.contains(t);

[Jennifer Messerly, 2015/04/07 19:05:49]

whenever you're calling _isExtendable it's being inverted, and it's inverted
here. Maybe try these renames:


_isExtendable -> _isSealed (or _isFinalType)
_nonExtendableTypes -> _sealedTypes (or _finalTypes)

[vsm, 2015/04/07 19:48:46]

Done.

+  }
+
   @override // to propagate types to identifiers
   visitMethodInvocation(MethodInvocation node) {
     // TODO(sigmund): follow up with analyzer team - why is this needed?
     visitSimpleIdentifier(node.methodName);
     super.visitMethodInvocation(node);
 
+    // Search for Object methods.
+    var objectMap = _getObjectMemberMap();
+    var name = node.methodName.name;
+    if (node.staticType.isDynamic && objectMap.containsKey(name)) {
+      var target = node.target;
+      if (target is SimpleIdentifier &&

[Jennifer Messerly, 2015/04/07 19:05:49]

why does target need to be a SimpleIdentifier? does the code handle this case:

dynamic foo() { ... }

main() {
  print(foo().toString());
}

[vsm, 2015/04/07 19:48:46]

Good point.  Fixed and test added.

+          target.staticElement is! PrefixElement) {

[Jennifer Messerly, 2015/04/07 19:05:49]

follow up to above suggestion, I wonder if something like this would help:

/// Returns true if the expression is `dynamic`.
/// This excludes library prefixes, which pretend to be `dynamic` but really are
not.
bool _isDynamicExpression(Expression e) {
  return e.staticType.isDynamic && !(e is SimpleIdentifier && e.staticElement is
PrefixElement);
}


it seems like we're mostly trying to avoid the fake-dynamic for library
prefixes, would that address the issue?

[vsm, 2015/04/07 19:48:46]

It cleans it up.  Done.

+        var type = objectMap[name];
+        if (type is FunctionType && node.argumentList.arguments.isEmpty) {
+          node.methodName.staticType = type;
+          if (!_isExtendable(type.returnType)) {
+            // Don't infer the type of the overall expression in this case -
+            // it may be too strict.

[Jennifer Messerly, 2015/04/07 19:05:49]

can this actually happen? I thought we didn't allow loosening the return type.

class Derived extends Object {
  Object get toString() => 42; // I hope this is an error?
}


dynamic d = ...;
var x = d.toString(); // we can infer `x` is a String, right?

[vsm, 2015/04/07 19:48:46]

Note: my comment was in a confusing place - fixed it.

Your case is fine.  This is problematic:

class Derived extends Object {
  String toString([int x]) => '$x';
}

dynamic d = ...
var x = d.toString;
var y = x(42);

If we infer x as ()->String, we produce a static error on the above even though
it'd work at runtime.

+            node.staticType = type.returnType;
+          }
+        }
+      }
+    }
+
     var e = node.methodName.staticElement;
     if (e is FunctionElement &&
         e.library.name == '_foreign_helper' &&
         e.name == 'JS') {
       // Fix types for JS builtin calls.
       //
       // This code was taken from analyzer. It's not super sophisticated:
       // only looks for the type name in dart:core, so we just copy it here.
       //
       // TODO(jmesserly): we'll likely need something that can handle a wider
       // variety of types, especially when we get to JS interop.
       var args = node.argumentList.arguments;
       if (args.isNotEmpty && args.first is SimpleStringLiteral) {
         var coreLib = _typeProvider.objectType.element.library;
         var classElem = coreLib.getType(args.first.stringValue);
         if (classElem != null) node.staticType = classElem.type;
       }
     }
   }
 
+  void _inferObjectAccess(
+      Expression node, Expression target, SimpleIdentifier id) {
+    // Search for Object accesses.
+    var objectMap = _getObjectMemberMap();
+    var name = id.name;
+    if (node.staticType.isDynamic && objectMap.containsKey(name)) {
+      if (target is SimpleIdentifier &&
+          target.staticElement is! PrefixElement) {
+        var type = objectMap[name];
+        id.staticType = type;
+        if (!_isExtendable(type)) {
+          // Don't infer the type of the overall expression in this case -
+          // it may be too strict.
+          node.staticType = type;
+        }
+      }
+    }
+  }
+
+  @override
+  visitPropertyAccess(PropertyAccess node) {
+    super.visitPropertyAccess(node);
+
+    _inferObjectAccess(node, node.target, node.propertyName);
+  }
+
+  @override
+  visitPrefixedIdentifier(PrefixedIdentifier node) {
+    super.visitPrefixedIdentifier(node);
+
+    _inferObjectAccess(node, node.prefix, node.identifier);
+  }
+
   @override
   visitConditionalExpression(ConditionalExpression node) {
     // TODO(vsm): The static type of a conditional should be the LUB of the
     // then and else expressions.  The analyzer appears to compute dynamic when
     // one or the other is the null literal.  Remove this fix once the
     // corresponding analyzer bug is fixed:
     // https://code.google.com/p/dart/issues/detail?id=22854
     super.visitConditionalExpression(node);
     if (node.staticType.isDynamic) {
       var thenExpr = node.thenExpression;
       var elseExpr = node.elseExpression;
       if (thenExpr.staticType.isBottom) {
         node.staticType = elseExpr.staticType;
       } else if (elseExpr.staticType.isBottom) {
         node.staticType = thenExpr.staticType;
       }
     }
   }
 
   // Review note: no longer need to override visitFunctionExpression, this is
   // handled by the analyzer internally.
   // TODO(vsm): in visitbinaryExpression: check computeStaticReturnType result?
   // TODO(vsm): in visitFunctionDeclaration: Should we ever use the expression
   // type in a (...) => expr or just the written type?
 
 }