Fixes for review comments.

pkg/analyzer/lib/src/dart/analysis/driver.dart

 // Copyright (c) 2016, 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.
 
 import 'dart:async';
 import 'dart:collection';
 import 'dart:convert';
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ skipping 95 matching lines @@
   /**
    * The combined unlinked and linked package for the SDK, extracted from
    * the given [_sourceFactory].
    */
   PackageBundle _sdkBundle;
 
   /**
    * The mapping from the files for which analysis was requested using
    * [getResult] to the [Completer]s to report the result.
    */
-  final _requestedFiles = <String, Completer<AnalysisResult>>{};
+  final _requestedFiles = <String, List<Completer<AnalysisResult>>>{};
 
   /**
    * The set of explicitly analyzed files.
    */
   final _explicitFiles = new LinkedHashSet<String>();
 
   /**
    * The set of files that are currently scheduled for analysis.
    */
   final _filesToAnalyze = new LinkedHashSet<String>();
 
   /**
    * The mapping of [Uri]s to the mapping of textual URIs to the [Source]
    * that correspond in the current [_sourceFactory].
    */
   final _uriResolutionCache = <Uri, Map<String, Source>>{};
 
   /**
    * The current file state.
    *
    * It maps file paths to the MD5 hash of the file content.
    */
   final _fileContentHashMap = <String, String>{};
 
   /**
-   * Mapping from library URIs to the linked hash of the library.
+   * Mapping from library URIs to the dependency signature of the library.
    */
-  final _linkedHashMap = <Uri, String>{};
+  final _dependencySignatureMap = <Uri, String>{};
 
   /**
    * TODO(scheglov) document and improve
    */
   final _hasWorkStreamController = new StreamController<String>();
 
   AnalysisDriver(this._logger, this._resourceProvider, this._byteStore,
       this._contentCache, this._sourceFactory, this._analysisOptions) {
     _sdkBundle = _sourceFactory.dartSdk.getLinkedBundle();
   }
@@ skipping 30 matching lines @@
    * Results might be produced even for files that have never been added
    * using [addFile], for example when [getResult] was called for a file.
    */
   Stream<AnalysisResult> get results async* {
     try {
       while (true) {
         // TODO(scheglov) implement state transitioning
         await for (String why in _hasWorkStreamController.stream) {
           // Analyze the first file in the general queue.
           if (_filesToAnalyze.isNotEmpty) {
-            _logger.runTimed('Analyzed ${_filesToAnalyze.length} files', () {
+            _logger.run('Analyze ${_filesToAnalyze.length} files', () {
               while (_filesToAnalyze.isNotEmpty) {
                 String path = _filesToAnalyze.first;
                 _filesToAnalyze.remove(path);
                 _File file = _fileForPath(path);
                 _computeAndPrintErrors(file);
                 // TODO(scheglov) yield the result
               }
             });
           }
         }
@@ skipping 31 matching lines @@
    * transitions to "idle".
    *
    * Invocation of this method will not prevent a [Future] returned from
    * [getResult] from completing with a result, but the result is not
    * guaranteed to be consistent with the new current file state after this
    * [changeFile] invocation.
    */
   void changeFile(String path) {
     // TODO(scheglov) Don't clear, schedule API signature validation.
     _fileContentHashMap.clear();
-    _linkedHashMap.clear();
+    _dependencySignatureMap.clear();
     _filesToAnalyze.add(path);
     _filesToAnalyze.addAll(_explicitFiles);
     // TODO(scheglov) name?!
     _hasWorkStreamController.add('do it!');
   }
 
   /**
    * Return the [Future] that completes with a [AnalysisResult] for the file
    * with the given [path].
    *
    * The [path] must be absolute and normalized.
    *
    * The [path] can be any file - explicitly or implicitly analyzed, or neither.
    *
    * Causes the analysis state to transition to "analyzing" (if it is not in
    * that state already), the driver will read the file and produce the analysis
    * result for it, which is consistent with the current file state (including
    * the new state of the file), prior to the next time the analysis state
    * transitions to "idle".
    */
   Future<AnalysisResult> getResult(String path) {
     var completer = new Completer<AnalysisResult>();
-    _requestedFiles[path] = completer;
+    _requestedFiles
+        .putIfAbsent(path, () => <Completer<AnalysisResult>>[])
+        .add(completer);
+    _hasWorkStreamController.add(path);
     return completer.future;
   }
 
   /**
    * Remove the file with the given [path] from the list of files to analyze.
    *
    * The [path] must be absolute and normalized.
    *
    * The results of analysis of the file might still be produced by the
    * [results] stream. The driver will try to stop producing these results,
@@ skipping 21 matching lines @@
         file.path.endsWith('pkg/analyzer/lib/src/generated/error.dart') ||
         file.path.endsWith('pkg/analyzer/lib/src/generated/scanner.dart') ||
         file.path.endsWith('pkg/analyzer/lib/src/generated/sdk_io.dart') ||
         file.path.endsWith('pkg/analyzer/lib/src/generated/visitors.dart') ||
         file.path.endsWith('pkg/analyzer/test/generated/constant_test.dart') ||
         file.path.endsWith('pkg/analyzer/test/source/embedder_test.dart')) {
       return [];
     }
 
     List<String> errorStrings = _logger.run('Compute errors $file', () {
-      LibraryContext libraryContext = _createLibraryContext(file);
+      _LibraryContext libraryContext = _createLibraryContext(file);
 
       String errorsKey;
       {
         ApiSignature signature = new ApiSignature();
-        signature.addString(libraryContext.node.linkedHash);
+        signature.addString(libraryContext.node.dependencySignature);
         signature.addString(file.contentHash);
         errorsKey = '${signature.toHex()}.errors';
       }
 
       {
         List<int> bytes = _byteStore.get(errorsKey);
         if (bytes != null) {
           fb.BufferContext bp = new fb.BufferContext.fromBytes(bytes);
           int table = bp.derefObject(0);
           return const fb.ListReader<String>(const fb.StringReader())
               .vTableGet(bp, table, 0);
         }
       }
 
       AnalysisContext analysisContext = _createAnalysisContext(libraryContext);
       analysisContext.setContents(file.source, file.content);
       try {
         // Compute resolved unit.
 //        _logger.runTimed('Computed resolved unit', () {
 //          analysisContext.resolveCompilationUnit2(
 //              libraryContext.file.source, libraryContext.file.source);
 //        });
         // Compute errors.
         List<AnalysisError> errors;
         try {
-          errors = _logger.runTimed('Computed errors', () {
+          errors = _logger.run('Compute errors', () {
             return analysisContext.computeErrors(file.source);
           });
         } catch (e, st) {
           // TODO(scheglov) why does it fail?
           // Caused by Bad state: Unmatched TypeParameterElementImpl T
           errors = [];
         }
         List<String> errorStrings = errors
             .where((error) => error.errorCode is! TodoCode)
             .map((error) => error.toString())
@@ skipping 17 matching lines @@
     });
 
     if (errorStrings.isNotEmpty) {
       errorStrings.forEach((errorString) => print('\t$errorString'));
     } else {
       print('\tNO ERRORS');
     }
     return errorStrings;
   }
 
-  AnalysisContext _createAnalysisContext(LibraryContext libraryContext) {
+  AnalysisContext _createAnalysisContext(_LibraryContext libraryContext) {
     AnalysisContextImpl analysisContext =
         AnalysisEngine.instance.createAnalysisContext();
 
     analysisContext.sourceFactory =
         new SourceFactory((_sourceFactory as SourceFactoryImpl).resolvers);
     analysisContext.resultProvider =
         new InputPackagesResultProvider(analysisContext, libraryContext.store);
     analysisContext
         .applyChanges(new ChangeSet()..addedSource(libraryContext.file.source));
     return analysisContext;
   }
 
   /**
-   * Return the content in which the library represented by the given
+   * Return the context in which the library represented by the given
    * [libraryFile] should be analyzed it.
    *
-   * TODO(scheglov) We often don't need [SummaryDataStore], only linked hash.
+   * TODO(scheglov) We often don't need [SummaryDataStore], only dependency
+   * signature.
    */
-  LibraryContext _createLibraryContext(_File libraryFile) {
-    Map<String, _LibraryNode> nodes = <String, _LibraryNode>{};
-
+  _LibraryContext _createLibraryContext(_File libraryFile) {
     return _logger.run('Create library context', () {
+      Map<String, _LibraryNode> nodes = <String, _LibraryNode>{};
       SummaryDataStore store = new SummaryDataStore(const <String>[]);
       store.addBundle(null, _sdkBundle);
 
-      void createLibraryNodes(_File libraryFile) {
+      _LibraryNode createLibraryNodes(_File libraryFile) {
         Uri libraryUri = libraryFile.uri;
+
+        // URIs with the 'dart:' scheme are served from the SDK bundle.
         if (libraryUri.scheme == 'dart') {
-          return;
+          return null;
         }
-        String uriStr = libraryUri.toString();
-        if (!nodes.containsKey(uriStr)) {
-          _LibraryNode node = new _LibraryNode(this, nodes, libraryUri);
-          nodes[uriStr] = node;
-          ReferencedUris referenced = _getReferencedUris(libraryFile);
+
+        String libraryUriStr = libraryUri.toString();
+        _LibraryNode node = nodes[libraryUriStr];
+        if (node == null) {
+          node = new _LibraryNode(this, nodes, libraryUri);
+          nodes[libraryUriStr] = node;
+          _ReferencedUris referenced = _getReferencedUris(libraryFile);
 
           // Append unlinked bundles.
           for (String uri in referenced.parted) {
             _File file = libraryFile.resolveUri(uri);
             PackageBundle unlinked = _getUnlinked(file);
             node.unlinkedBundles.add(unlinked);
             store.addBundle(null, unlinked);
           }
 
           // Create nodes for referenced libraries.
           for (String uri in referenced.imported) {
             _File file = libraryFile.resolveUri(uri);
             createLibraryNodes(file);
           }
           for (String uri in referenced.exported) {
             _File file = libraryFile.resolveUri(uri);
             createLibraryNodes(file);
           }
         }
+
+        // Done with this node.
+        return node;
       }
 
-      _logger.runTimed2(() {
-        createLibraryNodes(libraryFile);
-      }, () => 'Computed ${nodes.length} nodes');
-      _LibraryNode libraryNode = nodes[libraryFile.uri.toString()];
+      _LibraryNode libraryNode = _logger.run('Compute library nodes', () {
+        return createLibraryNodes(libraryFile);
+      });
 
       Set<String> libraryUrisToLink = new Set<String>();
-      int numberOfNodesWithLinked = 0;
-      _logger.runTimed2(() {
+      _logger.run('Load linked bundles', () {
         for (_LibraryNode node in nodes.values) {
-          String key = '${node.linkedHash}.linked';
+          String key = '${node.dependencySignature}.linked';
           List<int> bytes = _byteStore.get(key);
           if (bytes != null) {
             PackageBundle linked = new PackageBundle.fromBuffer(bytes);
             store.addBundle(null, linked);
-            numberOfNodesWithLinked++;
           } else {
             libraryUrisToLink.add(node.uri.toString());
           }
         }
-      }, () => 'Loaded $numberOfNodesWithLinked linked bundles');
+        int numOfLoaded = nodes.length - libraryUrisToLink.length;
+        _logger.writeln('Loaded $numOfLoaded linked bundles.');
+      });
 
       Map<String, LinkedLibraryBuilder> linkedLibraries = {};
-      _logger.runTimed2(() {
+      _logger.run('Link bundles', () {
         linkedLibraries = link(libraryUrisToLink, (String uri) {
           LinkedLibrary linkedLibrary = store.linkedMap[uri];
           if (linkedLibrary == null) {
             throw new StateError('No linked library for: $uri');
           }
           return linkedLibrary;
         }, (String uri) {
           UnlinkedUnit unlinkedUnit = store.unlinkedMap[uri];
           if (unlinkedUnit == null) {
             throw new StateError('No unlinked unit for: $uri');
           }
           return unlinkedUnit;
         }, (_) => null, _analysisOptions.strongMode);
-      }, () => 'Linked ${linkedLibraries.length} bundles');
+        _logger.writeln('Linked ${linkedLibraries.length} bundles.');
+      });
 
       linkedLibraries.forEach((uri, linkedBuilder) {
         _LibraryNode node = nodes[uri];
-        String key = '${node.linkedHash}.linked';
+        String key = '${node.dependencySignature}.linked';
         List<int> bytes;
         {
           PackageBundleAssembler assembler = new PackageBundleAssembler();
           assembler.addLinkedLibrary(uri, linkedBuilder);
           bytes = assembler.assemble().toBuffer();
         }
         PackageBundle linked = new PackageBundle.fromBuffer(bytes);
         store.addBundle(null, linked);
         _byteStore.put(key, bytes);
       });
 
-      return new LibraryContext(libraryFile, libraryNode, store);
+      return new _LibraryContext(libraryFile, libraryNode, store);
     });
   }
 
   /**
    * Return the [_File] for the given [path] in [_sourceFactory].
    */
   _File _fileForPath(String path) {
     Source fileSource = _resourceProvider.getFile(path).createSource();
     Uri uri = _sourceFactory.restoreUri(fileSource);
     Source source = _resourceProvider.getFile(path).createSource(uri);
     return new _File(this, source);
   }
 
   /**
    * TODO(scheglov) It would be nice to get URIs of "parts" from unlinked.
    */
-  ReferencedUris _getReferencedUris(_File file) {
+  _ReferencedUris _getReferencedUris(_File file) {
     // Try to get from the store.
     {
       String key = '${file.contentHash}.uris';
       List<int> bytes = _byteStore.get(key);
       if (bytes != null) {
         fb.BufferContext bp = new fb.BufferContext.fromBytes(bytes);
         int table = bp.derefObject(0);
         const fb.ListReader<String> stringListReader =
             const fb.ListReader<String>(const fb.StringReader());
         bool isLibrary = const fb.BoolReader().vTableGet(bp, table, 0);
         List<String> imported = stringListReader.vTableGet(bp, table, 1);
         List<String> exported = stringListReader.vTableGet(bp, table, 2);
         List<String> parted = stringListReader.vTableGet(bp, table, 3);
-        ReferencedUris referencedUris = new ReferencedUris();
+        _ReferencedUris referencedUris = new _ReferencedUris();
         referencedUris.isLibrary = isLibrary;
         referencedUris.imported.addAll(imported);
         referencedUris.exported.addAll(exported);
         referencedUris.parted.addAll(parted);
         return referencedUris;
       }
     }
 
     // Compute URIs.
-    ReferencedUris referencedUris = new ReferencedUris();
+    _ReferencedUris referencedUris = new _ReferencedUris();
     referencedUris.parted.add(file.uri.toString());
     for (Directive directive in file.unit.directives) {
       if (directive is PartOfDirective) {
         referencedUris.isLibrary = false;
       } else if (directive is UriBasedDirective) {
         String uri = directive.uri.stringValue;
         if (directive is ImportDirective) {
           referencedUris.imported.add(uri);
         } else if (directive is ExportDirective) {
           referencedUris.exported.add(uri);
@@ skipping 45 matching lines @@
    */
   PackageBundle _getUnlinked(_File file) {
     // Try to get bytes for file's unlinked bundle.
     List<int> bytes;
     {
       String key = '${file.contentHash}.unlinked';
       bytes = _byteStore.get(key);
     }
     // If no cached unlinked bundle, compute it.
     if (bytes == null) {
-      _logger.runTimed('Create unlinked for $file', () {
+      _logger.run('Create unlinked for $file', () {
         // We read the content and recomputed the hash.
         // So, we need to update the key.
         String key = '${file.contentHash}.unlinked';
         UnlinkedUnitBuilder unlinkedUnit = serializeAstUnlinked(file.unit);
         PackageBundleAssembler assembler = new PackageBundleAssembler();
         assembler.addUnlinkedUnitWithHash(
             file.uri.toString(), unlinkedUnit, key);
         bytes = assembler.assemble().toBuffer();
         _byteStore.put(key, bytes);
       });
@@ skipping 43 matching lines @@
 
   /**
    * The full list of computed analysis errors, both syntactic and semantic.
    */
   final List<AnalysisError> errors;
 
   AnalysisResult(this.path, this.uri, this.content, this.contentHash, this.unit,
       this.errors);
 }
 
-class LibraryContext {
-  final _File file;
-  final _LibraryNode node;
-  final SummaryDataStore store;
-  LibraryContext(this.file, this.node, this.store);
-}
-
+/**
+ * This class is used to gather and print performance information.
+ */
 class PerformanceLog {
   final StringSink sink;
   int _level = 0;
 
   PerformanceLog(this.sink);
 
+  /**
+   * Return the result of the function [f] invocation and log the elapsed time.
+   *
+   * Each invocation of [run] creates a new enclosed section in the log,
+   * which begins with printing [msg], then any log output produced during
+   * [f] invocation, and ends with printing [msg] with the elapsed time.
+   */
   /*=T*/ run/*<T>*/(String msg, /*=T*/ f()) {
     Stopwatch timer = new Stopwatch()..start();
     try {
       writeln('+++ $msg.');
       _level++;
       return f();
     } finally {
       _level--;
       int ms = timer.elapsedMilliseconds;
       writeln('--- $msg in $ms ms.');
     }
   }
 
-  /*=T*/ runTimed/*<T>*/(String msg, /*=T*/ f()) {
-    _level++;
-    Stopwatch timer = new Stopwatch()..start();
-    try {
-      return f();
-    } finally {
-      _level--;
-      int ms = timer.elapsedMilliseconds;
-      writeln('$msg in $ms ms.');
-    }
-  }
-
-  runTimed2(f(), String getMsg()) {
-    _level++;
-    Stopwatch timer = new Stopwatch()..start();
-    try {
-      return f();
-    } finally {
-      _level--;
-      int ms = timer.elapsedMilliseconds;
-      String msg = getMsg();
-      writeln('$msg in $ms ms.');
-    }
-  }
-
+  /**
+   * Write a new line into the log
+   */
   void writeln(String msg) {
     String indent = '\t' * _level;
     sink.writeln('$indent$msg');
   }
 }
 
-class ReferencedUris {
-  bool isLibrary = true;
-  final List<String> imported = <String>[];
-  final List<String> exported = <String>[];
-  final List<String> parted = <String>[];
-}
-
 /**
  * Information about a file being analyzed, explicitly or implicitly.
  *
  * It keeps a consistent view on its [content], [contentHash] and [unit].
+ *
+ * Instances of this class may only be used during computing a single analysis
+ * result and should not be cached anywhere. We need this limitation to prevent
+ * references from caches to the resolved [unit], so to element models, etc.
+ * The data structures should be short lived - computed, returned to the client,
+ * processed there and quickly thrown away.
  */
 class _File {
   /**
    * The driver instance that is used to access [SourceFactory] and caches.
    */
   final AnalysisDriver driver;
 
   /**
    * The [Source] this [_File] instance represent.
    */
@@ skipping 22 matching lines @@
     return _content;
   }
 
   /**
    * Ensure that the [contentHash] is filled.
    *
    * If the hash is already in the current file state, return the current
    * value. Otherwise, read the [content], compute the hash, put it into
    * the current file state, and update the [contentHash] field.
    *
-   * The client cannot remember values of this property, because its value
+   * The client should not remember values of this property, because its value
    * might change when [content] is read and the hash is recomputed.
    */
   String get contentHash {
     _contentHash ??= driver._fileContentHashMap[path];
     if (_contentHash == null) {
       _readContentAndComputeHash();
     }
     return _contentHash;
   }
 
   String get path => source.fullName;
 
   /**
-   * Return the [CompilationUnit] of the file.
+   * Return the unresolved [CompilationUnit] of the file.
    *
-   * Current this unit is resolved, it is used to compute unlinked summaries
-   * and and URIs. We use a separate analysis context to perform resolution
-   * and computing errors. But this might change in the future.
+   * Performing resolution and computing errors is done in a separate analysis
+   * context. In the future we might push the existing unresolved unit into the
+   * analysis context, so at some point the unit might become resolved.
    */
   CompilationUnit get unit {
     AnalysisErrorListener errorListener = AnalysisErrorListener.NULL_LISTENER;
 
     CharSequenceReader reader = new CharSequenceReader(content);
     Scanner scanner = new Scanner(source, reader, errorListener);
     scanner.scanGenericMethodComments = driver._analysisOptions.strongMode;
     Token token = scanner.tokenize();
     LineInfo lineInfo = new LineInfo(scanner.lineStarts);
 
@@ skipping 16 matching lines @@
         .putIfAbsent(uri, () => driver._sourceFactory.resolveUri(source, uri));
     return new _File(driver, uriSource);
   }
 
   @override
   String toString() => uri.toString();
 
   /**
    * Fill the [_content] and [_contentHash] fields.
    *
-   * If the [_content] field if it is still `null`, get the content from the
+   * If the [_content] field is still `null`, get the content from the
    * content cache or from the [source]. If the content cannot be accessed
    * because of an exception, it considers to be an empty string.
    *
    * When a new content is read, the new [_contentHash] should be computed and
    * the current file state should be updated.
    */
   void _readContentAndComputeHash() {
     try {
       _content = driver._contentCache.getContents(source);
       _content ??= source.contents.data;
     } catch (_) {
+      // TODO(scheglov) Fix the bug with not existing sources.
+      // We should not put "self URI" into cached _ReferencedUris.
+      // Otherwise such not-existing/empty sources all have the same hash,
+      // but their "self URIs" must be all different.
       _content = '';
     }
     // Compute the content hash.
     List<int> textBytes = UTF8.encode(_content);
     List<int> hashBytes = md5.convert(textBytes).bytes;
     _contentHash = hex.encode(hashBytes);
     // Update the current file state.
     driver._fileContentHashMap[path] = _contentHash;
   }
 }
 
+/**
+ * TODO(scheglov) document
+ */
+class _LibraryContext {
+  final _File file;
+  final _LibraryNode node;
+  final SummaryDataStore store;
+  _LibraryContext(this.file, this.node, this.store);
+}
+
 class _LibraryNode {
   final AnalysisDriver driver;
   final Map<String, _LibraryNode> nodes;
   final Uri uri;
   final List<PackageBundle> unlinkedBundles = <PackageBundle>[];
 
   Set<_LibraryNode> transitiveDependencies;
   List<_LibraryNode> _dependencies;
-  String _linkedHash;
+  String _dependencySignature;
 
   _LibraryNode(this.driver, this.nodes, this.uri);
 
   /**
    * Retrieve the dependencies of this node.
    */
   List<_LibraryNode> get dependencies {
     if (_dependencies == null) {
       Set<_LibraryNode> dependencies = new Set<_LibraryNode>();
 
@@ skipping 26 matching lines @@
             appendDependency(export.uri);
           }
         }
       }
 
       _dependencies = dependencies.toList();
     }
     return _dependencies;
   }
 
-  @override
-  int get hashCode => uri.hashCode;
-
-  String get linkedHash {
-    _linkedHash ??= driver._linkedHashMap.putIfAbsent(uri, () {
+  String get dependencySignature {
+    return _dependencySignature ??=
+        driver._dependencySignatureMap.putIfAbsent(uri, () {
       computeTransitiveDependencies();
 
       // Add all unlinked API signatures.
       List<String> signatures = <String>[];
       signatures.add(driver._sdkBundle.apiSignature);
       transitiveDependencies
           .map((node) => node.unlinkedBundles)
           .expand((bundles) => bundles)
           .map((bundle) => bundle.apiSignature)
           .forEach(signatures.add);
       signatures.sort();
 
       // Combine into a single hash.
       ApiSignature signature = new ApiSignature();
       signature.addString(uri.toString());
       signatures.forEach(signature.addString);
       return signature.toHex();
     });
-    return _linkedHash;
   }
 
+  @override
+  int get hashCode => uri.hashCode;
+
   bool operator ==(other) {
     return other is _LibraryNode && other.uri == uri;
   }
 
   void computeTransitiveDependencies() {
     if (transitiveDependencies == null) {
       transitiveDependencies = new Set<_LibraryNode>();
 
       void appendDependencies(_LibraryNode node) {
         if (transitiveDependencies.add(node)) {
           node.dependencies.forEach(appendDependencies);
         }
       }
 
       appendDependencies(this);
     }
   }
 
   @override
   String toString() => uri.toString();
 }
+
+/**
+ * TODO(scheglov) document
+ */
+class _ReferencedUris {
+  bool isLibrary = true;
+  final List<String> imported = <String>[];
+  final List<String> exported = <String>[];
+  final List<String> parted = <String>[];
+}