fixes #24507, allow known functions to be treated as strict arrows

In a known function, * -> * will be top -> top.
In all other function types, it will still be bottom -> top.

[email protected]

Review URL: https://codereview.chromium.org/1700523002 .

Author: John Messerly

pkg/analyzer/lib/src/generated/type_system.dart

@@ -284,18 +284,15 @@ class StrongTypeSystemImpl implements TypeSystem {
   /**
    * Check that [f1] is a subtype of [f2].
    *
-   * [fuzzyArrows] indicates whether or not the f1 and f2 should be
-   * treated as fuzzy arrow types (and hence dynamic parameters to f2 treated
-   * as bottom).
+   * This will always assume function types use fuzzy arrows, in other words
+   * that dynamic parameters of f1 and f2 are treated as bottom.
    */
-  bool _isFunctionSubtypeOf(FunctionType f1, FunctionType f2,
-      {bool fuzzyArrows: true}) {
-
+  bool _isFunctionSubtypeOf(FunctionType f1, FunctionType f2) {
     return FunctionTypeImpl.relate(
         f1,
         f2,
         (DartType t1, DartType t2) =>
-            _isSubtypeOf(t2, t1, null, dynamicIsBottom: fuzzyArrows),
+            _isSubtypeOf(t2, t1, null, dynamicIsBottom: true),
         instantiateToBounds,
         returnRelation: isSubtypeOf);
   }

pkg/analyzer/lib/src/task/strong/checker.dart

@@ -12,6 +12,7 @@ import 'package:analyzer/dart/ast/token.dart' show Token, TokenType;
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/generated/resolver.dart' show TypeProvider;
 import 'package:analyzer/src/generated/type_system.dart';
@@ -565,7 +566,7 @@ class CodeChecker extends RecursiveAstVisitor {
   }
 
   StaticInfo _checkAssignment(Expression expr, DartType toT) {
-    final fromT = expr.staticType ?? DynamicTypeImpl.instance;
+    final fromT = _getStaticType(expr);
     final Coercion c = _coerceTo(fromT, toT);
     if (c is Identity) return null;
     if (c is CoercionError) return new StaticTypeError(rules, expr, toT);
@@ -770,7 +771,63 @@ class CodeChecker extends RecursiveAstVisitor {
   }
 
   DartType _getStaticType(Expression expr) {
-    return expr.staticType ?? DynamicTypeImpl.instance;
+    DartType t = expr.staticType ?? DynamicTypeImpl.instance;
+
+    // Remove fuzzy arrow if possible.
+    if (t is FunctionType && StaticInfo.isKnownFunction(expr)) {
+      t = _removeFuzz(t);
+    }
+
+    return t;
+  }
+
+  /// Remove "fuzzy arrow" in this function type.
+  ///
+  /// Normally we treat dynamically typed parameters as bottom for function
+  /// types. This allows type tests such as `if (f is SingleArgFunction)`.
+  /// It also requires a dynamic check on the parameter type to call these
+  /// functions.
+  ///
+  /// When we convert to a strict arrow, dynamically typed parameters become
+  /// top. This is safe to do for known functions, like top-level or local
+  /// functions and static methods. Those functions must already be essentially
+  /// treating dynamic as top.
+  ///
+  /// Only the outer-most arrow can be strict. Any others must be fuzzy, because
+  /// we don't know what function value will be passed there.
+  // TODO(jmesserly): should we use a real "fuzzyArrow" bit on the function
+  // type? That would allow us to implement this in the subtype relation.
+  // TODO(jmesserly): we'll need to factor this differently if we want to
+  // move CodeChecker's functionality into existing analyzer. Likely we can
+  // let the Expression have a strict arrow, then in places were we do
+  // inference, convert back to a fuzzy arrow.
+  FunctionType _removeFuzz(FunctionType t) {
+    bool foundFuzz = false;
+    List<ParameterElement> parameters = <ParameterElement>[];
+    for (ParameterElement p in t.parameters) {
+      ParameterElement newP = _removeParameterFuzz(p);
+      parameters.add(newP);
+      if (p != newP) foundFuzz = true;
+    }
+    if (!foundFuzz) {
+      return t;
+    }
+
+    FunctionElementImpl function = new FunctionElementImpl("", -1);
+    function.synthetic = true;
+    function.returnType = t.returnType;
+    function.shareTypeParameters(t.typeFormals);
+    function.shareParameters(parameters);
+    return function.type = new FunctionTypeImpl(function);
+  }
+
+  /// Removes fuzzy arrow, see [_removeFuzz].
+  ParameterElement _removeParameterFuzz(ParameterElement p) {
+    if (p.type.isDynamic) {
+      return new ParameterElementImpl.synthetic(
+          p.name, typeProvider.objectType, p.parameterKind);
+    }
+    return p;
   }
 
   /// Given an expression, return its type assuming it is

pkg/analyzer/lib/src/task/strong/info.dart

@@ -138,16 +138,7 @@ abstract class DownCast extends CoercionInfo {
       }
     }
 
-    Element element = null;
-    if (expression is PropertyAccess) {
-      element = expression.propertyName.staticElement;
-    } else if (expression is Identifier) {
-      element = expression.staticElement;
-    }
-    // First class functions and static methods, where we know the original
-    // declaration, will have an exact type, so we know a downcast will fail.
-    if (element is FunctionElement ||
-        element is MethodElement && element.isStatic) {
+    if (StaticInfo.isKnownFunction(expression)) {
       return new StaticTypeError(rules, expression, toT);
     }
 
@@ -551,6 +542,19 @@ abstract class StaticInfo {
   // TODO(jmesserly): review the usage of error codes. We probably want our own,
   // as well as some DDC specific [ErrorType]s.
   ErrorCode toErrorCode();
+
+  static bool isKnownFunction(Expression expression) {
+    Element element = null;
+    if (expression is PropertyAccess) {
+      element = expression.propertyName.staticElement;
+    } else if (expression is Identifier) {
+      element = expression.staticElement;
+    }
+    // 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;
+  }
 }
 
 class StaticTypeError extends StaticError {

pkg/analyzer/test/src/task/strong/checker_test.dart

@@ -556,12 +556,11 @@ void main() {
       typedef A Right(dynamic x);         // Right branch
       typedef A Bottom(A x);              // Bottom of the lattice
 
-      dynamic left(A x) => x;
-      A bot(A x) => x;
-      dynamic top(dynamic x) => x;
-      A right(dynamic x) => /*info:DYNAMIC_CAST*/x;
-
       void main() {
+        Top top;
+        Left left;
+        Right right;
+        Bottom bot;
         {
           Top f;
           f = top;
@@ -571,29 +570,106 @@ void main() {
         }
         {
           Left f;
-          f = /*severe:STATIC_TYPE_ERROR*/top;
+          f = /*warning:DOWN_CAST_COMPOSITE*/top;
           f = left;
-          f = /*severe:STATIC_TYPE_ERROR*/right;
+          f = /*warning:DOWN_CAST_COMPOSITE*/right;
           f = bot;
         }
         {
           Right f;
-          f = /*severe:STATIC_TYPE_ERROR*/top;
-          f = /*severe:STATIC_TYPE_ERROR*/left;
+          f = /*warning:DOWN_CAST_COMPOSITE*/top;
+          f = /*warning:DOWN_CAST_COMPOSITE*/left;
           f = right;
           f = bot;
         }
         {
           Bottom f;
-          f = /*severe:STATIC_TYPE_ERROR*/top;
-          f = /*severe:STATIC_TYPE_ERROR*/left;
-          f = /*severe:STATIC_TYPE_ERROR*/right;
+          f = /*warning:DOWN_CAST_COMPOSITE*/top;
+          f = /*warning:DOWN_CAST_COMPOSITE*/left;
+          f = /*warning:DOWN_CAST_COMPOSITE*/right;
           f = bot;
         }
       }
    ''');
     });
 
+    test('dynamic - known functions', () {
+
+      // Our lattice should look like this:
+      //
+      //
+      //           Bot -> Top
+      //          /        \
+      //      A -> Top    Bot -> A
+      //       /     \      /
+      // Top -> Top   A -> A
+      //         \      /
+      //         Top -> A
+      //
+      checkFile('''
+        class A {}
+
+        typedef dynamic BotTop(dynamic x);
+        typedef dynamic ATop(A x);
+        typedef A BotA(dynamic x);
+        typedef A AA(A x);
+        typedef A TopA(Object x);
+        typedef dynamic TopTop(Object x);
+
+        dynamic aTop(A x) => x;
+        A aa(A x) => x;
+        dynamic topTop(dynamic x) => x;
+        A topA(dynamic x) => /*info:DYNAMIC_CAST*/x;
+
+        void main() {
+          BotTop botTop;
+          BotA botA;
+          {
+            BotTop f;
+            f = topTop;
+            f = aTop;
+            f = topA;
+            f = aa;
+          }
+          {
+            ATop f;
+            f = topTop;
+            f = aTop;
+            f = topA;
+            f = aa;
+          }
+          {
+            BotA f;
+            f = /*severe:STATIC_TYPE_ERROR*/topTop;
+            f = /*severe:STATIC_TYPE_ERROR*/aTop;
+            f = topA;
+            f = aa;
+          }
+          {
+            AA f;
+            f = /*severe:STATIC_TYPE_ERROR*/topTop;
+            f = /*severe:STATIC_TYPE_ERROR*/aTop;
+            f = topA;
+            f = aa;
+          }
+          {
+            TopTop f;
+            f = topTop;
+            f = /*severe:STATIC_TYPE_ERROR*/aTop;
+            f = topA;
+            f = /*severe:STATIC_TYPE_ERROR*/aa;
+          }
+          {
+            TopA f;
+            f = /*severe:STATIC_TYPE_ERROR*/topTop;
+            f = /*severe:STATIC_TYPE_ERROR*/aTop;
+            f = topA;
+            f = /*severe:STATIC_TYPE_ERROR*/aa;
+          }
+        }
+     ''');
+    });
+
     test('function literal variance', () {
       checkFile('''