fix #27036, pass definite function types to LUB

pkg/analyzer/lib/src/task/strong/checker.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.
 
 // TODO(jmesserly): this was ported from package:dev_compiler, and needs to be
 // refactored to fit into analyzer.
 library analyzer.src.task.strong.checker;
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/dart/ast/ast.dart';
@@ skipping 11 matching lines @@
 import 'ast_properties.dart';
 
 bool isKnownFunction(Expression expression) {
   var element = _getKnownElement(expression);
   // First class functions and static methods, where we know the original
   // declaration, will have an exact type, so we know a downcast will fail.
   return element is FunctionElement ||
       element is MethodElement && element.isStatic;
 }
 
+/// Given an [expression] and a corresponding [typeSystem] and [typeProvider],
+/// gets the known static type of the expression.
+///
+/// Normally when we ask for an expression's type, we get the type of the
+/// storage slot that would contain it. For function types, this is necessarily
+/// a "fuzzy arrow" that treats `dynamic` as bottom. However, if we're
+/// interested in the expression's own type, it can often be a "strict arrow"
+/// because we know it evaluates to a specific, concrete function, and we can
+/// treat "dynamic" as top for that case, which is more permissive.
+DartType getDefiniteType(
+    Expression expression, TypeSystem typeSystem, TypeProvider typeProvider) {
+  DartType type = expression.staticType ?? DynamicTypeImpl.instance;
+  if (typeSystem is StrongTypeSystemImpl &&
+      type is FunctionType &&
+      _hasStrictArrow(expression)) {
+    // Remove fuzzy arrow if possible.
+    return typeSystem.functionTypeToConcreteType(typeProvider, type);
+  }
+  return type;
+}
+
 bool _hasStrictArrow(Expression expression) {
   var element = _getKnownElement(expression);
   return element is FunctionElement || element is MethodElement;
 }
 
 Element _getKnownElement(Expression expression) {
   if (expression is ParenthesizedExpression) {
     expression = (expression as ParenthesizedExpression).expression;
   }
   if (expression is FunctionExpression) {
@@ skipping 138 matching lines @@
       }
     }
   }
 
   /// Analyzer checks boolean conversions, but we need to check too, because
   /// it uses the default assignability rules that allow `dynamic` and `Object`
   /// to be assigned to bool with no message.
   void checkBoolean(Expression expr) =>
       checkAssignment(expr, typeProvider.boolType);
 
-  void checkFunctionApplication(
-      Expression node, Expression f, ArgumentList list) {
-    if (_isDynamicCall(f)) {
+  void checkFunctionApplication(InvocationExpression node) {

[Jennifer Messerly, 2016/08/09 12:35:19]

fyi -- I did a small refactor in patch set 2, taking advantage of
InvocationExpression to treat things more uniformly.

It might be a slight fix as well for calls to generic functions via
FunctionInvocationExpressions (which doesn't come up much, but if it did...)

+    var ft = _getTypeAsCaller(node);
+
+    if (_isDynamicCall(node, ft)) {
       // If f is Function and this is a method invocation, we should have
       // gotten an analyzer error, so no need to issue another error.
-      _recordDynamicInvoke(node, f);
+      _recordDynamicInvoke(node, node.function);
     } else {
-      checkArgumentList(list, _getTypeAsCaller(f));
+      checkArgumentList(node.argumentList, ft);
     }
   }
 
   DartType getType(TypeName name) {
     return (name == null) ? DynamicTypeImpl.instance : name.type;
   }
 
   void reset() {
     _failure = false;
   }
@@ skipping 208 matching lines @@
   }
 
   @override
   void visitFunctionExpression(FunctionExpression node) {
     _checkForUnsafeBlockClosureInference(node);
     super.visitFunctionExpression(node);
   }
 
   @override
   void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
-    checkFunctionApplication(node, node.function, node.argumentList);
+    checkFunctionApplication(node);
     node.visitChildren(this);
   }
 
   @override
   void visitIfStatement(IfStatement node) {
     checkBoolean(node.condition);
     node.visitChildren(this);
   }
 
   @override
@@ skipping 107 matching lines @@
       //
       // ... from case like:
       //
       //     SomeType s;
       //     s.someDynamicField(...); // static get, followed by dynamic call.
       //
       // The first case is handled here, the second case is handled below when
       // we call [checkFunctionApplication].
       setIsDynamicInvoke(node.methodName, true);
     } else {
-      checkFunctionApplication(node, node.methodName, node.argumentList);
+      checkFunctionApplication(node);
     }
     node.visitChildren(this);
   }
 
   @override
   void visitPostfixExpression(PostfixExpression node) {
     _checkUnary(node);
     node.visitChildren(this);
   }
 
@@ skipping 397 matching lines @@
     if (type.isDynamic) {
       return type;
     } else if (type is InterfaceType && type.element == expectedType.element) {
       return type.typeArguments[0];
     } else {
       // Malformed type - fallback on analyzer error.
       return null;
     }
   }
 
-  DartType _getStaticType(Expression expr) {
-    DartType t = expr.staticType ?? DynamicTypeImpl.instance;
-
-    // Remove fuzzy arrow if possible.
-    if (t is FunctionType && _hasStrictArrow(expr)) {
-      t = rules.functionTypeToConcreteType(typeProvider, t);
-    }
-
-    return t;
-  }
+  DartType _getStaticType(Expression expr) =>

[vsm, 2016/08/10 21:22:21]

Maybe rename to _getDefiniteType?

The terminology between here and static_type_analyzer seems inconsistent - there
getStaticType returns the fuzzy one.  

[Jennifer Messerly, 2016/08/10 22:28:09]

good catch. Done!

+      getDefiniteType(expr, rules, typeProvider);
 
   /// Given an expression, return its type assuming it is
   /// in the caller position of a call (that is, accounting
   /// for the possibility of a call method).  Returns null
   /// if expression is not statically callable.
-  FunctionType _getTypeAsCaller(Expression node) {
-    DartType t = _getStaticType(node);
-    if (node is SimpleIdentifier) {
-      Expression parent = node.parent;
-      if (parent is MethodInvocation) {
-        t = parent.staticInvokeType;
-      }
-    }
-    if (t is InterfaceType) {
-      return rules.getCallMethodType(t);
-    }
-    if (t is FunctionType) {
-      return t;
+  FunctionType _getTypeAsCaller(InvocationExpression node) {
+    DartType type = node.staticInvokeType;
+    if (type is FunctionType) {
+      return type;
+    } else if (type is InterfaceType) {
+      return rules.getCallMethodType(type);
     }
     return null;
   }
 
   /// Returns `true` if the expression is a dynamic function call or method
   /// invocation.
-  bool _isDynamicCall(Expression call) {
-    var ft = _getTypeAsCaller(call);
+  bool _isDynamicCall(InvocationExpression call, FunctionType ft) {
     // TODO(leafp): This will currently return true if t is Function
     // This is probably the most correct thing to do for now, since
     // this code is also used by the back end.  Maybe revisit at some
     // point?
     if (ft == null) return true;
     // Dynamic as the parameter type is treated as bottom.  A function with
     // a dynamic parameter type requires a dynamic call in general.
     // However, as an optimization, if we have an original definition, we know
     // dynamic is reified as Object - in this case a regular call is fine.
-    if (_hasStrictArrow(call)) {
+    if (_hasStrictArrow(call.function)) {
       return false;
     }
     return rules.anyParameterType(ft, (pt) => pt.isDynamic);
   }
 
   /// Returns `true` if the target expression is dynamic.
   bool _isDynamicTarget(Expression node) {
     if (node == null) return false;
 
     if (_isLibraryPrefix(node)) return false;
@@ skipping 486 matching lines @@
     var visited = new Set<InterfaceType>();
     do {
       visited.add(current);
       current.mixins.reversed.forEach(
           (m) => _checkIndividualOverridesFromClass(node, m, seen, true));
       _checkIndividualOverridesFromClass(node, current.superclass, seen, true);
       current = current.superclass;
     } while (!current.isObject && !visited.contains(current));
   }
 }