Fix #30858, generic function types not serializable when const.

Previously declaring for instance const lists (or even final lists) of
generic function types ie `final x = <void Function()>[]` would throw
errors during summarization.

The type parameters don't seem correctly stored yet, but that is an
edge case. Left a TODO, for now this should go in to prevent the
analyzer crash.

One thing I changed as well is that `serializeType` assumed it was
getting a type name, and `serializeTypeName` accepted types but threw
when it didn't get a type name. I flipped the names so that
`serializeTypeName` accepts a type name and `serializeType` accepts a
type in general and decides whether its a type name or a generic
function type to proceed with specialization.

Bug: 30858
Change-Id: Id128f8625cbf03bb94d05ff0efdbac3b158e637e
Reviewed-on: https://dart-review.googlesource.com/20481
Reviewed-by: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: MichaelRFairhurst

pkg/analyzer/lib/src/dart/element/type.dart

@@ -418,14 +418,17 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
    * Initialize a newly created function type that is semantically the same as
    * [original], but which has been syntactically renamed with fresh type
    * parameters at its outer binding site (if any).
+   *
+   * If type formals is empty, this returns the original unless [force] is set
+   * to [true].
    */
-  factory FunctionTypeImpl.fresh(FunctionType original) {
+  factory FunctionTypeImpl.fresh(FunctionType original, {bool force = false}) {
     // We build up a substitution for the type parameters,
     // {variablesFresh/variables} then apply it.
 
     var originalFormals = original.typeFormals;
     var formalCount = originalFormals.length;
-    if (formalCount == 0) return original;
+    if (formalCount == 0 && !force) return original;
 
     // Allocate fresh type variables
     var typeVars = <DartType>[];

pkg/analyzer/lib/src/summary/format.dart

@@ -2307,6 +2307,12 @@ class EntityRefBuilder extends Object
       _implicitFunctionTypeIndices ??= <int>[];
 
   /**
+   * Notice: This will be deprecated. However, its not deprecated yet, as we're
+   * keeping it for backwards compatibilty, and marking it deprecated makes it
+   * unreadable.
+   *
+   * TODO(mfairhurst) mark this deprecated, and remove its logic.
+   *
    * If this is a reference to a function type implicitly defined by a
    * function-typed parameter, a list of zero-based indices indicating the path
    * from the entity referred to by [reference] to the appropriate type

pkg/analyzer/lib/src/summary/format.fbs

@@ -1215,6 +1215,12 @@ table EntityRef {
   entityKind:EntityRefKind (id: 8);
 
   /**
+   * Notice: This will be deprecated. However, its not deprecated yet, as we're
+   * keeping it for backwards compatibilty, and marking it deprecated makes it
+   * unreadable.
+   *
+   * TODO(mfairhurst) mark this deprecated, and remove its logic.
+   *
    * If this is a reference to a function type implicitly defined by a
    * function-typed parameter, a list of zero-based indices indicating the path
    * from the entity referred to by [reference] to the appropriate type

pkg/analyzer/lib/src/summary/idl.dart

@@ -405,6 +405,12 @@ abstract class EntityRef extends base.SummaryClass {
   EntityRefKind get entityKind;
 
   /**
+   * Notice: This will be deprecated. However, its not deprecated yet, as we're
+   * keeping it for backwards compatibilty, and marking it deprecated makes it
+   * unreadable.
+   *
+   * TODO(mfairhurst) mark this deprecated, and remove its logic.
+   *
    * If this is a reference to a function type implicitly defined by a
    * function-typed parameter, a list of zero-based indices indicating the path
    * from the entity referred to by [reference] to the appropriate type

pkg/analyzer/lib/src/summary/link.dart

@@ -229,14 +229,18 @@ EntityRefBuilder _createLinkedType(
       // TODO(paulberry): do I need to store type arguments?
       return result;
     }
-    if (element is GenericFunctionTypeElement) {
+    if (element is GenericFunctionTypeElementForLink) {
+      // Function types are their own type parameter context
+      typeParameterContext = element;
       result.entityKind = EntityRefKind.genericFunctionType;
       result.syntheticReturnType = _createLinkedType(
           type.returnType, compilationUnit, typeParameterContext);
       result.syntheticParams = type.parameters
           .map((ParameterElement param) => _serializeSyntheticParam(
               param, compilationUnit, typeParameterContext))
           .toList();
+      _storeTypeArguments(
+          type.typeArguments, result, compilationUnit, typeParameterContext);
       return result;
     }
     // TODO(paulberry): implement other cases.

pkg/analyzer/lib/src/summary/summarize_ast.dart

@@ -74,7 +74,8 @@ class _ConstExprSerializer extends AbstractConstExprSerializer {
   @override
   EntityRefBuilder serializeConstructorRef(DartType type, Identifier typeName,
       TypeArgumentList typeArguments, SimpleIdentifier name) {
-    EntityRefBuilder typeBuilder = serializeType(type, typeName, typeArguments);
+    EntityRefBuilder typeBuilder =
+        serializeTypeName(type, typeName, typeArguments);
     if (name == null) {
       return typeBuilder;
     } else {
@@ -143,10 +144,14 @@ class _ConstExprSerializer extends AbstractConstExprSerializer {
   }
 
   @override
-  EntityRefBuilder serializeType(
+  EntityRefBuilder serializeTypeName(
       DartType type, Identifier name, TypeArgumentList arguments) {
-    return visitor.serializeType(name, arguments);
+    return visitor.serializeTypeName(name, arguments);
   }
+
+  @override
+  EntityRefBuilder serializeGenericFunctionType(GenericFunctionType node) =>
+      visitor.serializeGenericFunctionType(node);
 }
 
 /**
@@ -449,16 +454,15 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
     b.typeParameters =
         serializeTypeParameters(typeParameters, typeParameterScope);
     if (superclass != null) {
-      b.supertype = serializeTypeName(superclass);
+      b.supertype = serializeType(superclass);
     } else {
       b.hasNoSupertype = isCoreLibrary && name == 'Object';
     }
     if (withClause != null) {
-      b.mixins = withClause.mixinTypes.map(serializeTypeName).toList();
+      b.mixins = withClause.mixinTypes.map(serializeType).toList();
     }
     if (implementsClause != null) {
-      b.interfaces =
-          implementsClause.interfaces.map(serializeTypeName).toList();
+      b.interfaces = implementsClause.interfaces.map(serializeType).toList();
     }
     if (members != null) {
       scopes.add(buildClassMemberScope(name, members));
@@ -624,7 +628,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
     if (!isTopLevel) {
       b.isStatic = isDeclaredStatic;
     }
-    b.returnType = serializeTypeName(returnType);
+    b.returnType = serializeType(returnType);
     bool isSemanticallyStatic = isTopLevel || isDeclaredStatic;
     if (formalParameters != null) {
       bool oldMayInheritCovariance = _parametersMayInheritCovariance;
@@ -735,7 +739,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
    */
   void serializeFunctionTypedParameterDetails(UnlinkedParamBuilder b,
       TypeAnnotation returnType, FormalParameterList parameters) {
-    EntityRefBuilder serializedReturnType = serializeTypeName(returnType);
+    EntityRefBuilder serializedReturnType = serializeType(returnType);
     if (serializedReturnType != null) {
       b.type = serializedReturnType;
     }
@@ -759,7 +763,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
         serializeTypeParameters(node.typeParameters, typeParameterScope);
     b.syntheticReturnType = node.returnType == null
         ? serializeDynamic()
-        : serializeTypeName(node.returnType);
+        : serializeType(node.returnType);
     b.syntheticParams = node.parameters.parameters
         .map((FormalParameter p) => p.accept(this) as UnlinkedParamBuilder)
         .toList();
@@ -866,7 +870,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
    * a [EntityRef].  Note that this method does the right thing if the
    * name doesn't refer to an entity other than a type (e.g. a class member).
    */
-  EntityRefBuilder serializeType(
+  EntityRefBuilder serializeTypeName(
       Identifier identifier, TypeArgumentList typeArguments) {
     if (identifier == null) {
       return null;
@@ -911,8 +915,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
             'Unexpected identifier type: ${identifier.runtimeType}');
       }
       if (typeArguments != null) {
-        b.typeArguments =
-            typeArguments.arguments.map(serializeTypeName).toList();
+        b.typeArguments = typeArguments.arguments.map(serializeType).toList();
       }
       return b;
     }
@@ -924,9 +927,9 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
    * a [EntityRef].  Note that this method does the right thing if the
    * name doesn't refer to an entity other than a type (e.g. a class member).
    */
-  EntityRefBuilder serializeTypeName(TypeAnnotation node) {
+  EntityRefBuilder serializeType(TypeAnnotation node) {
     if (node is TypeName) {
-      return serializeType(node?.name, node?.typeArguments);
+      return serializeTypeName(node?.name, node?.typeArguments);
     } else if (node is GenericFunctionType) {
       return serializeGenericFunctionType(node);
     } else if (node != null) {
@@ -974,7 +977,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
       b.isStatic = isDeclaredStatic;
       b.name = variable.name.name;
       b.nameOffset = variable.name.offset;
-      b.type = serializeTypeName(variables.type);
+      b.type = serializeType(variables.type);
       b.documentationComment = serializeDocumentation(documentationComment);
       b.annotations = serializeAnnotations(annotations);
       b.codeRange = serializeCodeRange(variables.parent);
@@ -1158,7 +1161,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
       if (node.parameters != null) {
         serializeFunctionTypedParameterDetails(b, node.type, node.parameters);
       } else {
-        b.type = serializeTypeName(node.type);
+        b.type = serializeType(node.type);
       }
     }
     return b;
@@ -1222,7 +1225,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
     b.nameOffset = node.name.offset;
     b.typeParameters =
         serializeTypeParameters(node.typeParameters, typeParameterScope);
-    EntityRefBuilder serializedReturnType = serializeTypeName(node.returnType);
+    EntityRefBuilder serializedReturnType = serializeType(node.returnType);
     if (serializedReturnType != null) {
       b.returnType = serializedReturnType;
     }
@@ -1345,7 +1348,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
   @override
   UnlinkedParamBuilder visitSimpleFormalParameter(SimpleFormalParameter node) {
     UnlinkedParamBuilder b = serializeParameter(node);
-    b.type = serializeTypeName(node.type);
+    b.type = serializeType(node.type);
     return b;
   }
 
@@ -1361,7 +1364,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
     b.name = node.name.name;
     b.nameOffset = node.name.offset;
     if (node.bound != null) {
-      b.bound = serializeTypeName(node.bound);
+      b.bound = serializeType(node.bound);
     }
     b.annotations = serializeAnnotations(node.metadata);
     b.codeRange = serializeCodeRange(node);

pkg/analyzer/lib/src/summary/summarize_const_expr.dart

@@ -198,15 +198,25 @@ abstract class AbstractConstExprSerializer {
    * given [name] and [arguments].  The parameter [type] might be `null` if the
    * type is not resolved.
    */
-  EntityRefBuilder serializeType(
+  EntityRefBuilder serializeTypeName(
       DartType type, Identifier name, TypeArgumentList arguments);
 
+  /**
+   * Return [EntityRefBuilder] that corresponds to the [type], which is defined
+   * using generic function type syntax. These may appear as the type arguments
+   * of a const list, etc.
+   */
+  EntityRefBuilder serializeGenericFunctionType(GenericFunctionType type);
+
   /**
    * Return [EntityRefBuilder] that corresponds to the given [type].
    */
-  EntityRefBuilder serializeTypeName(TypeAnnotation type) {
+  EntityRefBuilder serializeType(TypeAnnotation type) {
     if (type is TypeName) {
-      return serializeType(type?.type, type?.name, type?.typeArguments);
+      return serializeTypeName(type?.type, type?.name, type?.typeArguments);
+    }
+    if (type is GenericFunctionType) {
+      return serializeGenericFunctionType(type);
     }
     throw new ArgumentError(
         'Cannot serialize an instance of ${type.runtimeType}');
@@ -407,12 +417,12 @@ abstract class AbstractConstExprSerializer {
     } else if (expr is AsExpression) {
       isValidConst = false;
       _serialize(expr.expression);
-      references.add(serializeTypeName(expr.type));
+      references.add(serializeType(expr.type));
       operations.add(UnlinkedExprOperation.typeCast);
     } else if (expr is IsExpression) {
       isValidConst = false;
       _serialize(expr.expression);
-      references.add(serializeTypeName(expr.type));
+      references.add(serializeType(expr.type));
       operations.add(UnlinkedExprOperation.typeCheck);
     } else if (expr is SuperExpression) {
       operations.add(UnlinkedExprOperation.pushSuper);
@@ -554,7 +564,7 @@ abstract class AbstractConstExprSerializer {
     }
     if (expr.typeArguments != null &&
         expr.typeArguments.arguments.length == 1) {
-      references.add(serializeTypeName(expr.typeArguments.arguments[0]));
+      references.add(serializeType(expr.typeArguments.arguments[0]));
       operations.add(UnlinkedExprOperation.makeTypedList);
     } else {
       operations.add(UnlinkedExprOperation.makeUntypedList);
@@ -573,8 +583,8 @@ abstract class AbstractConstExprSerializer {
     }
     if (expr.typeArguments != null &&
         expr.typeArguments.arguments.length == 2) {
-      references.add(serializeTypeName(expr.typeArguments.arguments[0]));
-      references.add(serializeTypeName(expr.typeArguments.arguments[1]));
+      references.add(serializeType(expr.typeArguments.arguments[0]));
+      references.add(serializeType(expr.typeArguments.arguments[1]));
       operations.add(UnlinkedExprOperation.makeTypedMap);
     } else {
       operations.add(UnlinkedExprOperation.makeUntypedMap);
@@ -698,7 +708,7 @@ abstract class AbstractConstExprSerializer {
     } else {
       ints.add(typeArguments.arguments.length);
       for (TypeAnnotation type in typeArguments.arguments) {
-        references.add(serializeTypeName(type));
+        references.add(serializeType(type));
       }
     }
   }

pkg/analyzer/lib/src/task/strong_mode.dart

@@ -169,7 +169,16 @@ class InstanceMemberInferrer {
           parameter, index, overriddenTypes[i].parameters);
       DartType type = matchingParameter?.type ?? typeProvider.dynamicType;
       if (parameterType == null) {
-        parameterType = type;
+        if (type is FunctionType) {
+          // The resulting parameter's type element has an `enclosingElement` of
+          // the overridden parameter. Change it to the overriding parameter.
+          parameterType = new FunctionTypeImpl.fresh(type, force: true);
+          (parameterType.element as ElementImpl).enclosingElement = parameter;
+          // TODO(mfairhurst) handle cases where non-functions contain functions
+          // See test_inferredType_parameter_genericFunctionType_asTypeArgument
+        } else {
+          parameterType = type;
+        }
       } else if (parameterType != type) {
         if (parameter is ParameterElementForLink) {
           parameter.setInferenceError(new TopLevelInferenceErrorBuilder(

pkg/analyzer/test/src/summary/linker_test.dart

@@ -467,6 +467,33 @@ class C extends A<int> {
     expect(fType.parameters[0].type.toString(), 'int');
   }
 
+  @failingTest
+  void test_inferredType_parameter_genericFunctionType_asTypeArgument() {
+    var bundle = createPackageBundle('''
+class A<T> {
+  A<R> map<R>(List<R Function(T)> fs) => null;
+}
+''', path: '/a.dart');
+    addBundle('/a.ds', bundle);
+    createLinker('''
+import 'a.dart';
+class C extends A<int> {
+  map<R2>(fs) => null;
+}
+''');
+    LibraryElementForLink library = linker.getLibrary(linkerInputs.testDartUri);
+    library.libraryCycleForLink.ensureLinked();
+    ClassElementForLink_Class c = library.getContainedName('C');
+    expect(c.methods, hasLength(1));
+    MethodElementForLink map = c.methods[0];
+    expect(map.parameters, hasLength(1));
+    InterfaceType iType = map.parameters[0].type;
+    expect(iType.typeArguments, hasLength(1));
+    FunctionType fType = iType.typeArguments[0];
+    expect(fType.returnType.toString(), 'R2');
+    expect(fType.parameters[0].type.toString(), 'int');
+  }
+
   void test_inferredType_staticField_dynamic() {
     createLinker('''
 dynamic x = null;