Prepare tool_runner/definition for migration to NNBD (#2784)

* Prepare tool_runner/definition for migration to NNBD

* Fix up test failures

Author: jcollins-g

lib/dartdoc.dart

@@ -23,8 +23,6 @@ import 'package:dartdoc/src/logging.dart';
 import 'package:dartdoc/src/matching_link_result.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/package_meta.dart';
-import 'package:dartdoc/src/tool_definition.dart';
-import 'package:dartdoc/src/tool_runner.dart';
 import 'package:dartdoc/src/tuple.dart';
 import 'package:dartdoc/src/utils.dart';
 import 'package:dartdoc/src/version.dart';
@@ -194,12 +192,15 @@ class Dartdoc {
 
   Stream<String> get onCheckProgress => _onCheckProgress.stream;
 
+  @Deprecated('Will be removed in 4.0.0. '
+      'Use the return value from generateDocsBase instead.')
   PackageGraph packageGraph;
 
   @visibleForTesting
   Future<DartdocResults> generateDocsBase() async {
     var stopwatch = Stopwatch()..start();
-    packageGraph = await packageBuilder.buildPackageGraph();
+    var packageGraph = await packageBuilder.buildPackageGraph();
+    this.packageGraph = packageGraph;
     var seconds = stopwatch.elapsedMilliseconds / 1000.0;
     var libs = packageGraph.libraries.length;
     logInfo("Initialized dartdoc with $libs librar${libs == 1 ? 'y' : 'ies'} "
@@ -243,10 +244,11 @@ class Dartdoc {
   /// thrown if dartdoc fails in an expected way, for example if there is an
   /// analysis error in the code.
   Future<DartdocResults> generateDocs() async {
+    DartdocResults dartdocResults;
     try {
       logInfo('Documenting ${config.topLevelPackageMeta}...');
 
-      var dartdocResults = await generateDocsBase();
+      dartdocResults = await generateDocsBase();
       if (dartdocResults.packageGraph.localPublicLibraries.isEmpty) {
         logWarning('dartdoc could not find any libraries to document');
       }
@@ -261,10 +263,7 @@ class Dartdoc {
       logInfo('Success! Docs generated into $outDirPath');
       return dartdocResults;
     } finally {
-      // Clear out any cached tool snapshots and temporary directories.
-      SnapshotCache.instanceFor(config.resourceProvider).dispose();
-      // ignore: unawaited_futures
-      ToolTempFileTracker.instance?.dispose();
+      dartdocResults?.packageGraph?.dispose();
     }
   }
 

lib/src/model/package_graph.dart

@@ -20,6 +20,8 @@ import 'package:dartdoc/src/package_meta.dart'
     show PackageMeta, PackageMetaProvider;
 import 'package:dartdoc/src/render/renderer_factory.dart';
 import 'package:dartdoc/src/special_elements.dart';
+import 'package:dartdoc/src/tool_definition.dart';
+import 'package:dartdoc/src/tool_runner.dart';
 import 'package:dartdoc/src/tuple.dart';
 import 'package:dartdoc/src/warnings.dart';
 
@@ -37,6 +39,14 @@ class PackageGraph with CommentReferable, Nameable {
     Package.fromPackageMeta(packageMeta, this);
   }
 
+  void dispose() {
+    // Clear out any cached tool snapshots and temporary directories.
+    // TODO(jcollins-g): Consider ownership change for these objects
+    // so they are tied to PackageGraph instead of being global.
+    SnapshotCache.instanceFor(config.resourceProvider).dispose();
+    ToolTempFileTracker.instanceFor(config.resourceProvider).dispose();
+  }
+
   @override
   String get name => null;
 

lib/src/tool_definition.dart

@@ -247,7 +247,7 @@ class SnapshotCache {
   void dispose() {
     _instances.remove(_resourceProvider);
     if (snapshotCache != null && snapshotCache.exists) {
-      return snapshotCache.delete();
+      snapshotCache.delete();
     }
     return null;
   }

lib/src/tool_runner.dart

@@ -25,13 +25,11 @@ class ToolTempFileTracker {
       : temporaryDirectory =
             resourceProvider.createSystemTemp('dartdoc_tools_');
 
-  static ToolTempFileTracker _instance;
+  static final Map<ResourceProvider, ToolTempFileTracker> _instances = {};
 
-  static ToolTempFileTracker get instance => _instance;
-
-  static ToolTempFileTracker createInstance(
-          ResourceProvider resourceProvider) =>
-      _instance ??= ToolTempFileTracker._(resourceProvider);
+  static ToolTempFileTracker instanceFor(ResourceProvider resourceProvider) =>
+      _instances.putIfAbsent(
+          resourceProvider, () => ToolTempFileTracker._(resourceProvider));
 
   int _temporaryFileCount = 0;
 
@@ -46,9 +44,9 @@ class ToolTempFileTracker {
   }
 
   /// Call once no more files are to be created.
-  Future<void> dispose() async {
+  void dispose() {
     if (temporaryDirectory.exists) {
-      return temporaryDirectory.delete();
+      temporaryDirectory.delete();
     }
   }
 }
@@ -219,8 +217,8 @@ class ToolRunner {
     // file before running the tool synchronously.
 
     // Write the content to a temp file.
-    var tmpFile = ToolTempFileTracker.createInstance(resourceProvider)
-        .createTemporaryFile();
+    var tmpFile =
+        ToolTempFileTracker.instanceFor(resourceProvider).createTemporaryFile();
     tmpFile.writeAsStringSync(content);
     return pathContext.absolute(tmpFile.path);
   }