Give correct warnings/errors on type expressions.

sdk/lib/_internal/compiler/implementation/resolution/members.dart

Index: sdk/lib/_internal/compiler/implementation/resolution/members.dart
diff --git a/sdk/lib/_internal/compiler/implementation/resolution/members.dart b/sdk/lib/_internal/compiler/implementation/resolution/members.dart
index bc9081324c0d69d188c02d8720c795e2460056ce..c0ba39bff5dca9a8a16c5dc9a9e8c7c40ca29b86 100644
--- a/sdk/lib/_internal/compiler/implementation/resolution/members.dart
+++ b/sdk/lib/_internal/compiler/implementation/resolution/members.dart
@@ -1268,11 +1268,49 @@ class StatementScope {
   }
 }
 
+/**
+ * Interface for the predicates and methods needed by the [TypeResolver].
+ */
+abstract class TypeResolverContext {
+  Scope get scope;
+  Element get enclosingElement;
+
+  void error(Node node, MessageKind kind, [Map arguments = const {}]);
+  void warning(Node node, MessageKind kind, [Map arguments = const {}]);
+  void useType(Node node, DartType type);
+}
+
 class TypeResolver {
   final Compiler compiler;
+  TypeResolverContext context;
+  bool isTypeExpression;
 
   TypeResolver(this.compiler);
 
+  Scope get scope => context.scope;
+  Element get enclosingElement => context.enclosingElement;
+
+  void error(Node node, MessageKind kind, [Map arguments = const {}]) {
+    context.error(node, kind, arguments);
+  }
+
+  void warning(Node node, MessageKind kind, [Map arguments = const {}]) {
+    context.warning(node, kind, arguments);
+  }
+
+  void reportFailure(bool failureIsError,
+                     Node node, MessageKind kind, [Map arguments = const {}]) {
+    if (failureIsError) {
+      error(node, kind, arguments);
+    } else {
+      warning(node, kind, arguments);
+    }
+  }
+
+  void whenResolved(Node node, DartType type) {
+    context.useType(node, type);
+  }
+
   Element resolveTypeName(Scope scope,
                           SourceString prefixName,
                           Identifier typeName) {
@@ -1308,30 +1346,17 @@ class TypeResolver {
     }
   }
 
-  // TODO(johnniwinther): Change  [onFailure] and [whenResolved] to use boolean
-  // flags instead of closures.
-  DartType resolveTypeAnnotation(
-      TypeAnnotation node,
-      Scope scope,
-      Element enclosingElement,
-      {onFailure(Node node, MessageKind kind, [Map arguments]),
-       whenResolved(Node node, DartType type)}) {
-    if (onFailure == null) {
-      onFailure = (n, k, [arguments]) {};
-    }
-    if (whenResolved == null) {
-      whenResolved = (n, t) {};
-    }
-    if (scope == null) {
-      compiler.internalError('resolveTypeAnnotation: no scope specified');
-    }
-    return resolveTypeAnnotationInContext(scope, node, enclosingElement,
-        onFailure, whenResolved);
+  DartType resolveTypeExpression(TypeAnnotation node) {
+    this.isTypeExpression = true;

[ahe, 2013/02/05 11:26:29]

I don't think it is safe to call resolveTypeExpression from
resolveTypeAnnotation and vice versa. I cannot convince myself that is good. So
it would ease my mind if you could store the old value and restore it after the
call.

But no need to use try/finally.

+    return resolveTypeAnnotationInternal(node);
+  }
+
+  DartType resolveTypeAnnotation(TypeAnnotation node) {
+    this.isTypeExpression = false;
+    return resolveTypeAnnotationInternal(node);
   }
 
-  DartType resolveTypeAnnotationInContext(Scope scope, TypeAnnotation node,
-                                          Element enclosingElement,
-                                          onFailure, whenResolved) {
+  DartType resolveTypeAnnotationInternal(TypeAnnotation node) {
     Identifier typeName;
     SourceString prefixName;
     Send send = node.typeName.asSend();
@@ -1346,24 +1371,22 @@ class TypeResolver {
     Element element = resolveTypeName(scope, prefixName, typeName);
     DartType type;
 
-    DartType reportFailureAndCreateType(MessageKind messageKind,
+    DartType reportFailureAndCreateType(bool failureIsError,
+                                        MessageKind messageKind,
                                         Map messageArguments) {
-      onFailure(node, messageKind, messageArguments);
+      reportFailure(failureIsError, node, messageKind, messageArguments);
       var erroneousElement = new ErroneousElementX(
           messageKind, messageArguments, typeName.source, enclosingElement);
       var arguments = new LinkBuilder<DartType>();
-      resolveTypeArguments(
-          node, null, enclosingElement,
-          scope, onFailure, whenResolved, arguments);
+      resolveTypeArguments(node, null, arguments);
       return new MalformedType(erroneousElement, null, arguments.toLink());
     }
 
     DartType checkNoTypeArguments(DartType type) {
       var arguments = new LinkBuilder<DartType>();
-      bool hashTypeArgumentMismatch = resolveTypeArguments(
-          node, const Link<DartType>(), enclosingElement,
-          scope, onFailure, whenResolved, arguments);
-      if (hashTypeArgumentMismatch) {
+      bool hasTypeArgumentMismatch = resolveTypeArguments(

[ahe, 2013/02/05 11:26:29]

The old name was funnier ;-)

+          node, const Link<DartType>(), arguments);
+      if (hasTypeArgumentMismatch) {
         type = new MalformedType(
             new ErroneousElementX(MessageKind.TYPE_ARGUMENT_COUNT_MISMATCH,
                 {'type': node}, typeName.source, enclosingElement),
@@ -1373,14 +1396,14 @@ class TypeResolver {
     }
 
     if (element == null) {
-      type = reportFailureAndCreateType(
+      type = reportFailureAndCreateType(false,
           MessageKind.CANNOT_RESOLVE_TYPE, {'typeName': node.typeName});
     } else if (element.isAmbiguous()) {
       AmbiguousElement ambiguous = element;
-      type = reportFailureAndCreateType(
+      type = reportFailureAndCreateType(isTypeExpression,
           ambiguous.messageKind, ambiguous.messageArguments);
     } else if (!element.impliesType()) {
-      type = reportFailureAndCreateType(
+      type = reportFailureAndCreateType(false,
           MessageKind.NOT_A_TYPE, {'node': node.typeName});
     } else {
       if (identical(element, compiler.types.voidType.element) ||
@@ -1391,10 +1414,9 @@ class TypeResolver {
         compiler.resolver._ensureClassWillBeResolved(cls);
         element.computeType(compiler);
         var arguments = new LinkBuilder<DartType>();
-        bool hashTypeArgumentMismatch = resolveTypeArguments(
-            node, cls.typeVariables, enclosingElement,
-            scope, onFailure, whenResolved, arguments);
-        if (hashTypeArgumentMismatch) {
+        bool hasTypeArgumentMismatch = resolveTypeArguments(
+            node, cls.typeVariables, arguments);
+        if (hasTypeArgumentMismatch) {
           type = new MalformedType(
               new ErroneousElementX(MessageKind.TYPE_ARGUMENT_COUNT_MISMATCH,
                   {'type': node}, typeName.source, enclosingElement),
@@ -1412,8 +1434,7 @@ class TypeResolver {
         compiler.resolveTypedef(typdef);
         var arguments = new LinkBuilder<DartType>();
         bool hashTypeArgumentMismatch = resolveTypeArguments(
-            node, typdef.typeVariables, enclosingElement,
-            scope, onFailure, whenResolved, arguments);
+            node, typdef.typeVariables, arguments);
         if (hashTypeArgumentMismatch) {
           type = new MalformedType(
               new ErroneousElementX(MessageKind.TYPE_ARGUMENT_COUNT_MISMATCH,
@@ -1456,36 +1477,28 @@ class TypeResolver {
    * Returns [: true :] if the number of type arguments did not match the
    * number of type variables.
    */
-  bool resolveTypeArguments(
-      TypeAnnotation node,
-      Link<DartType> typeVariables,
-      Element enclosingElement,
-      Scope scope,
-      onFailure, whenResolved,
-      LinkBuilder<DartType> arguments) {
+  bool resolveTypeArguments(TypeAnnotation node,
+                            Link<DartType> typeVariables,
+                            LinkBuilder<DartType> arguments) {
     if (node.typeArguments == null) {
       return false;
     }
     bool typeArgumentCountMismatch = false;
-    for (Link<Node> typeArguments = node.typeArguments.nodes;
-         !typeArguments.isEmpty;
-         typeArguments = typeArguments.tail) {
+    for (Node typeArgument in node.typeArguments.nodes) {

[ahe, 2013/02/05 11:26:29]

Please avoid for-in.

       if (typeVariables != null && typeVariables.isEmpty) {
-        onFailure(typeArguments.head, MessageKind.ADDITIONAL_TYPE_ARGUMENT);
+        reportFailure(isTypeExpression,
+                      typeArgument, MessageKind.ADDITIONAL_TYPE_ARGUMENT);
         typeArgumentCountMismatch = true;
       }
-      DartType argType = resolveTypeAnnotationInContext(scope,
-                                                        typeArguments.head,
-                                                        enclosingElement,
-                                                        onFailure,
-                                                        whenResolved);
+      DartType argType = resolveTypeAnnotationInternal(typeArgument);
       arguments.addLast(argType);
       if (typeVariables != null && !typeVariables.isEmpty) {
         typeVariables = typeVariables.tail;
       }
     }
     if (typeVariables != null && !typeVariables.isEmpty) {
-      onFailure(node.typeArguments, MessageKind.MISSING_TYPE_ARGUMENT);
+      reportFailure(isTypeExpression,
+                    node.typeArguments, MessageKind.MISSING_TYPE_ARGUMENT);
       typeArgumentCountMismatch = true;
     }
     return typeArgumentCountMismatch;
@@ -1498,7 +1511,8 @@ class TypeResolver {
  * Do not subclass or instantiate this class outside this library
  * except for testing.
  */
-class ResolverVisitor extends CommonResolverVisitor<Element> {
+class ResolverVisitor extends CommonResolverVisitor<Element>
+    implements TypeResolverContext {
   final TreeElementMapping mapping;
   Element enclosingElement;
   final TypeResolver typeResolver;
@@ -1527,7 +1541,9 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
       scope = element.buildScope(),
       inCheckContext = compiler.enableTypeAssertions,
       inCatchBlock = false,
-      super(compiler);
+      super(compiler) {
+    typeResolver.context = this;
+  }
 
   ResolutionEnqueuer get world => compiler.enqueuer.resolution;
 
@@ -2403,10 +2419,9 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
   }
 
   DartType resolveTypeAnnotation(TypeAnnotation node) {
-    Function report = typeRequired ? error : warning;
-    DartType type = typeResolver.resolveTypeAnnotation(
-        node, scope, enclosingElement,
-        onFailure: report, whenResolved: useType);
+    DartType type = typeRequired ?
+        typeResolver.resolveTypeExpression(node) :
+        typeResolver.resolveTypeAnnotation(node);
     if (type == null) return null;
     if (inCheckContext) {
       compiler.enqueuer.resolution.registerIsCheck(type);
@@ -2746,16 +2761,24 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
   }
 }
 
-class TypeDefinitionVisitor extends CommonResolverVisitor<DartType> {
+class TypeDefinitionVisitor extends CommonResolverVisitor<DartType>
+    implements TypeResolverContext {
   Scope scope;
-  TypeDeclarationElement element;
+  final TypeDeclarationElement element;
   TypeResolver typeResolver;
+  Element get enclosingElement => element;
 
   TypeDefinitionVisitor(Compiler compiler, TypeDeclarationElement element)
       : this.element = element,
         scope = Scope.buildEnclosingScope(element),
         typeResolver = new TypeResolver(compiler),
-        super(compiler);
+        super(compiler) {
+    typeResolver.context = this;
+  }
+
+  void useType(Node node, DartType type) {
+    // Do not register used types.
+  }
 
   void resolveTypeVariableBounds(NodeList node) {
     if (node == null) return;
@@ -2776,8 +2799,7 @@ class TypeDefinitionVisitor extends CommonResolverVisitor<DartType> {
 
       TypeVariableElement variableElement = typeVariable.element;
       if (typeNode.bound != null) {
-        DartType boundType = typeResolver.resolveTypeAnnotation(
-            typeNode.bound, scope, element, onFailure: warning);
+        DartType boundType = typeResolver.resolveTypeAnnotation(typeNode.bound);
         if (boundType != null && boundType.element == variableElement) {
           // TODO(johnniwinther): Check for more general cycles, like
           // [: <A extends B, B extends C, C extends B> :].
@@ -2854,7 +2876,7 @@ class ClassResolverVisitor extends TypeDefinitionVisitor {
     if (node.superclass != null) {
       MixinApplication superMixin = node.superclass.asMixinApplication();
       if (superMixin != null) {
-        DartType supertype = resolveSupertype(element, superMixin.superclass);
+        DartType supertype = resolveSupertype(superMixin.superclass);
         Link<Node> link = superMixin.mixins.nodes;
         while (!link.isEmpty) {
           supertype = applyMixin(supertype, visit(link.head));
@@ -2862,7 +2884,7 @@ class ClassResolverVisitor extends TypeDefinitionVisitor {
         }
         element.supertype = supertype;
       } else {
-        element.supertype = resolveSupertype(element, node.superclass);
+        element.supertype = resolveSupertype(node.superclass);
       }
     }
 
@@ -2899,7 +2921,7 @@ class ClassResolverVisitor extends TypeDefinitionVisitor {
 
     // Generate anonymous mixin application elements for the
     // intermediate mixin applications (excluding the last).
-    DartType supertype = resolveSupertype(element, node.superclass);
+    DartType supertype = resolveSupertype(node.superclass);
     Link<Node> link = node.mixins.nodes;
     while (!link.tail.isEmpty) {
       supertype = applyMixin(supertype, visit(link.head));
@@ -3028,9 +3050,8 @@ class ClassResolverVisitor extends TypeDefinitionVisitor {
     return e.computeType(compiler);
   }
 
-  DartType resolveSupertype(ClassElement cls, TypeAnnotation superclass) {
-    DartType supertype = typeResolver.resolveTypeAnnotation(
-        superclass, scope, cls, onFailure: error);
+  DartType resolveSupertype(TypeAnnotation superclass) {
+    DartType supertype = typeResolver.resolveTypeExpression(superclass);
     if (supertype != null) {
       if (identical(supertype.kind, TypeKind.MALFORMED_TYPE)) {
         // Error has already been reported.
@@ -3050,16 +3071,14 @@ class ClassResolverVisitor extends TypeDefinitionVisitor {
   Link<DartType> resolveInterfaces(NodeList interfaces, Node superclass) {
     Link<DartType> result = const Link<DartType>();
     if (interfaces == null) return result;
-    for (Link<Node> link = interfaces.nodes; !link.isEmpty; link = link.tail) {
-      DartType interfaceType = typeResolver.resolveTypeAnnotation(
-          link.head, scope, element, onFailure: error);
+    for (TypeAnnotation interface in interfaces.nodes){

[ahe, 2013/02/05 11:26:29]

Please avoid for-in.

+      DartType interfaceType = typeResolver.resolveTypeExpression(interface);
       if (interfaceType != null) {
         if (identical(interfaceType.kind, TypeKind.MALFORMED_TYPE)) {
           // Error has already been reported.
         } else if (!identical(interfaceType.kind, TypeKind.INTERFACE)) {
           // TODO(johnniwinther): Handle dynamic.
-          TypeAnnotation typeAnnotation = link.head;
-          error(typeAnnotation.typeName, MessageKind.CLASS_NAME_EXPECTED);
+          error(interface.typeName, MessageKind.CLASS_NAME_EXPECTED);
         } else {
           if (interfaceType == element.supertype) {
             compiler.reportErrorCode(
@@ -3067,19 +3086,19 @@ class ClassResolverVisitor extends TypeDefinitionVisitor {
                 MessageKind.DUPLICATE_EXTENDS_IMPLEMENTS,
                 {'type': interfaceType});
             compiler.reportErrorCode(
-                link.head,
+                interface,
                 MessageKind.DUPLICATE_EXTENDS_IMPLEMENTS,
                 {'type': interfaceType});
           }
           if (result.contains(interfaceType)) {
             compiler.reportErrorCode(
-                link.head,
+                interface,
                 MessageKind.DUPLICATE_IMPLEMENTS,
                 {'type': interfaceType});
           }
           result = result.prepend(interfaceType);
           if (isBlackListed(interfaceType)) {
-            error(link.head, MessageKind.CANNOT_IMPLEMENT,
+            error(interface, MessageKind.CANNOT_IMPLEMENT,
                   {'type': interfaceType});
           }
         }