Make creating LibraryContext async, so all its transitive clients.

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:typed_data';
 
 import 'package:analyzer/context/context_root.dart';
 import 'package:analyzer/context/declared_variables.dart';
@@ skipping 550 matching lines @@
    * be analyzed, the [Future] completes with `null`.
    *
    * The [path] must be absolute and normalized.
    *
    * This method does not use analysis priorities, and must not be used in
    * interactive analysis, such as Analysis Server or its plugins.
    */
   Future<ErrorsResult> getErrors(String path) async {
     // Ask the analysis result without unit, so return cached errors.
     // If no cached analysis result, it will be computed.
-    AnalysisResult analysisResult = _computeAnalysisResult(path);
+    AnalysisResult analysisResult = await _computeAnalysisResult(path);
+
+    // Check for asynchronous changes during computing the result.
+    _runTestAsyncWorkDuringAnalysis(path);

[Brian Wilkerson, 2017/08/25 19:44:12]

Missing await?

Also, could we document when we need to invoke `_runTestAsyncWorkDuringAnalysis`
after invoking `_computeAnalysisResult` and when we don't?

+    if (_fileTracker.hasChangedFiles) {
+      analysisResult = null;
+    }

[Brian Wilkerson, 2017/08/25 19:44:12]

And when do we need to check for changed files, and when not?

[scheglov, 2017/08/25 19:51:56]

I added comments to `_runTestAsyncWorkDuringAnalysis`.
It is used only for tests, so we don't have to call it everywhere, as long as we
are sure that there is no problem that we want to test.

 
     // If not computed yet, because a part file without a known library,
     // we have to compute the full analysis result, with the unit.
     analysisResult ??= await getResult(path);
     if (analysisResult == null) {
       return null;
     }
 
     return new ErrorsResult(currentSession, path, analysisResult.uri,
         analysisResult.lineInfo, analysisResult.errors);
@@ skipping 217 matching lines @@
   @override
   Future<Null> performWork() async {
     if (_fileTracker.verifyChangedFilesIfNeeded()) {
       return;
     }
 
     // Analyze a requested file.
     if (_requestedFiles.isNotEmpty) {
       String path = _requestedFiles.keys.first;
       try {
-        AnalysisResult result = _computeAnalysisResult(path, withUnit: true);
+        AnalysisResult result =
+            await _computeAnalysisResult(path, withUnit: true);
+        // Check for asynchronous changes during computing the result.
+        await _runTestAsyncWorkDuringAnalysis(path);
+        if (_fileTracker.hasChangedFiles) {
+          return;
+        }
         // If a part without a library, delay its analysis.
         if (result == null) {
           _requestedParts
               .putIfAbsent(path, () => [])
               .addAll(_requestedFiles.remove(path));
           return;
         }
         // Notify the completers.
         _requestedFiles.remove(path).forEach((completer) {
           completer.complete(result);
         });
         // Remove from to be analyzed and produce it now.
         _fileTracker.fileWasAnalyzed(path);
         _resultController.add(result);
       } catch (exception, stackTrace) {
         _fileTracker.fileWasAnalyzed(path);
         _requestedFiles.remove(path).forEach((completer) {
           completer.completeError(exception, stackTrace);
         });
       }
       return;
     }
 
     // Process an index request.
     if (_indexRequestedFiles.isNotEmpty) {
       String path = _indexRequestedFiles.keys.first;
-      AnalysisDriverUnitIndex index = _computeIndex(path);
+      AnalysisDriverUnitIndex index = await _computeIndex(path);
       _indexRequestedFiles.remove(path).forEach((completer) {
         completer.complete(index);
       });
       return;
     }
 
     // Process a unit element key request.
     if (_unitElementSignatureRequests.isNotEmpty) {
       String path = _unitElementSignatureRequests.keys.first;
       String signature = _computeUnitElementSignature(path);
       _unitElementSignatureRequests.remove(path).forEach((completer) {
         completer.complete(signature);
       });
       return;
     }
 
     // Process a unit element request.
     if (_unitElementRequestedFiles.isNotEmpty) {
       String path = _unitElementRequestedFiles.keys.first;
-      UnitElementResult result = _computeUnitElement(path);
+      UnitElementResult result = await _computeUnitElement(path);
       _unitElementRequestedFiles.remove(path).forEach((completer) {
         completer.complete(result);
       });
       return;
     }
 
     // Compute files defining a name.
     if (_definingClassMemberNameTasks.isNotEmpty) {
       _FilesDefiningClassMemberNameTask task =
           _definingClassMemberNameTasks.first;
@@ skipping 23 matching lines @@
       }
       return;
     }
 
     // Analyze a priority file.
     if (_priorityFiles.isNotEmpty) {
       for (String path in _priorityFiles) {
         if (_fileTracker.isFilePending(path)) {
           try {
             AnalysisResult result =
-                _computeAnalysisResult(path, withUnit: true);
+                await _computeAnalysisResult(path, withUnit: true);
+            await _runTestAsyncWorkDuringAnalysis(path);
             if (result == null) {
               _partsToAnalyze.add(path);
             } else {
               _resultController.add(result);
             }
           } catch (exception, stackTrace) {
             _reportException(path, exception, stackTrace);
           } finally {
             _fileTracker.fileWasAnalyzed(path);
           }
           return;
         }
       }
     }
 
     // Analyze a general file.
     if (_fileTracker.hasPendingFiles) {
       String path = _fileTracker.anyPendingFile;
       try {
-        AnalysisResult result = _computeAnalysisResult(path,
+        AnalysisResult result = await _computeAnalysisResult(path,
             withUnit: false, skipIfSameSignature: true);
+        await _runTestAsyncWorkDuringAnalysis(path);
         if (result == null) {
           _partsToAnalyze.add(path);
         } else if (result == AnalysisResult._UNCHANGED) {
           // We found that the set of errors is the same as we produced the
           // last time, so we don't need to produce it again now.
         } else {
           _resultController.add(result);
           _lastProducedSignatures[result.path] = result._signature;
         }
       } catch (exception, stackTrace) {
         _reportException(path, exception, stackTrace);
       } finally {
         _fileTracker.fileWasAnalyzed(path);
       }
       return;
     }
 
     // Analyze a requested part file.
     if (_requestedParts.isNotEmpty) {
       String path = _requestedParts.keys.first;
       try {
-        AnalysisResult result = _computeAnalysisResult(path,
+        AnalysisResult result = await _computeAnalysisResult(path,
             withUnit: true, asIsIfPartWithoutLibrary: true);
+        // Check for asynchronous changes during computing the result.
+        if (_fileTracker.hasChangedFiles) {
+          return;
+        }
         // Notify the completers.
         _requestedParts.remove(path).forEach((completer) {
           completer.complete(result);
         });
         // Remove from to be analyzed and produce it now.
         _partsToAnalyze.remove(path);
         _resultController.add(result);
       } catch (exception, stackTrace) {
         _partsToAnalyze.remove(path);
         _requestedParts.remove(path).forEach((completer) {
           completer.completeError(exception, stackTrace);
         });
       }
       return;
     }
 
     // Analyze a general part.
     if (_partsToAnalyze.isNotEmpty) {
       String path = _partsToAnalyze.first;
       _partsToAnalyze.remove(path);
       try {
-        AnalysisResult result = _computeAnalysisResult(path,
+        AnalysisResult result = await _computeAnalysisResult(path,
             withUnit: _priorityFiles.contains(path),
             asIsIfPartWithoutLibrary: true);
         _resultController.add(result);
       } catch (exception, stackTrace) {
         _reportException(path, exception, stackTrace);
       }
       return;
     }
   }
 
@@ skipping 37 matching lines @@
    * compute only if [withUnit] is `true`.
    *
    * Return `null` if the file is a part of an unknown library, so cannot be
    * analyzed yet. But [asIsIfPartWithoutLibrary] is `true`, then the file is
    * analyzed anyway, even without a library.
    *
    * Return [AnalysisResult._UNCHANGED] if [skipIfSameSignature] is `true` and
    * the resolved signature of the file in its library is the same as the one
    * that was the most recently produced to the client.
    */
-  AnalysisResult _computeAnalysisResult(String path,
+  Future<AnalysisResult> _computeAnalysisResult(String path,
       {bool withUnit: false,
       bool asIsIfPartWithoutLibrary: false,
-      bool skipIfSameSignature: false}) {
+      bool skipIfSameSignature: false}) async {
     FileState file = _fsState.getFileForPath(path);
 
     // Prepare the library - the file itself, or the known library.
     FileState library = file.isPart ? file.library : file;
     if (library == null) {
       if (asIsIfPartWithoutLibrary) {
         library = file;
       } else {
         return null;
       }
@@ skipping 15 matching lines @@
     if (!withUnit) {
       List<int> bytes = DriverPerformance.cache.makeCurrentWhile(() {
         return _byteStore.get(key);
       });
       if (bytes != null) {
         return _getAnalysisResultFromBytes(file, signature, bytes);
       }
     }
 
     // We need the fully resolved unit, or the result is not cached.
-    return _logger.run('Compute analysis result for $path', () {
+    return _logger.runAsync('Compute analysis result for $path', () async {
       try {
-        LibraryContext libraryContext = _createLibraryContext(library);
+        LibraryContext libraryContext = await _createLibraryContext(library);
         try {
           _testView.numOfAnalyzedLibraries++;
           LibraryAnalyzer analyzer = new LibraryAnalyzer(
               analysisOptions,
               declaredVariables,
               sourceFactory,
               _fsState,
               libraryContext.store,
               library);
           Map<FileState, UnitAnalysisResult> results = analyzer.analyze();
@@ skipping 33 matching lines @@
           libraryContext.dispose();
         }
       } catch (exception, stackTrace) {
         String contextKey =
             _storeExceptionContext(path, library, exception, stackTrace);
         throw new _ExceptionState(exception, stackTrace, contextKey);
       }
     });
   }
 
-  AnalysisDriverUnitIndex _computeIndex(String path) {
-    AnalysisResult analysisResult = _computeAnalysisResult(path,
+  Future<AnalysisDriverUnitIndex> _computeIndex(String path) async {
+    AnalysisResult analysisResult = await _computeAnalysisResult(path,
         withUnit: false, asIsIfPartWithoutLibrary: true);
     return analysisResult._index;
   }
 
-  UnitElementResult _computeUnitElement(String path) {
+  Future<UnitElementResult> _computeUnitElement(String path) async {
     FileState file = _fsState.getFileForPath(path);
     FileState library = file.library ?? file;
 
     // Create the AnalysisContext to resynthesize elements in.
-    LibraryContext libraryContext = _createLibraryContext(library);
+    LibraryContext libraryContext = await _createLibraryContext(library);
 
     // Resynthesize the CompilationUnitElement in the context.
     try {
       CompilationUnitElement element =
           libraryContext.computeUnitElement(library.source, file.source);
       String signature = library.transitiveSignature;
       return new UnitElementResult(
           currentSession, path, file.uri, signature, element);
     } finally {
       libraryContext.dispose();
@@ skipping 16 matching lines @@
     _fillSalt();
     _fsState = new FileSystemState(_logger, _byteStore, _contentOverlay,
         _resourceProvider, sourceFactory, analysisOptions, _salt,
         externalSummaries: _externalSummaries);
     _fileTracker = new FileTracker(_logger, _fsState, _changeHook);
   }
 
   /**
    * Return the context in which the [library] should be analyzed.
    */
-  LibraryContext _createLibraryContext(FileState library) {
+  Future<LibraryContext> _createLibraryContext(FileState library) async {
     _testView.numOfCreatedLibraryContexts++;
     return new LibraryContext.forSingleLibrary(
         library,
         _logger,
         _sdkBundle,
         _byteStore,
         _analysisOptions,
         declaredVariables,
         _sourceFactory,
         _externalSummaries,
@@ skipping 110 matching lines @@
       var state = exception as _ExceptionState;
       exception = state.exception;
       stackTrace = state.stackTrace;
       contextKey = state.contextKey;
     }
     CaughtException caught = new CaughtException(exception, stackTrace);
     _exceptionController.add(new ExceptionResult(path, caught, contextKey));
   }
 
   /**
+   * Tests need a reliable way to simulate work during analysis.
+   */
+  Future _runTestAsyncWorkDuringAnalysis(String path) {
+    var work = _testView.workToWaitAfterComputingResult;
+    _testView.workToWaitAfterComputingResult = null;
+    return work != null ? work(path) : null;

[Paul Berry, 2017/08/25 20:01:55]

I get scared when I see a method with a return type of Future that returns null.
 How about this instead?

return work != null ? work(path) : new Future.value();

[scheglov, 2017/08/25 20:30:46]

Done.

+  }
+
+  /**
    * Serialize the given [resolvedUnit] errors and index into bytes.
    */
   List<int> _serializeResolvedUnit(
       CompilationUnit resolvedUnit, List<AnalysisError> errors) {
     AnalysisDriverUnitIndexBuilder index = indexUnit(resolvedUnit);
     return new AnalysisDriverResolvedUnitBuilder(
             errors: errors
                 .map((error) => new AnalysisDriverUnitErrorBuilder(
                     offset: error.offset,
                     length: error.length,
@@ skipping 328 matching lines @@
 }
 
 @visibleForTesting
 class AnalysisDriverTestView {
   final AnalysisDriver driver;
 
   int numOfCreatedLibraryContexts = 0;
 
   int numOfAnalyzedLibraries = 0;
 
+  Future<Null> Function(String path) workToWaitAfterComputingResult;

[Paul Berry, 2017/08/25 20:01:55]

Would you mind adding a doc comment for this?  It's not obvious what its
behavior is from the name.

E.g. something like:

/// If non-null, a function that should be executed asynchronously after
/// the next result is computed.
///
/// This can be used by a test to simulate file changes during analysis.

[scheglov, 2017/08/25 20:30:46]

Done.

+
   AnalysisDriverTestView(this.driver);
 
   FileTracker get fileTracker => driver._fileTracker;
 
   Map<String, AnalysisResult> get priorityResults => driver._priorityResults;
 
-  SummaryDataStore getSummaryStore(String libraryPath) {
+  Future<SummaryDataStore> getSummaryStore(String libraryPath) async {
     FileState library = driver.fsState.getFileForPath(libraryPath);
-    LibraryContext libraryContext = driver._createLibraryContext(library);
+    LibraryContext libraryContext = await driver._createLibraryContext(library);
     try {
       return libraryContext.store;
     } finally {
       libraryContext.dispose();
     }
   }
 }
 
 /**
  * The result of analyzing of a single file.
@@ skipping 408 matching lines @@
           libraryDeclarations.add(new TopLevelDeclarationInSource(
               file.source, declaration, isExported));
         }
       }
     }
 
     // We're not done yet.
     return false;
   }
 }