Fixes to associating existing elements with an AST

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

Index: pkg/analyzer/lib/src/generated/resolver.dart
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index fd228934ec2b9611519b69df16b1de4136fa7047..97f5ef8b842bcd79a004bd003d73f2abb1c775bb 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -36,6 +36,26 @@ import 'package:analyzer/src/task/strong/info.dart'
 export 'package:analyzer/src/generated/type_system.dart';
 
 /**
+ * Throw an [ElementMismatchException] to report that the element model and the
+ * AST do not match. The [message] will have the path to the given [node]
+ * appended to it.
+ */
+void _mismatch(String message, AstNode node) {
+  StringBuffer buffer = new StringBuffer();
+  buffer.writeln(message);
+  buffer.write('Path to root:');
+  String separator = ' ';
+  AstNode parent = node;
+  while (parent != null) {
+    buffer.write(separator);
+    buffer.write(parent.runtimeType.toString());
+    separator = ', ';
+    parent = parent.parent;
+  }
+  throw new ElementMismatchException(buffer.toString());
+}
+
+/**
  * Instances of the class `BestPracticesVerifier` traverse an AST structure looking for
  * violations of Dart best practices.
  */
@@ -2119,6 +2139,11 @@ class DeadCodeVerifier extends RecursiveAstVisitor<Object> {
 /**
  * A visitor that resolves declarations in an AST structure to already built
  * elements.
+ *
+ * The resulting AST must have everything resolved that would have been resolved
+ * by a [CompilationUnitBuilder] (that is, must be a valid [RESOLVED_UNIT1]).
+ * This class must not assume that the [CompilationUnitElement] passed to it is
+ * any more complete than a [COMPILATION_UNIT_ELEMENT].
  */
 class DeclarationResolver extends RecursiveAstVisitor<Object> {
   /**
@@ -2135,7 +2160,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
   /**
    * The compilation unit containing the AST nodes being visited.
    */
-  CompilationUnitElement _enclosingUnit;
+  CompilationUnitElementImpl _enclosingUnit;
 
   /**
    * The function type alias containing the AST nodes being visited, or `null`
@@ -2200,7 +2225,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       SimpleIdentifier className = node.name;
       _enclosingClass = _findIdentifier(_enclosingUnit.types, className);
       super.visitClassDeclaration(node);
-      _resolveMetadata(node.metadata, _enclosingClass);
+      _resolveMetadata(node, node.metadata, _enclosingClass);
       return null;
     } finally {
       _enclosingClass = outerClass;
@@ -2214,7 +2239,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       SimpleIdentifier className = node.name;
       _enclosingClass = _findIdentifier(_enclosingUnit.types, className);
       super.visitClassTypeAlias(node);
-      _resolveMetadata(node.metadata, _enclosingClass);
+      _resolveMetadata(node, node.metadata, _enclosingClass);
       return null;
     } finally {
       _enclosingClass = outerClass;
@@ -2244,7 +2269,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       _expectedElements.remove(_enclosingExecutable);
       node.element = _enclosingExecutable as ConstructorElement;
       super.visitConstructorDeclaration(node);
-      _resolveMetadata(node.metadata, _enclosingExecutable);
+      _resolveMetadata(node, node.metadata, _enclosingExecutable);
       return null;
     } finally {
       _enclosingExecutable = outerExecutable;
@@ -2257,7 +2282,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
     Element element =
         _findIdentifier(_enclosingExecutable.localVariables, variableName);
     super.visitDeclaredIdentifier(node);
-    _resolveMetadata(node.metadata, element);
+    _resolveMetadata(node, node.metadata, element);
     return null;
   }
 
@@ -2279,7 +2304,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
     try {
       _enclosingParameter = element;
       super.visitDefaultFormalParameter(node);
-      _resolveMetadata(node.metadata, element);
+      _resolveMetadata(node, node.metadata, element);
       return null;
     } finally {
       _enclosingParameter = outerParameter;
@@ -2295,30 +2320,22 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       _findIdentifier(constants, constant.name);
     }
     super.visitEnumDeclaration(node);
-    _resolveMetadata(node.metadata, enclosingEnum);
+    _resolveMetadata(node, node.metadata, enclosingEnum);
     return null;
   }
 
   @override
   Object visitExportDirective(ExportDirective node) {
-    String uri = _getStringValue(node.uri);
-    ExportElement exportElement;
-    if (uri != null) {
-      LibraryElement library = _enclosingUnit.library;
-      Source source = _enclosingUnit.context.sourceFactory
-          .resolveUri(_enclosingUnit.source, uri);
-      exportElement = _findExport(node, library.exports, source);
-      node.element = exportElement;
-    }
     super.visitExportDirective(node);
-    _resolveMetadata(node.metadata, exportElement);
+    _resolveAnnotations(
+        node, node.metadata, _enclosingUnit.getAnnotations(node.offset));
     return null;
   }
 
   @override
   Object visitFieldDeclaration(FieldDeclaration node) {
     super.visitFieldDeclaration(node);
-    _resolveMetadata(node.metadata, node.fields.variables[0].element);
+    _resolveMetadata(node, node.metadata, node.fields.variables[0].element);
     return null;
   }
 
@@ -2331,7 +2348,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       try {
         _enclosingParameter = element;
         super.visitFieldFormalParameter(node);
-        _resolveMetadata(node.metadata, element);
+        _resolveMetadata(node, node.metadata, element);
         return null;
       } finally {
         _enclosingParameter = outerParameter;
@@ -2380,7 +2397,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       }
       node.functionExpression.element = _enclosingExecutable;
       super.visitFunctionDeclaration(node);
-      _resolveMetadata(node.metadata, _enclosingExecutable);
+      _resolveMetadata(node, node.metadata, _enclosingExecutable);
       return null;
     } finally {
       _enclosingExecutable = outerExecutable;
@@ -2412,7 +2429,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       _enclosingAlias =
           _findIdentifier(_enclosingUnit.functionTypeAliases, aliasName);
       super.visitFunctionTypeAlias(node);
-      _resolveMetadata(node.metadata, _enclosingAlias);
+      _resolveMetadata(node, node.metadata, _enclosingAlias);
       return null;
     } finally {
       _enclosingAlias = outerAlias;
@@ -2428,7 +2445,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       try {
         _enclosingParameter = element;
         super.visitFunctionTypedFormalParameter(node);
-        _resolveMetadata(node.metadata, _enclosingParameter);
+        _resolveMetadata(node, node.metadata, _enclosingParameter);
         return null;
       } finally {
         _enclosingParameter = outerParameter;
@@ -2440,17 +2457,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitImportDirective(ImportDirective node) {
-    String uri = _getStringValue(node.uri);
-    ImportElement importElement;
-    if (uri != null) {
-      LibraryElement library = _enclosingUnit.library;
-      Source source = _enclosingUnit.context.sourceFactory
-          .resolveUri(_enclosingUnit.source, uri);
-      importElement = _findImport(node, library.imports, source);
-      node.element = importElement;
-    }
     super.visitImportDirective(node);
-    _resolveMetadata(node.metadata, importElement);
+    _resolveAnnotations(
+        node, node.metadata, _enclosingUnit.getAnnotations(node.offset));
     return null;
   }
 
@@ -2465,10 +2474,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitLibraryDirective(LibraryDirective node) {
-    LibraryElement libraryElement = _enclosingUnit.library;
-    node.element = libraryElement;
     super.visitLibraryDirective(node);
-    _resolveMetadata(node.metadata, libraryElement);
+    _resolveAnnotations(
+        node, node.metadata, _enclosingUnit.getAnnotations(node.offset));
     return null;
   }
 
@@ -2500,7 +2508,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
         _enclosingExecutable = accessor;
       }
       super.visitMethodDeclaration(node);
-      _resolveMetadata(node.metadata, _enclosingExecutable);
+      _resolveMetadata(node, node.metadata, _enclosingExecutable);
       return null;
     } finally {
       _enclosingExecutable = outerExecutable;
@@ -2509,16 +2517,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitPartDirective(PartDirective node) {
-    String uri = _getStringValue(node.uri);
-    CompilationUnitElement compilationUnitElement;
-    if (uri != null) {
-      Source partSource = _enclosingUnit.context.sourceFactory
-          .resolveUri(_enclosingUnit.source, uri);
-      compilationUnitElement =
-          _findPart(_enclosingUnit.library.parts, node, partSource);
-    }
     super.visitPartDirective(node);
-    _resolveMetadata(node.metadata, compilationUnitElement);
+    _resolveAnnotations(
+        node, node.metadata, _enclosingUnit.getAnnotations(node.offset));
     return null;
   }
 
@@ -2537,7 +2538,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       try {
         _enclosingParameter = element;
         super.visitSimpleFormalParameter(node);
-        _resolveMetadata(node.metadata, element);
+        _resolveMetadata(node, node.metadata, element);
         return null;
       } finally {
         _enclosingParameter = outerParameter;
@@ -2567,7 +2568,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
   @override
   Object visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
     super.visitTopLevelVariableDeclaration(node);
-    _resolveMetadata(node.metadata, node.variables.variables[0].element);
+    _resolveMetadata(node, node.metadata, node.variables.variables[0].element);
     return null;
   }
 
@@ -2596,7 +2597,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
           'Could not find type parameter with name "$name" at $offset', node);
     }
     super.visitTypeParameter(node);
-    _resolveMetadata(node.metadata, element);
+    _resolveMetadata(node, node.metadata, element);
     return null;
   }
 
@@ -2634,7 +2635,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
     super.visitVariableDeclarationList(node);
     if (node.parent is! FieldDeclaration &&
         node.parent is! TopLevelVariableDeclaration) {
-      _resolveMetadata(node.metadata, node.variables[0].element);
+      _resolveMetadata(node, node.metadata, node.variables[0].element);
     }
     return null;
   }
@@ -2651,25 +2652,6 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       _findWithNameAndOffset(elements, node, '', offset);
 
   /**
-   * Return the export element from the given list of [exports] whose library
-   * has the given [source]. Throw an [ElementMismatchException] if an element
-   * corresponding to the identifier cannot be found.
-   */
-  ExportElement _findExport(
-      ExportDirective node, List<ExportElement> exports, Source source) {
-    if (source == null || !_context.exists(source)) {
-      return null;
-    }
-    for (ExportElement export in exports) {
-      if (export.exportedLibrary.source == source) {
-        return export;
-      }
-    }
-    _mismatch("Could not find export element for '$source'", node);
-    return null; // Never reached
-  }
-
-  /**
    * Return the element in the given list of [elements] that was created for the
    * declaration with the given [identifier]. As a side-effect, associate the
    * returned element with the identifier. Throw an [ElementMismatchException]
@@ -2687,64 +2669,6 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
   }
 
   /**
-   * Return the import element from the given list of [imports] whose library
-   * has the given [source]. Throw an [ElementMismatchException] if an element
-   * corresponding to the [source] cannot be found.
-   */
-  ImportElement _findImport(
-      ImportDirective node, List<ImportElement> imports, Source source) {
-    if (source == null || !_context.exists(source)) {
-      return null;
-    }
-    SimpleIdentifier prefix = node.prefix;
-    bool foundSource = false;
-    for (ImportElement element in imports) {
-      if (element.importedLibrary.source == source) {
-        foundSource = true;
-        PrefixElement prefixElement = element.prefix;
-        if (prefix == null) {
-          if (prefixElement == null) {
-            return element;
-          }
-        } else {
-          if (prefixElement != null &&
-              prefix.name == prefixElement.displayName) {
-            return element;
-          }
-        }
-      }
-    }
-    if (foundSource) {
-      if (prefix == null) {
-        _mismatch(
-            "Could not find import element for '$source' with no prefix", node);
-      }
-      _mismatch(
-          "Could not find import element for '$source' with prefix ${prefix.name}",
-          node);
-    }
-    _mismatch("Could not find import element for '$source'", node);
-    return null; // Never reached
-  }
-
-  /**
-   * Return the element in the given list of [parts] that was created for the
-   * part with the given [source]. Throw an [ElementMismatchException] if an
-   * element corresponding to the source cannot be found.
-   */
-  CompilationUnitElement _findPart(List<CompilationUnitElement> parts,
-      PartDirective directive, Source source) {
-    for (CompilationUnitElement part in parts) {
-      if (part.source == source) {
-        return part;
-      }
-    }
-    _mismatch(
-        'Could not find compilation unit element for "$source"', directive);
-    return null; // Never reached
-  }
-
-  /**
    * Return the element in the given list of [elements] that was created for the
    * declaration with the given [name] at the given [offset]. Throw an
    * [ElementMismatchException] if an element corresponding to the identifier
@@ -2811,51 +2735,36 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
   }
 
   /**
-   * Return the value of the given string [literal], or `null` if the string is
-   * not a constant string without any string interpolation.
+   * Associate each of the annotation [nodes] with the corresponding
+   * [ElementAnnotation] in [annotations]. If there is a problem, report it
+   * against the given [parent] node.
    */
-  String _getStringValue(StringLiteral literal) {
-    if (literal is StringInterpolation) {
-      return null;
+  void _resolveAnnotations(AstNode parent, NodeList<Annotation> nodes,
+      List<ElementAnnotation> annotations) {
+    int nodeCount = nodes.length;
+    if (nodeCount != annotations.length) {

[Paul Berry, 2016/03/04 22:02:11]

Nit: in the previous version of the code, this check is done in an assert(),
meaning that it only happens in checked mode (which in practice means, it only
happens during unit tests).  Now it happens in production as well.

If we weren't planning to try to cherry-pick this CL to the stable release
candidate, I would be ok changing the behavior.  But since we are contemplating
a cherry-pick, I think we should minimize behavioral changes as much as
possible.

[Brian Wilkerson, 2016/03/07 16:20:28]

I made the change intentionally so that we would catch bugs earlier because it
makes it much easier to fix them. Given that this code should never be triggered
anyway, I think it's a positive change for users, but if you feel strongly
enough about it I guess I can temporarily back it out.

+      _mismatch(
+          'Found $nodeCount annotation nodes and ${annotations.length} element annotations',

[Paul Berry, 2016/03/04 22:02:11]

Nit: this line is more than 80 columns.  I recommend splitting it into two
strings so that the formatter can squeeze it into 80 columns, e.g.:

_mismatch(
    'Found $nodeCount annotation nodes and ${annotations.length}'
    ' element annotations',
    parent);

[Brian Wilkerson, 2016/03/07 16:20:28]

Done

+          parent);
     }
-    return literal.stringValue;
-  }
-
-  /**
-   * Throw an [ElementMismatchException] to report that the element model and
-   * the AST do not match. The [message] will have the path to the given [node]
-   * appended to it.
-   */
-  void _mismatch(String message, AstNode node) {
-    StringBuffer buffer = new StringBuffer();
-    buffer.writeln(message);
-    buffer.write('Path to root:');
-    String separator = ' ';
-    AstNode parent = node;
-    while (parent != null) {
-      buffer.write(separator);
-      buffer.write(parent.runtimeType.toString());
-      separator = ', ';
-      parent = parent.parent;
+    for (int i = 0; i < nodeCount; i++) {
+      nodes[i].elementAnnotation = annotations[i];
     }
-    throw new ElementMismatchException(buffer.toString());
   }
 
   /**
-   * If [element] is not `null`, associate each [Annotation] in [astMetadata]
-   * with the corresponding [ElementAnnotation] in [element.metadata].
+   * If [element] is not `null`, associate each of the annotation [nodes] with
+   * the corresponding [ElementAnnotation] in [element.metadata]. If there is a
+   * problem, report it against the given [parent] node.
    *
    * If [element] is `null`, do nothing--this allows us to be robust in the
    * case where we are operating on an element model that hasn't been fully
    * built.
    */
-  void _resolveMetadata(NodeList<Annotation> astMetadata, Element element) {
+  void _resolveMetadata(
+      AstNode parent, NodeList<Annotation> nodes, Element element) {
     if (element != null) {
-      List<ElementAnnotation> elementMetadata = element.metadata;
-      assert(astMetadata.length == elementMetadata.length);
-      for (int i = 0; i < astMetadata.length; i++) {
-        astMetadata[i].elementAnnotation = elementMetadata[i];
-      }
+      _resolveAnnotations(parent, nodes, element.metadata);
     }
   }
 
@@ -2878,6 +2787,129 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 }
 
 /**
+ * A visitor that resolves directives in an AST structure to already built
+ * elements.
+ *
+ * The resulting AST must have everything resolved that would have been resolved
+ * by a [DirectiveElementBuilder].
+ */
+class DirectiveResolver extends SimpleAstVisitor {
+  CompilationUnitElementImpl _enclosingUnit;
+
+  @override
+  void visitCompilationUnit(CompilationUnit node) {
+    _enclosingUnit = node.element;
+    for (Directive directive in node.directives) {
+      directive.accept(this);
+    }
+  }
+
+  @override
+  void visitExportDirective(ExportDirective node) {
+    String uri = _getStringValue(node.uri);
+    if (uri != null) {
+      LibraryElement library = _enclosingUnit.library;
+      Source source = _enclosingUnit.context.sourceFactory
+          .resolveUri(_enclosingUnit.source, uri);
+      ExportElement exportElement = _findExport(node, library.exports, source);
+      node.element = exportElement;
+    } else {
+      node.element = null;
+    }
+  }
+
+  @override
+  void visitImportDirective(ImportDirective node) {
+    String uri = _getStringValue(node.uri);
+    if (uri != null) {
+      LibraryElement library = _enclosingUnit.library;
+      Source source = _enclosingUnit.context.sourceFactory
+          .resolveUri(_enclosingUnit.source, uri);
+      ImportElement importElement = _findImport(node, library.imports, source);
+      node.element = importElement;
+    } else {
+      node.element = null;
+    }
+  }
+
+  @override
+  void visitLibraryDirective(LibraryDirective node) {
+    node.element = _enclosingUnit.library;
+  }
+
+  /**
+   * Return the export element from the given list of [exports] whose library
+   * has the given [source]. Throw an [ElementMismatchException] if an element
+   * corresponding to the identifier cannot be found.
+   */
+  ExportElement _findExport(
+      ExportDirective node, List<ExportElement> exports, Source source) {
+    if (source == null || !_enclosingUnit.context.exists(source)) {
+      return null;
+    }
+    for (ExportElement export in exports) {
+      if (export.exportedLibrary.source == source) {
+        return export;
+      }
+    }
+    _mismatch("Could not find export element for '$source'", node);
+    return null; // Never reached
+  }
+
+  /**
+   * Return the import element from the given list of [imports] whose library
+   * has the given [source]. Throw an [ElementMismatchException] if an element
+   * corresponding to the [source] cannot be found.
+   */
+  ImportElement _findImport(
+      ImportDirective node, List<ImportElement> imports, Source source) {
+    if (source == null || !_enclosingUnit.context.exists(source)) {
+      return null;
+    }
+    SimpleIdentifier prefix = node.prefix;
+    bool foundSource = false;
+    for (ImportElement element in imports) {
+      if (element.importedLibrary.source == source) {
+        foundSource = true;
+        PrefixElement prefixElement = element.prefix;
+        if (prefix == null) {
+          if (prefixElement == null) {
+            return element;
+          }
+        } else {
+          if (prefixElement != null &&
+              prefix.name == prefixElement.displayName) {
+            return element;
+          }
+        }
+      }
+    }
+    if (foundSource) {
+      if (prefix == null) {
+        _mismatch(
+            "Could not find import element for '$source' with no prefix", node);
+      }
+      _mismatch(
+          "Could not find import element for '$source' with prefix ${prefix.name}",
+          node);
+    }
+    _mismatch("Could not find any import element for '$source'", node);
+    return null; // Never reached
+  }
+
+  /**
+   * Return the value of the given string [literal], or `null` if the string is
+   * not a constant string without any string interpolation.
+   */
+  String _getStringValue(StringLiteral literal) {
+    if (literal is StringInterpolation) {
+      return null;
+    }
+    return literal.stringValue;
+  }
+}
+
+/**
  * Instances of the class `ElementHolder` hold on to elements created while traversing an AST
  * structure so that they can be accessed when creating their enclosing element.
  */
@@ -3255,6 +3287,7 @@ class EnclosedScope extends Scope {
   /**
    * The scope in which this scope is lexically enclosed.
    */
+  @override
   final Scope enclosingScope;
 
   /**
@@ -6046,6 +6079,7 @@ class LibraryImportScope extends Scope {
   /**
    * The listener that is to be informed when an error is encountered.
    */
+  @override
   final AnalysisErrorListener errorListener;
 
   /**
@@ -7243,6 +7277,7 @@ class ResolverVisitor extends ScopedVisitor {
    * The class element representing the class containing the current node,
    * or `null` if the current node is not contained in a class.
    */
+  @override
   ClassElement enclosingClass = null;
 
   /**