Don't call dinvoke on Object methods

lib/src/checker/rules.dart

Index: lib/src/checker/rules.dart
diff --git a/lib/src/checker/rules.dart b/lib/src/checker/rules.dart
index 5fe33e6f811ca1fba3e9cb48f4be27fde31d851c..e6e0569b5d0537c128f67a8263457a57e003f47f 100644
--- a/lib/src/checker/rules.dart
+++ b/lib/src/checker/rules.dart
@@ -43,7 +43,7 @@ abstract class TypeRules {
 
   bool isDynamic(DartType t);
   bool isDynamicTarget(Expression expr);
-  bool isDynamicGet(Expression expr);
+  bool isDynamicGet(Expression expr, String name);
   bool isDynamicCall(Expression call);
 }
 
@@ -74,7 +74,7 @@ class DartRules extends TypeRules {
   /// By default, all invocations are dynamic in Dart.
   bool isDynamic(DartType t) => true;
   bool isDynamicTarget(Expression expr) => true;
-  bool isDynamicGet(Expression expr) => true;
+  bool isDynamicGet(Expression expr, String name) => true;
   bool isDynamicCall(Expression call) => true;
 }
 
@@ -517,11 +517,14 @@ class RestrictedRules extends TypeRules {
 
   /// Returns `true` if the expression is a dynamic property access or prefixed
   /// identifier.
-  bool isDynamicGet(Expression expr) {
+  bool isDynamicGet(Expression expr, String name) {
+    // Check if this is a library-level (top-level access)
     if (options.ignoreTypes) return true;
     var t = getStaticType(expr);
     // TODO(jmesserly): we should not allow all property gets on `Function`
-    return t.isDynamic || t.isDartCoreFunction;
+    return (t.isDynamic || t.isDartCoreFunction) &&
+        name != 'hashCode' &&

[Jacob, 2015/04/02 18:29:49]

Add TODO: get all object properties from the AST.

+        name != 'runtimeType';
   }
 
   /// Returns `true` if the expression is a dynamic function call or method
@@ -529,9 +532,27 @@ class RestrictedRules extends TypeRules {
   bool isDynamicCall(Expression call) {
     if (options.ignoreTypes) return true;
     var t = getStaticType(call);
+
     // TODO(jmesserly): fix handling of types with `call` methods. These are not
     // FunctionType, but they also aren't dynamic calls.
+
     if (t.isDynamic || t.isDartCoreFunction || t is! FunctionType) {
+      // Special case Object.toString().  This is always safe to invoke.

[Jacob, 2015/04/02 18:29:49]

add a TODO to lookup all methods on Object instead of hardcoding.

[Jennifer Messerly, 2015/04/02 18:56:53]

Agreed. It's not too hard to look up the methodelement:
   provider.objectType.element.getMethod('toString')

I guess the analyzer has not resolved `call.element` to point at it? A quick
test with:
```dart
    Object foo;
    foo.toString();

    var bar;
    bar.toString();
```

"go to definition" for the former will jump to Object.toString(), for the latter
it says "no definition to go to"

I wonder if we should implement this at resolution time. It feels a bit off to
add a special cases like this to isDynamicCall. (also, instead of isDynamicCall
we should probably just check if resolution failed to find an element. That
means dynamic. And it's simpler.)

[vsm, 2015/04/02 19:21:56]

Good point.  I'll add the lookup.
 

+      if (call is SimpleIdentifier && call.name == 'toString') {

[Jennifer Messerly, 2015/04/02 18:56:53]

also, what about noSuchMethod? :)

[vsm, 2015/04/02 19:21:56]

We need the dinvoke to verify the type of the argument.  Actually, something
lightweight would do - but we  need something.

+        var parent = call.parent;
+        // Verify this is a method invocation with no arguments.
+        if (parent is MethodInvocation &&

[Jennifer Messerly, 2015/04/02 18:56:53]

getting the parent here feels a bit odd. we have to do it in the codegen
sometimes (no way to pass that down) but here we could just pass in the
MethodInvocation, or move this logic into visitMethodInvocation.

also: would this work for tear offs?

var f = obj.toString;
f();

[vsm, 2015/04/02 19:21:56]

Interesting thought.

If we infer that f is ()->*, we won't need the dinvoke.  If we only infer that f
is dynamic or Function, we need it. 

Perhaps we should be handling this earlier.

+            parent.methodName == call &&
+            parent.argumentList.arguments.isEmpty) {
+          // Verify that the target is an object (and not a library prefix).

[Jennifer Messerly, 2015/04/02 18:56:53]

this can't happen right? calls to library prefixes are never dynamic... they're
statically resolved top level methods.

[vsm, 2015/04/02 19:21:56]

It can ... see toString in my test.

+          var target = parent.realTarget;
+          if (target != null &&
+              (target is! SimpleIdentifier ||
+                  target.staticElement is! PrefixElement)) {
+            return false;
+          }
+        }
+      }
       return true;
     }
     // Dynamic as the parameter type is treated as bottom.  A function with