Sort files in nnbd_migration package.

Also add a test to ensure that they don't get out of order again.

Change-Id: Ia847b2b21d24de49c2ce74bbc9f2652642b45f38
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/234663
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>

pkg/analysis_server/test/verify_sorted_test.dart

@@ -27,6 +27,10 @@ void main() {
   group('analyzer_plugin', () {
     buildTestsForAnalyzerPlugin();
   });
+
+  group('nnbd_migration', () {
+    buildTestsForNnbdMigration();
+  });
 }
 
 void buildTests({
@@ -116,6 +120,12 @@ void buildTestsForAnalyzerPlugin() {
   );
 }
 
+void buildTestsForNnbdMigration() {
+  buildTests(
+      packagePath: 'nnbd_migration',
+      excludedPaths: ['lib/src/front_end/resources/resources.g.dart']);
+}
+
 void buildTestsIn(AnalysisSession session, String testDirPath,
     List<String> excludedPath, Folder directory) {
   var pathContext = session.resourceProvider.pathContext;

pkg/nnbd_migration/lib/src/edit_plan.dart

@@ -1713,6 +1713,26 @@ extension AtomicEditList on List<AtomicEdit> {
 /// [AtomicEdit]s.  This data structure is used by [EditPlan]s to accumulate
 /// source file changes.
 extension AtomicEditMap on Map<int?, List<AtomicEdit>>? {
+  /// Destructively combines two change representations.  If one or the other
+  /// input is null, the other input is returned unchanged for efficiency.
+  Map<int?, List<AtomicEdit>>? operator +(
+      Map<int?, List<AtomicEdit>>? newChanges) {
+    if (newChanges == null) return this;
+    if (this == null) {
+      return newChanges;
+    } else {
+      for (var entry in newChanges.entries) {
+        var currentValue = this![entry.key];
+        if (currentValue == null) {
+          this![entry.key] = entry.value;
+        } else {
+          currentValue.addAll(entry.value);
+        }
+      }
+      return this;
+    }
+  }
+
   /// Applies the changes to source file text.
   ///
   /// If [includeInformative] is `true`, informative edits are included;
@@ -1734,26 +1754,6 @@ extension AtomicEditMap on Map<int?, List<AtomicEdit>>? {
             .toSourceEdit(offset!, includeInformative: includeInformative)
     ];
   }
-
-  /// Destructively combines two change representations.  If one or the other
-  /// input is null, the other input is returned unchanged for efficiency.
-  Map<int?, List<AtomicEdit>>? operator +(
-      Map<int?, List<AtomicEdit>>? newChanges) {
-    if (newChanges == null) return this;
-    if (this == null) {
-      return newChanges;
-    } else {
-      for (var entry in newChanges.entries) {
-        var currentValue = this![entry.key];
-        if (currentValue == null) {
-          this![entry.key] = entry.value;
-        } else {
-          currentValue.addAll(entry.value);
-        }
-      }
-      return this;
-    }
-  }
 }
 
 /// Extension allowing an AstNode to be queried to see if it ends in a casade

pkg/nnbd_migration/lib/src/front_end/charcodes.dart

@@ -2,11 +2,11 @@
 // 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.
 
-/// "Line feed" control character.
-const int $lf = 0x0a;
-
 /// "Carriage return" control character.
 const int $cr = 0x0d;
 
+/// "Line feed" control character.
+const int $lf = 0x0a;
+
 /// Space character.
 const int $space = 0x20;

pkg/nnbd_migration/lib/src/front_end/migration_info.dart

@@ -81,14 +81,14 @@ class MigrationInfo {
   String absolutePathFromRoot(String? path) =>
       pathContext.join(includedRoot!, path);
 
-  /// Returns the relative path of [path] from [includedRoot].
-  String relativePathFromRoot(String path) =>
-      pathContext.relative(path, from: includedRoot);
-
   /// Return the path to [unit] from [includedRoot], to be used as a display
   /// name for a library.
   String computeName(UnitInfo unit) => relativePathFromRoot(unit.path!);
 
+  /// Returns the relative path of [path] from [includedRoot].
+  String relativePathFromRoot(String path) =>
+      pathContext.relative(path, from: includedRoot);
+
   List<UnitLink> unitLinks() {
     var links = <UnitLink>[];
     for (var unit in units!) {

pkg/nnbd_migration/lib/src/utilities/annotation_tracker.dart

@@ -9,9 +9,8 @@ import 'package:analyzer/dart/ast/visitor.dart';
 class AnnotationTracker extends RecursiveAstVisitor<void> {
   final Set<Annotation> _nodes = {};
 
-  @override
-  void visitAnnotation(Annotation node) {
-    _nodes.add(node);
+  void finalize() {
+    assert(_nodes.isEmpty, 'Annotation nodes not visited: $_nodes');
   }
 
   void nodeVisited(Annotation node) {
@@ -20,7 +19,8 @@ class AnnotationTracker extends RecursiveAstVisitor<void> {
     }
   }
 
-  void finalize() {
-    assert(_nodes.isEmpty, 'Annotation nodes not visited: $_nodes');
+  @override
+  void visitAnnotation(Annotation node) {
+    _nodes.add(node);
   }
 }

pkg/nnbd_migration/lib/src/utilities/multi_future_tracker.dart

@@ -37,14 +37,6 @@ class MultiFutureTracker {
 
   MultiFutureTracker(this.parallel);
 
-  /// Wait until fewer or equal to this many Futures are outstanding.
-  Future<void> _waitUntil(int max) async {
-    assert(_trackedFutures.length <= parallel);
-    while (_trackedFutures.length > max) {
-      await Future.any(_trackedFutures);
-    }
-  }
-
   /// Generates a [Future] from the given closure and adds it to the queue,
   /// once the queue is sufficiently empty.  The returned future completes
   /// when the generated [Future] has been added to the queue.
@@ -73,4 +65,12 @@ class MultiFutureTracker {
 
   /// Wait until all futures added so far have completed.
   Future<void> wait() => _waitUntil(0);
+
+  /// Wait until fewer or equal to this many Futures are outstanding.
+  Future<void> _waitUntil(int max) async {
+    assert(_trackedFutures.length <= parallel);
+    while (_trackedFutures.length > max) {
+      await Future.any(_trackedFutures);
+    }
+  }
 }

pkg/nnbd_migration/lib/src/utilities/subprocess_launcher.dart

@@ -9,7 +9,6 @@
 /// This is a modified version of dartdoc's
 /// SubprocessLauncher from test/src/utils.dart, for use with the
 /// nnbd_migration script.
-
 import 'dart:convert';
 import 'dart:io';
 
@@ -34,20 +33,6 @@ class SubprocessLauncher {
   final String context;
   final Map<String, String> environmentDefaults;
 
-  /// From flutter:dev/tools/dartdoc.dart, modified.
-  static Future<void> _printStream(Stream<List<int>> stream, Stdout output,
-      {String prefix = '', Iterable<String> Function(String line)? filter}) {
-    filter ??= (line) => [line];
-    return stream
-        .transform(utf8.decoder)
-        .transform(const LineSplitter())
-        .expand(filter)
-        .listen((String line) {
-      output.write('$prefix$line'.trim());
-      output.write('\n');
-    }).asFuture();
-  }
-
   SubprocessLauncher(this.context, [Map<String, String>? environment])
       : environmentDefaults = environment ?? <String, String>{};
 
@@ -192,4 +177,18 @@ class SubprocessLauncher {
     }
     return jsonObjects;
   }
+
+  /// From flutter:dev/tools/dartdoc.dart, modified.
+  static Future<void> _printStream(Stream<List<int>> stream, Stdout output,
+      {String prefix = '', Iterable<String> Function(String line)? filter}) {
+    filter ??= (line) => [line];
+    return stream
+        .transform(utf8.decoder)
+        .transform(const LineSplitter())
+        .expand(filter)
+        .listen((String line) {
+      output.write('$prefix$line'.trim());
+      output.write('\n');
+    }).asFuture();
+  }
 }

pkg/nnbd_migration/test/front_end/info_builder_test.dart

@@ -798,34 +798,6 @@ void f(int/*?*/ a) {
         edits: isEmpty);
   }
 
-  Future<void> test_insertedRequired_fieldFormal_hint() async {
-    var unit = await buildInfoForSingleTestFile('''
-class C {
-  int level;
-  int level2;
-  C({this.level}) : this.level2 = level + 1;
-}
-''', migratedContent: '''
-class C {
-  int  level;
-  int  level2;
-  C({required this.level}) : this.level2 = level + 1;
-}
-''');
-    var regions = unit.fixRegions;
-    expect(regions, hasLength(1));
-    var region = regions[0];
-    var edits = region.edits;
-    assertRegion(
-        region: region,
-        offset: 44,
-        length: 9,
-        explanation: "Add 'required' keyword to parameter 'level' in 'C.'",
-        kind: NullabilityFixKind.addRequired);
-    assertEdit(
-        edit: edits[0], offset: 42, length: 0, replacement: '/*required*/ ');
-  }
-
   Future<void> test_insertedRequired_fieldFormal() async {
     addMetaPackage();
     var unit = await buildInfoForSingleTestFile('''
@@ -857,6 +829,63 @@ class C {
         edit: edits[0], offset: 75, length: 0, replacement: '@required ');
   }
 
+  Future<void> test_insertedRequired_fieldFormal_hint() async {
+    var unit = await buildInfoForSingleTestFile('''
+class C {
+  int level;
+  int level2;
+  C({this.level}) : this.level2 = level + 1;
+}
+''', migratedContent: '''
+class C {
+  int  level;
+  int  level2;
+  C({required this.level}) : this.level2 = level + 1;
+}
+''');
+    var regions = unit.fixRegions;
+    expect(regions, hasLength(1));
+    var region = regions[0];
+    var edits = region.edits;
+    assertRegion(
+        region: region,
+        offset: 44,
+        length: 9,
+        explanation: "Add 'required' keyword to parameter 'level' in 'C.'",
+        kind: NullabilityFixKind.addRequired);
+    assertEdit(
+        edit: edits[0], offset: 42, length: 0, replacement: '/*required*/ ');
+  }
+
+  Future<void> test_insertedRequired_parameter() async {
+    addMetaPackage();
+    var unit = await buildInfoForSingleTestFile('''
+import 'package:meta/meta.dart';
+class C {
+  int level = 0;
+  bool f({int lvl}) => lvl >= level;
+}
+''', migratedContent: '''
+import 'package:meta/meta.dart';
+class C {
+  int  level = 0;
+  bool  f({required int  lvl}) => lvl >= level;
+}
+''');
+    var regions = unit.fixRegions;
+    expect(regions, hasLength(1));
+    var region = regions[0];
+    var edits = region.edits;
+    assertRegion(
+        region: region,
+        offset: 72,
+        length: 9,
+        explanation: "Add 'required' keyword to parameter 'lvl' in 'C.f'",
+        kind: NullabilityFixKind.addRequired);
+    assertEdit(
+        edit: edits[0], offset: 70, length: 0, replacement: '@required ');
+  }
+
   Future<void> test_insertedRequired_parameter_hint() async {
     var unit = await buildInfoForSingleTestFile('''
 class C {
@@ -906,35 +935,6 @@ class C {
         edit: edits[0], offset: 78, length: 0, replacement: '@meta.required ');
   }
 
-  Future<void> test_insertedRequired_parameter() async {
-    addMetaPackage();
-    var unit = await buildInfoForSingleTestFile('''
-import 'package:meta/meta.dart';
-class C {
-  int level = 0;
-  bool f({int lvl}) => lvl >= level;
-}
-''', migratedContent: '''
-import 'package:meta/meta.dart';
-class C {
-  int  level = 0;
-  bool  f({required int  lvl}) => lvl >= level;
-}
-''');
-    var regions = unit.fixRegions;
-    expect(regions, hasLength(1));
-    var region = regions[0];
-    var edits = region.edits;
-    assertRegion(
-        region: region,
-        offset: 72,
-        length: 9,
-        explanation: "Add 'required' keyword to parameter 'lvl' in 'C.f'",
-        kind: NullabilityFixKind.addRequired);
-    assertEdit(
-        edit: edits[0], offset: 70, length: 0, replacement: '@required ');
-  }
-
   Future<void> test_insertParens() async {
     var originalContent = '''
 class C {

pkg/nnbd_migration/test/front_end/navigation_tree_renderer_test.dart

@@ -330,11 +330,11 @@ class NavigationTreeRendererTest extends NnbdMigrationTestBase {
 }
 
 extension<T extends NavigationTreeNode> on TypeMatcher<T> {
-  TypeMatcher<T> named(dynamic matcher) =>
-      having((node) => node.name, 'name', matcher);
-
   TypeMatcher<T> havingMigrationStatus(dynamic matcher) =>
       having((node) => node.migrationStatus, 'migrationStatus', matcher);
+
+  TypeMatcher<T> named(dynamic matcher) =>
+      having((node) => node.name, 'name', matcher);
 }
 
 extension on TypeMatcher<NavigationTreeDirectoryNode> {

pkg/nnbd_migration/test/utilities/multi_future_tracker_test.dart

@@ -24,6 +24,22 @@ class MultiFutureTrackerTest {
     testTracker = null;
   }
 
+  Future<void> test_doesNotBlockWithoutLimit() async {
+    var completer1 = Completer();
+
+    // Limit is set above the number of futures we are adding.
+    testTracker = MultiFutureTracker(10);
+    await testTracker!.addFutureFromClosure(() => completer1.future);
+    // The second future added should be executing even though the first
+    // future is not complete.  A test failure will time out.
+    await testTracker!.addFutureFromClosure(() async {
+      expect(completer1.isCompleted, isFalse);
+      completer1.complete();
+    });
+
+    return await testTracker!.wait();
+  }
+
   Future<void> test_multiFutureBlocksOnLimit() async {
     var completer1 = Completer();
 
@@ -40,20 +56,12 @@ class MultiFutureTrackerTest {
     return await testTracker!.wait();
   }
 
-  Future<void> test_doesNotBlockWithoutLimit() async {
-    var completer1 = Completer();
-
-    // Limit is set above the number of futures we are adding.
-    testTracker = MultiFutureTracker(10);
-    await testTracker!.addFutureFromClosure(() => completer1.future);
-    // The second future added should be executing even though the first
-    // future is not complete.  A test failure will time out.
-    await testTracker!.addFutureFromClosure(() async {
-      expect(completer1.isCompleted, isFalse);
-      completer1.complete();
-    });
-
-    return await testTracker!.wait();
+  Future<void> test_returnsValueFromRun() async {
+    testTracker = MultiFutureTracker(1);
+    await expectLater(await testTracker!.runFutureFromClosure(() async => true),
+        equals(true));
+    await expectLater(
+        await testTracker!.runFutureFromClosure(() => true), equals(true));
   }
 
   Future<void> test_runsSeriallyAtLowLimit() async {
@@ -76,12 +84,4 @@ class MultiFutureTrackerTest {
     await runFuture1;
     await runFuture2;
   }
-
-  Future<void> test_returnsValueFromRun() async {
-    testTracker = MultiFutureTracker(1);
-    await expectLater(await testTracker!.runFutureFromClosure(() async => true),
-        equals(true));
-    await expectLater(
-        await testTracker!.runFutureFromClosure(() => true), equals(true));
-  }
 }

pkg/nnbd_migration/test/utilities/subprocess_launcher_test.dart

@@ -31,25 +31,6 @@ class SubprocessLauncherTest {
     await tempDir.delete(recursive: true);
   }
 
-  Future<void> test_subprocessWorksViaParallelSubprocessLimit() async {
-    SubprocessLauncher launcher =
-        SubprocessLauncher('test_subprocessWorksViaParallelSubprocessLimit');
-
-    await launcher.runStreamed(Platform.resolvedExecutable, ['--version'],
-        perLine: outputCallback);
-    expect(output, anyElement(contains('Dart')));
-  }
-
-  Future<void> test_subprocessRunsValidExecutable() async {
-    SubprocessLauncher launcher =
-        SubprocessLauncher('test_subprocessRunsValidExecutable');
-
-    await launcher.runStreamedImmediate(
-        Platform.resolvedExecutable, ['--version'],
-        perLine: outputCallback);
-    expect(output, anyElement(contains('Dart')));
-  }
-
   Future<void> test_subprocessPassesArgs() async {
     SubprocessLauncher launcher =
         SubprocessLauncher('test_subprocessPassesArgs');
@@ -90,6 +71,16 @@ class SubprocessLauncherTest {
             '^environment: .*__SUBPROCESS_PASSES_ENVIRONMENT_TEST: yes'))));
   }
 
+  Future<void> test_subprocessRunsValidExecutable() async {
+    SubprocessLauncher launcher =
+        SubprocessLauncher('test_subprocessRunsValidExecutable');
+
+    await launcher.runStreamedImmediate(
+        Platform.resolvedExecutable, ['--version'],
+        perLine: outputCallback);
+    expect(output, anyElement(contains('Dart')));
+  }
+
   Future<void> test_subprocessSetsWorkingDirectory() async {
     SubprocessLauncher launcher =
         SubprocessLauncher('test_subprocessSetsWorkingDirectory');
@@ -128,4 +119,13 @@ class SubprocessLauncherTest {
             perLine: outputCallback),
         throwsA(TypeMatcher<ProcessException>()));
   }
+
+  Future<void> test_subprocessWorksViaParallelSubprocessLimit() async {
+    SubprocessLauncher launcher =
+        SubprocessLauncher('test_subprocessWorksViaParallelSubprocessLimit');
+
+    await launcher.runStreamed(Platform.resolvedExecutable, ['--version'],
+        perLine: outputCallback);
+    expect(output, anyElement(contains('Dart')));
+  }
 }

pkg/nnbd_migration/test/verify_tests_test.dart

@@ -2,9 +2,9 @@
 // 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 'package:analyzer/file_system/physical_file_system.dart';
 import 'package:analyzer_utilities/package_root.dart' as package_root;
 import 'package:analyzer_utilities/verify_tests.dart';
-import 'package:analyzer/file_system/physical_file_system.dart';
 
 void main() {
   var provider = PhysicalResourceProvider.INSTANCE;

pkg/nnbd_migration/tool/codegen/generate_resources.dart

@@ -54,17 +54,6 @@ Run with '--verify' to validate that the web resource have been regenerated.
   }
 }
 
-/// Returns the dart2jsPath, either from [argResults] or the Platform.
-String? dart2jsPath(ArgResults argResults) {
-  if (argResults.wasParsed('dart2js_path')) {
-    return argResults['dart2js_path'] as String?;
-  } else {
-    var sdkBinDir = path.dirname(Platform.resolvedExecutable);
-    var dart2jsBinary = Platform.isWindows ? 'dart2js.bat' : 'dart2js';
-    return path.join(sdkBinDir, dart2jsBinary);
-  }
-}
-
 final File dartSources = File(path.join('pkg', 'nnbd_migration', 'lib', 'src',
     'front_end', 'web', 'migration.dart'));
 
@@ -131,6 +120,17 @@ void createResourcesGDart() {
   resourcesFile.writeAsStringSync(content);
 }
 
+/// Returns the dart2jsPath, either from [argResults] or the Platform.
+String? dart2jsPath(ArgResults argResults) {
+  if (argResults.wasParsed('dart2js_path')) {
+    return argResults['dart2js_path'] as String?;
+  } else {
+    var sdkBinDir = path.dirname(Platform.resolvedExecutable);
+    var dart2jsBinary = Platform.isWindows ? 'dart2js.bat' : 'dart2js';
+    return path.join(sdkBinDir, dart2jsBinary);
+  }
+}
+
 void fail(String message) {
   stderr.writeln(message);
   exit(1);

pkg/nnbd_migration/tool/src/package.dart

@@ -3,12 +3,33 @@
 // BSD-style license that can be found in the LICENSE file.
 
 /// Abstractions for the different sources of truth for different packages.
-
 import 'dart:io';
 
 import 'package:nnbd_migration/src/utilities/subprocess_launcher.dart';
 import 'package:path/path.dart' as path;
 
+final String defaultPlaygroundPath =
+    Platform.environment['TRIAL_MIGRATION_PLAYGROUND'] ??
+        resolveTildePath('~/.nnbd_trial_migration');
+
+/// The pub cache inherited by this process.
+final String defaultPubCache =
+    Platform.environment['PUB_CACHE'] ?? resolveTildePath('~/.pub-cache');
+
+/// Returns the path to the SDK repository this script is a part of.
+final String thisSdkRepo = () {
+  var maybeSdkRepoDir = Platform.script.toFilePath();
+  while (maybeSdkRepoDir != path.dirname(maybeSdkRepoDir)) {
+    maybeSdkRepoDir = path.dirname(maybeSdkRepoDir);
+    if (File(path.join(maybeSdkRepoDir, 'README.dart-sdk')).existsSync()) {
+      return maybeSdkRepoDir;
+    }
+  }
+  throw UnsupportedError(
+      'Script ${Platform.script} using this library must be within the SDK repository');
+}();
+Uri get thisSdkUri => Uri.file(thisSdkRepo);
+
 /// Return a resolved path including the home directory in place of tilde
 /// references.
 String resolveTildePath(String originalPath) {
@@ -27,93 +48,36 @@ String resolveTildePath(String originalPath) {
   return path.join(homeDir, originalPath.substring(2));
 }
 
-/// The pub cache inherited by this process.
-final String defaultPubCache =
-    Platform.environment['PUB_CACHE'] ?? resolveTildePath('~/.pub-cache');
-final String defaultPlaygroundPath =
-    Platform.environment['TRIAL_MIGRATION_PLAYGROUND'] ??
-        resolveTildePath('~/.nnbd_trial_migration');
-Uri get thisSdkUri => Uri.file(thisSdkRepo);
-
-class Playground {
-  final String playgroundPath;
-
-  /// If [clean] is true, this will delete the playground.  Otherwise,
-  /// if it exists it will assume it is properly constructed.
-  Playground(this.playgroundPath, bool clean) {
-    Directory playground = Directory(playgroundPath);
-    if (clean) {
-      if (playground.existsSync()) {
-        playground.deleteSync(recursive: true);
-      }
-    }
-    if (!playground.existsSync()) playground.createSync();
-  }
-
-  /// Build an environment for subprocesses.
-  Map<String, String> get env => {'PUB_CACHE': pubCachePath};
-
-  String get pubCachePath => path.join(playgroundPath, '.pub-cache');
-}
-
-/// Returns the path to the SDK repository this script is a part of.
-final String thisSdkRepo = () {
-  var maybeSdkRepoDir = Platform.script.toFilePath();
-  while (maybeSdkRepoDir != path.dirname(maybeSdkRepoDir)) {
-    maybeSdkRepoDir = path.dirname(maybeSdkRepoDir);
-    if (File(path.join(maybeSdkRepoDir, 'README.dart-sdk')).existsSync()) {
-      return maybeSdkRepoDir;
-    }
-  }
-  throw UnsupportedError(
-      'Script ${Platform.script} using this library must be within the SDK repository');
-}();
-
-/// Abstraction for an unmanaged package.
-class ManualPackage extends Package {
-  final String _packagePath;
-  ManualPackage(this._packagePath) : super(_packagePath);
-
-  @override
-  List<String> get migrationPaths => [_packagePath];
-}
-
 /// Abstraction for a package fetched via Git.
 class GitPackage extends Package {
+  static final RegExp _pathAndPeriodSplitter = RegExp('[\\/.]');
   final String _clonePath;
   final bool? _keepUpdated;
   final String label;
+
   final Playground _playground;
 
+  SubprocessLauncher? _launcher;
+
+  String? _packagePath;
+
   GitPackage._(this._clonePath, this._playground, this._keepUpdated,
       {String? name, this.label = 'master'})
       : super(name ?? _buildName(_clonePath));
 
-  static Future<GitPackage> gitPackageFactory(
-      String clonePath, Playground playground, bool? keepUpdated,
-      {String? name, String label = 'master'}) async {
-    GitPackage gitPackage = GitPackage._(clonePath, playground, keepUpdated,
-        name: name, label: label);
-    await gitPackage._init();
-    return gitPackage;
-  }
+  SubprocessLauncher get launcher =>
+      _launcher ??= SubprocessLauncher('$name-$label', _playground.env);
 
-  /// Calculate the "humanish" name of the clone (see `git help clone`).
-  static String _buildName(String clonePath) {
-    if (Directory(clonePath).existsSync()) {
-      // assume we are cloning locally
-      return path.basename(clonePath);
-    }
-    List<String> pathParts = clonePath.split(_pathAndPeriodSplitter);
-    int indexOfName = pathParts.lastIndexOf('git') - 1;
-    if (indexOfName < 0) {
-      throw ArgumentError(
-          'GitPackage can not figure out the name for $clonePath, pass it in manually?');
-    }
-    return pathParts[indexOfName];
-  }
+  @override
+  List<String?> get migrationPaths => [_packagePath];
+  String get packagePath =>
+      // TODO(jcollins-g): allow packages from subdirectories of clones
+      _packagePath ??= path.join(_playground.playgroundPath, '$name-$label');
 
-  static final RegExp _pathAndPeriodSplitter = RegExp('[\\/.]');
+  @override
+  String toString() {
+    return '$_clonePath ($label)' + (_keepUpdated! ? ' [synced]' : '');
+  }
 
   /// Initialize the package with a shallow clone.  Run only once per
   /// [GitPackage] instance.
@@ -139,58 +103,38 @@ class GitPackage extends Package {
     }
   }
 
-  SubprocessLauncher? _launcher;
-  SubprocessLauncher get launcher =>
-      _launcher ??= SubprocessLauncher('$name-$label', _playground.env);
-
-  String? _packagePath;
-  String get packagePath =>
-      // TODO(jcollins-g): allow packages from subdirectories of clones
-      _packagePath ??= path.join(_playground.playgroundPath, '$name-$label');
-
-  @override
-  List<String?> get migrationPaths => [_packagePath];
-
-  @override
-  String toString() {
-    return '$_clonePath ($label)' + (_keepUpdated! ? ' [synced]' : '');
-  }
-}
-
-/// Abstraction for a package fetched via pub.
-class PubPackage extends Package {
-  PubPackage(String name, [String? version]) : super(name) {
-    throw UnimplementedError();
+  static Future<GitPackage> gitPackageFactory(
+      String clonePath, Playground playground, bool? keepUpdated,
+      {String? name, String label = 'master'}) async {
+    GitPackage gitPackage = GitPackage._(clonePath, playground, keepUpdated,
+        name: name, label: label);
+    await gitPackage._init();
+    return gitPackage;
   }
 
-  @override
-  // TODO: implement packagePath
-  List<String> get migrationPaths => throw UnimplementedError();
-}
-
-/// Abstraction for a package located within pkg or third_party/pkg.
-class SdkPackage extends Package {
-  /// Where to find packages.  Constructor searches in-order.
-  static final List<String> _searchPaths = [
-    'pkg',
-    path.join('third_party', 'pkg'),
-  ];
-
-  SdkPackage(String name) : super(name) {
-    for (String potentialPath
-        in _searchPaths.map((p) => path.join(thisSdkRepo, p, name))) {
-      if (Directory(potentialPath).existsSync()) {
-        _packagePath = potentialPath;
-      }
+  /// Calculate the "humanish" name of the clone (see `git help clone`).
+  static String _buildName(String clonePath) {
+    if (Directory(clonePath).existsSync()) {
+      // assume we are cloning locally
+      return path.basename(clonePath);
     }
+    List<String> pathParts = clonePath.split(_pathAndPeriodSplitter);
+    int indexOfName = pathParts.lastIndexOf('git') - 1;
+    if (indexOfName < 0) {
+      throw ArgumentError(
+          'GitPackage can not figure out the name for $clonePath, pass it in manually?');
+    }
+    return pathParts[indexOfName];
   }
+}
+
+/// Abstraction for an unmanaged package.
+class ManualPackage extends Package {
+  final String _packagePath;
+  ManualPackage(this._packagePath) : super(_packagePath);
 
-  late final String _packagePath;
   @override
   List<String> get migrationPaths => [_packagePath];
-
-  @override
-  String toString() => path.relative(_packagePath, from: thisSdkRepo);
 }
 
 /// Base class for pub, github, SDK, or possibly other package sources.
@@ -206,6 +150,38 @@ abstract class Package {
   String toString() => name;
 }
 
+class Playground {
+  final String playgroundPath;
+
+  /// If [clean] is true, this will delete the playground.  Otherwise,
+  /// if it exists it will assume it is properly constructed.
+  Playground(this.playgroundPath, bool clean) {
+    Directory playground = Directory(playgroundPath);
+    if (clean) {
+      if (playground.existsSync()) {
+        playground.deleteSync(recursive: true);
+      }
+    }
+    if (!playground.existsSync()) playground.createSync();
+  }
+
+  /// Build an environment for subprocesses.
+  Map<String, String> get env => {'PUB_CACHE': pubCachePath};
+
+  String get pubCachePath => path.join(playgroundPath, '.pub-cache');
+}
+
+/// Abstraction for a package fetched via pub.
+class PubPackage extends Package {
+  PubPackage(String name, [String? version]) : super(name) {
+    throw UnimplementedError();
+  }
+
+  @override
+  // TODO: implement packagePath
+  List<String> get migrationPaths => throw UnimplementedError();
+}
+
 /// Abstraction for compiled Dart SDKs (not this repository).
 class Sdk {
   /// The root of the compiled SDK.
@@ -215,3 +191,28 @@ class Sdk {
     this.sdkPath = path.canonicalize(sdkPath);
   }
 }
+
+/// Abstraction for a package located within pkg or third_party/pkg.
+class SdkPackage extends Package {
+  /// Where to find packages.  Constructor searches in-order.
+  static final List<String> _searchPaths = [
+    'pkg',
+    path.join('third_party', 'pkg'),
+  ];
+
+  late final String _packagePath;
+
+  SdkPackage(String name) : super(name) {
+    for (String potentialPath
+        in _searchPaths.map((p) => path.join(thisSdkRepo, p, name))) {
+      if (Directory(potentialPath).existsSync()) {
+        _packagePath = potentialPath;
+      }
+    }
+  }
+  @override
+  List<String> get migrationPaths => [_packagePath];
+
+  @override
+  String toString() => path.relative(_packagePath, from: thisSdkRepo);
+}