Remove computeNode and use new analyzer interfaces for library retrieval (#1851)

* asynchronous PackageGraph construction and precaching

* checkpoint

* cleanup

* checkpoint

* Run tools more asynchronously

* Enhacements and dropping of Future.wait

* dartfmt

* Rate-limit the number of tools in flight

* checkpoint

* Workaround for duplicate ResovledLibraryResult objects

* Patch up merge problem and rebuild test package docs

* Use maps to avoid having to filter results

* dartfmt and review comments

* Update test package docs again (quotation marks different in stable branch)

* Clear pubspec overrides (this will only work in sdk-analyzer Travis)

* Change sdk test branch to master (analyzer-0.33 not integrated yet)

* Set correct analyzer version

* Eliminate initialization race where analyzer might not find all the files in time for empty packages

* Update analyzer requirement

* Fix merge error

Author: jcollins-g

lib/src/model.dart

@@ -10,6 +10,8 @@ import 'dart:collection' show UnmodifiableListView;
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:analyzer/dart/analysis/results.dart';
+import 'package:analyzer/dart/analysis/session.dart';
 import 'package:analyzer/dart/ast/ast.dart'
     show
         AnnotatedNode,
@@ -437,7 +439,7 @@ class Accessor extends ModelElement implements EnclosedElement {
     if (_sourceCode == null) {
       if (isSynthetic) {
         _sourceCode = packageGraph
-            ._getModelNodeFor((element as PropertyAccessorElement).variable)
+            ._getModelNodeFor((element as PropertyAccessorElement).variable, this.definingLibrary)
             .sourceCode;
       } else {
         _sourceCode = super.sourceCode;
@@ -495,8 +497,8 @@ class Accessor extends ModelElement implements EnclosedElement {
   @override
   ModelElement get enclosingElement {
     if (_accessor.enclosingElement is CompilationUnitElement) {
-      return packageGraph
-          .findOrCreateLibraryFor(_accessor.enclosingElement.enclosingElement);
+      return packageGraph.findButDoNotCreateLibraryFor(
+          _accessor.enclosingElement.enclosingElement);
     }
 
     return new ModelElement.from(
@@ -2059,16 +2061,17 @@ abstract class GetterSetterCombo implements ModelElement {
 }
 
 class Library extends ModelElement with Categorization, TopLevelContainer {
+  final ResolvedLibraryResult libraryResult;
   List<TopLevelVariable> _variables;
   Namespace _exportedNamespace;
   String _name;
 
   factory Library(LibraryElement element, PackageGraph packageGraph) {
-    return packageGraph.findOrCreateLibraryFor(element);
+    return packageGraph.findButDoNotCreateLibraryFor(element);
   }
 
-  Library._(LibraryElement element, PackageGraph packageGraph, this._package)
-      : super(element, null, packageGraph, null) {
+  Library._(this.libraryResult, PackageGraph packageGraph, this._package)
+      : super(libraryResult.element, null, packageGraph, null) {
     if (element == null) throw new ArgumentError.notNull('element');
     _exportedNamespace =
         new NamespaceBuilder().createExportNamespaceForLibrary(element);
@@ -2107,9 +2110,12 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
                 new ModelElement.fromElement(e.setter.element, packageGraph);
           }
         }
-        return new ModelElement.from(e.element,
-                packageGraph.findOrCreateLibraryFor(e.element), packageGraph,
-                getter: getter, setter: setter)
+        return new ModelElement.from(
+                e.element,
+                packageGraph.findButDoNotCreateLibraryFor(e.element),
+                packageGraph,
+                getter: getter,
+                setter: setter)
             .fullyQualifiedName;
       }).toList();
     }
@@ -2894,7 +2900,7 @@ abstract class ModelElement extends Canonicalization
       this._element, this._library, this._packageGraph, this._originalMember) {}
 
   factory ModelElement.fromElement(Element e, PackageGraph p) {
-    Library lib = p.findOrCreateLibraryFor(e);
+    Library lib = p.findButDoNotCreateLibraryFor(e);
     Accessor getter;
     Accessor setter;
     if (e is PropertyInducingElement) {
@@ -3099,7 +3105,7 @@ abstract class ModelElement extends Canonicalization
   ModelNode _modelNode;
   @override
   ModelNode get modelNode =>
-      _modelNode ??= packageGraph._getModelNodeFor(element);
+      _modelNode ??= packageGraph._getModelNodeFor(element, definingLibrary);
 
   List<String> get annotations => annotationsFromMetadata(element.metadata);
 
@@ -3344,7 +3350,8 @@ abstract class ModelElement extends Canonicalization
         documentationFrom.map((e) => e.documentationLocal).join('<p>'));
   }
 
-  Library get definingLibrary => packageGraph.findOrCreateLibraryFor(element);
+  Library get definingLibrary =>
+      packageGraph.findButDoNotCreateLibraryFor(element);
 
   Library _canonicalLibrary;
   // _canonicalLibrary can be null so we can't check against null to see whether
@@ -4651,31 +4658,28 @@ class PackageGraph {
   // to this graph.
 
   PackageGraph._(this.config, this.packageMeta, this._packageWarningOptions,
-      this.driver, this.sdk) {}
+      this.driver, this.sdk) : session = driver.currentSession {}
 
   static Future<PackageGraph> setUpPackageGraph(
-    Iterable<LibraryElement> libraryElements,
-    Iterable<LibraryElement> specialLibraryElements,
-    DartdocOptionContext config,
-    PackageMeta packageMeta,
-    packageWarningOptions,
-    driver,
-    sdk,
-  ) async {
+      Iterable<ResolvedLibraryResult> libraryResults,
+      Iterable<ResolvedLibraryResult> specialLibraryResults,
+      DartdocOptionContext config,
+      PackageMeta packageMeta,
+      packageWarningOptions,
+      driver,
+      sdk,) async {
     PackageGraph newGraph =
         PackageGraph._(config, packageMeta, packageWarningOptions, driver, sdk);
     assert(newGraph._allConstructedModelElements.isEmpty);
     assert(newGraph.allLibraries.isEmpty);
     newGraph._packageWarningCounter =
         new PackageWarningCounter(newGraph._packageWarningOptions);
 
-    // Build [Package] objects.
-    libraryElements.forEach((element) {});
-
     // Build [Library] objects, and link them to [Package]s.
-    libraryElements.forEach((element) {
+    libraryResults.forEach((result) {
+      LibraryElement element = result.element;
       var packageMeta = new PackageMeta.fromElement(element, config);
-      var lib = new Library._(element, newGraph,
+      var lib = new Library._(result, newGraph,
           new Package.fromPackageMeta(packageMeta, newGraph));
       newGraph.packageMap[packageMeta.name]._libraries.add(lib);
       newGraph.allLibraries[element] = lib;
@@ -4687,9 +4691,9 @@ class PackageGraph {
     newGraph.allLibrariesAdded = true;
 
     // [findOrCreateLibraryFor] already adds to the proper structures.
-    specialLibraryElements.forEach((element) {
-      newGraph.findOrCreateLibraryFor(element);
-    });
+    for (ResolvedLibraryResult result in specialLibraryResults) {
+      await newGraph.findOrCreateLibraryFor(result.element);
+    }
 
     // From here on in, we might find special objects.  Initialize the
     // specialClasses handler so when we find them, they get added.
@@ -4731,10 +4735,9 @@ class PackageGraph {
   // Many ModelElements have the same ModelNode; don't build/cache this data more
   // than once for them.
   final Map<Element, ModelNode> _modelNodes = Map();
-  ModelNode _getModelNodeFor(element) {
-    /// TODO(jcollins-g): merge with removal of computeNode.
+  ModelNode _getModelNodeFor(element, Library definingLibrary) {
     _modelNodes.putIfAbsent(
-        element, () => ModelNode(element?.computeNode(), element));
+        element, () => ModelNode(definingLibrary.libraryResult.getElementDeclaration(element)?.node , element));
     return _modelNodes[element];
   }
 
@@ -4808,7 +4811,9 @@ class PackageGraph {
   /// Map of package name to Package.
   final Map<String, Package> packageMap = {};
 
+  /// TODO(brianwilkerson) Replace the driver with the session.
   final AnalysisDriver driver;
+  final AnalysisSession session;
   final DartSdk sdk;
 
   Map<Source, SdkLibrary> _sdkLibrarySources;
@@ -5089,7 +5094,7 @@ class PackageGraph {
       // The first call to _tagReexportFor should not have a null libraryElement.
       assert(lastExportedElement != null);
       warnOnElement(
-          findOrCreateLibraryFor(lastExportedElement.enclosingElement),
+          findButDoNotCreateLibraryFor(lastExportedElement.enclosingElement),
           PackageWarning.unresolvedExport,
           message: '"${lastExportedElement.uri}"',
           referredFrom: <Locatable>[topLevelLibrary]);
@@ -5401,7 +5406,18 @@ class PackageGraph {
   /// This is used when we might need a Library object that isn't actually
   /// a documentation entry point (for elements that have no Library within the
   /// set of canonical Libraries).
-  Library findOrCreateLibraryFor(Element e) {
+  Library findButDoNotCreateLibraryFor(Element e) {
+    // This is just a cache to avoid creating lots of libraries over and over.
+    if (allLibraries.containsKey(e.library)) {
+      return allLibraries[e.library];
+    }
+    return null;
+  }
+
+  /// This is used when we might need a Library object that isn't actually
+  /// a documentation entry point (for elements that have no Library within the
+  /// set of canonical Libraries).
+  Future<Library> findOrCreateLibraryFor(Element e) async {
     // This is just a cache to avoid creating lots of libraries over and over.
     if (allLibraries.containsKey(e.library)) {
       return allLibraries[e.library];
@@ -5411,8 +5427,10 @@ class PackageGraph {
       return null;
     }
 
+    ResolvedLibraryResult result =
+        await session.getResolvedLibraryByElement(e.library);
     Library foundLibrary = new Library._(
-        e.library,
+        result,
         this,
         new Package.fromPackageMeta(
             new PackageMeta.fromElement(e.library, config), packageGraph));
@@ -6396,15 +6414,15 @@ class PackageBuilder {
     if (packageMeta.needsPubGet) {
       packageMeta.runPubGet();
     }
-    Set<LibraryElement> libraries = new Set();
-    Set<LibraryElement> specialLibraries = new Set();
+    Map<LibraryElement, ResolvedLibraryResult> libraryResults = new Map();
+    Map<LibraryElement, ResolvedLibraryResult> specialLibraryResults = new Map();
     DartSdk findSpecialsSdk = sdk;
     if (embedderSdk != null && embedderSdk.urlMappings.isNotEmpty) {
       findSpecialsSdk = embedderSdk;
     }
-    await getLibraries(libraries, specialLibraries, getFiles,
+    await getLibraries(libraryResults, specialLibraryResults, getFiles,
         specialLibraryFiles(findSpecialsSdk).toSet());
-    return await PackageGraph.setUpPackageGraph(libraries, specialLibraries,
+    return await PackageGraph.setUpPackageGraph(libraryResults.values, specialLibraryResults.values,
         config, config.topLevelPackageMeta, getWarningOptions(), driver, sdk);
   }
 
@@ -6516,11 +6534,22 @@ class PackageBuilder {
           options);
       driver.results.listen((_) {});
       driver.exceptions.listen((_) {});
+      _session = driver.currentSession;
       scheduler.start();
     }
     return _driver;
   }
 
+  AnalysisSession _session;
+  AnalysisSession get session {
+    if (_session == null) {
+      // The current session is initialized as a side-effect of initializing the
+      // driver.
+      driver;
+    }
+    return _session;
+  }
+
   PackageWarningOptions getWarningOptions() {
     PackageWarningOptions warningOptions =
         new PackageWarningOptions(config.verboseWarnings);
@@ -6554,7 +6583,7 @@ class PackageBuilder {
 
   /// Parse a single library at [filePath] using the current analysis driver.
   /// If [filePath] is not a library, returns null.
-  Future<LibraryElement> processLibrary(String filePath) async {
+  Future<ResolvedLibraryResult> processLibrary(String filePath) async {
     String name = filePath;
 
     if (name.startsWith(directoryCurrentPath)) {
@@ -6581,7 +6610,7 @@ class PackageBuilder {
     if (sourceKind == SourceKind.LIBRARY) {
       // Loading libraryElements from part files works, but is painfully slow
       // and creates many duplicates.
-      return (await driver.getResult(filePath))?.libraryElement;
+      return await session.getResolvedLibrary(source.fullName);
     }
     return null;
   }
@@ -6594,24 +6623,18 @@ class PackageBuilder {
     return metas;
   }
 
-  Future<List<LibraryElement>> _parseLibraries(Set<String> files) async {
-    Iterable<LibraryElement> libraries = new Iterable.empty();
+  Future<Map<LibraryElement, ResolvedLibraryResult>> _parseLibraries(Set<String> files) async {
+    Map<LibraryElement, ResolvedLibraryResult> libraries = new Map();
     Set<PackageMeta> lastPass = new Set();
     Set<PackageMeta> current;
-    Set<String> addedFiles = new Set();
     do {
       lastPass = _packageMetasForFiles(files);
-      files.difference(addedFiles).forEach((filename) {
-        addedFiles.add(filename);
-      });
-      libraries = quiverIterables.concat([
-        libraries,
-        (await Future.wait(files.map((f) => processLibrary(f))))
-            .where((LibraryElement l) => l != null)
-      ]);
+      for (ResolvedLibraryResult r in (await Future.wait(files.map((f) => processLibrary(f)))).where((ResolvedLibraryResult l) => l != null)) {
+        libraries[r.element] = r;
+      }
 
-      /// We don't care about upstream analysis errors, so save the first
-      /// source list.
+      // Be sure to give the analyzer enough time to find all the files.
+      await driver.discoverAvailableFiles();
       files.addAll(driver.knownFiles);
       files.addAll(_includeExternalsFrom(driver.knownFiles));
       current = _packageMetasForFiles(files);
@@ -6629,7 +6652,7 @@ class PackageBuilder {
         }
       }
     } while (!lastPass.containsAll(current));
-    return libraries.toList();
+    return libraries;
   }
 
   /// Given a package name, explore the directory and pull out all top level
@@ -6719,23 +6742,24 @@ class PackageBuilder {
   }
 
   Future<void> getLibraries(
-      Set<LibraryElement> libraries,
-      Set<LibraryElement> specialLibraries,
+      Map<LibraryElement, ResolvedLibraryResult> libraryResults,
+      Map<LibraryElement, ResolvedLibraryResult> specialLibraryResults,
       Set<String> files,
       Set<String> specialFiles) async {
-    libraries.addAll(await _parseLibraries(files));
-    specialLibraries
+    libraryResults.addAll(await _parseLibraries(files));
+    specialLibraryResults
         .addAll(await _parseLibraries(specialFiles.difference(files)));
     if (config.include.isNotEmpty) {
-      Iterable knownLibraryNames = libraries.map((l) => l.name);
+      Iterable knownLibraryNames = libraryResults.values.map((l) => l.element.name);
       Set notFound = new Set.from(config.include)
           .difference(new Set.from(knownLibraryNames))
           .difference(new Set.from(config.exclude));
       if (notFound.isNotEmpty) {
         throw 'Did not find: [${notFound.join(', ')}] in '
             'known libraries: [${knownLibraryNames.join(', ')}]';
       }
-      libraries.removeWhere((lib) => !config.include.contains(lib.name));
+      libraryResults.removeWhere(
+          (element, result) => !config.include.contains(result.element.name));
     }
   }
 

lib/src/model_utils.dart

@@ -131,4 +131,4 @@ String stripIndentFromSource(String source) {
     line = line.trimRight();
     return line.startsWith(indent) ? line.substring(indent.length) : line;
   }).join('\n');
-}
+}

pubspec.yaml

@@ -8,7 +8,7 @@ environment:
   sdk: '>=2.1.0-dev.9.4 <3.0.0'
 
 dependencies:
-  analyzer: ^0.33.3+2
+  analyzer: ^0.33.6
   args: '>=1.4.1 <2.0.0'
   collection: ^1.2.0
   crypto: ^2.0.6

tool/grind.dart

@@ -388,7 +388,7 @@ Future<String> createSdkDartdoc() async {
   await launcher.runStreamed('git', [
     'clone',
     '--branch',
-    'analyzer-0.33',
+    'master',
     '--depth',
     '1',
     'https://dart.googlesource.com/sdk.git',