Represent Pubspec.dependencies as a map (#1743)

This cleans up some existing code, but more importantly it makes some
algorithms for the new version solver more efficient.

Author: nex3

lib/src/barback/dependency_computer.dart

@@ -194,7 +194,7 @@ class DependencyComputer {
               packageName == _graph.entrypoint.root.name
           ? package.immediateDependencies
           : package.dependencies;
-      for (var dep in dependencies) {
+      for (var dep in dependencies.values) {
         try {
           traversePackage(dep.name);
         } on CycleException catch (error) {
@@ -355,7 +355,7 @@ class _PackageDependencyComputer {
         return _applicableTransformers
             .map((config) => config.id)
             .toSet()
-            .union(unionAll(dependencies.map((dep) {
+            .union(unionAll(dependencies.values.map((dep) {
           try {
             return _dependencyComputer._transformersNeededByPackage(dep.name);
           } on CycleException catch (error) {

lib/src/cached_package.dart

@@ -82,9 +82,10 @@ class _CachedPubspec implements Pubspec {
   YamlMap get fields => _inner.fields;
   String get name => _inner.name;
   Version get version => _inner.version;
-  List<PackageRange> get dependencies => _inner.dependencies;
-  List<PackageRange> get devDependencies => _inner.devDependencies;
-  List<PackageRange> get dependencyOverrides => _inner.dependencyOverrides;
+  Map<String, PackageRange> get dependencies => _inner.dependencies;
+  Map<String, PackageRange> get devDependencies => _inner.devDependencies;
+  Map<String, PackageRange> get dependencyOverrides =>
+      _inner.dependencyOverrides;
   Map<String, Feature> get features => _inner.features;
   VersionConstraint get dartSdkConstraint => _inner.dartSdkConstraint;
   VersionConstraint get originalDartSdkConstraint =>

lib/src/command/build.dart

@@ -223,8 +223,8 @@ class BuildCommand extends BarbackCommand {
   /// directories next to each entrypoint in [entrypoints].
   Future _copyBrowserJsFiles(Iterable<AssetId> entrypoints, AssetSet assets) {
     // Must depend on the browser package.
-    if (!entrypoint.root.immediateDependencies
-        .any((dep) => dep.name == 'browser' && dep.source is HostedSource)) {
+    var browser = entrypoint.root.immediateDependencies['browser'];
+    if (browser == null || browser.source is! HostedSource) {
       return new Future.value();
     }
 

lib/src/command/deps.dart

@@ -90,14 +90,12 @@ class DepsCommand extends PubCommand {
   /// line.
   void _outputCompact() {
     var root = entrypoint.root;
-    _outputCompactPackages(
-        "dependencies", root.dependencies.map((dep) => dep.name));
+    _outputCompactPackages("dependencies", root.dependencies.keys);
     if (_includeDev) {
-      _outputCompactPackages(
-          "dev dependencies", root.devDependencies.map((dep) => dep.name));
+      _outputCompactPackages("dev dependencies", root.devDependencies.keys);
     }
-    _outputCompactPackages("dependency overrides",
-        root.dependencyOverrides.map((dep) => dep.name));
+    _outputCompactPackages(
+        "dependency overrides", root.dependencyOverrides.keys);
 
     var transitive = _getTransitiveDependencies();
     _outputCompactPackages("transitive dependencies", transitive);
@@ -116,7 +114,7 @@ class DepsCommand extends PubCommand {
       if (package.dependencies.isEmpty) {
         _buffer.writeln();
       } else {
-        var depNames = package.dependencies.map((dep) => dep.name);
+        var depNames = package.dependencies.keys;
         var depsList = "[${depNames.join(' ')}]";
         _buffer.writeln(" ${log.gray(depsList)}");
       }
@@ -130,14 +128,11 @@ class DepsCommand extends PubCommand {
   /// shown.
   void _outputList() {
     var root = entrypoint.root;
-    _outputListSection(
-        "dependencies", root.dependencies.map((dep) => dep.name));
+    _outputListSection("dependencies", root.dependencies.keys);
     if (_includeDev) {
-      _outputListSection(
-          "dev dependencies", root.devDependencies.map((dep) => dep.name));
+      _outputListSection("dev dependencies", root.devDependencies.keys);
     }
-    _outputListSection("dependency overrides",
-        root.dependencyOverrides.map((dep) => dep.name));
+    _outputListSection("dependency overrides", root.dependencyOverrides.keys);
 
     var transitive = _getTransitiveDependencies();
     if (transitive.isEmpty) return;
@@ -156,7 +151,7 @@ class DepsCommand extends PubCommand {
       var package = _getPackage(name);
       _buffer.writeln("- ${_labelPackage(package)}");
 
-      for (var dep in package.dependencies) {
+      for (var dep in package.dependencies.values) {
         _buffer
             .writeln("  - ${log.bold(dep.name)} ${log.gray(dep.constraint)}");
       }
@@ -178,12 +173,13 @@ class DepsCommand extends PubCommand {
 
     // Start with the root dependencies.
     var packageTree = <String, Map>{};
-    var immediateDependencies = entrypoint.root.immediateDependencies.toSet();
+    var immediateDependencies =
+        entrypoint.root.immediateDependencies.keys.toSet();
     if (!_includeDev) {
-      immediateDependencies.removeAll(entrypoint.root.devDependencies);
+      immediateDependencies.removeAll(entrypoint.root.devDependencies.keys);
     }
-    for (var dep in immediateDependencies) {
-      toWalk.add(new Pair(_getPackage(dep.name), packageTree));
+    for (var name in immediateDependencies) {
+      toWalk.add(new Pair(_getPackage(name), packageTree));
     }
 
     // Do a breadth-first walk to the dependency graph.
@@ -203,7 +199,7 @@ class DepsCommand extends PubCommand {
       var childMap = <String, Map>{};
       map[_labelPackage(package)] = childMap;
 
-      for (var dep in package.dependencies) {
+      for (var dep in package.dependencies.values) {
         toWalk.add(new Pair(_getPackage(dep.name), childMap));
       }
     }
@@ -219,22 +215,21 @@ class DepsCommand extends PubCommand {
     var transitive = _getAllDependencies();
     var root = entrypoint.root;
     transitive.remove(root.name);
-    transitive.removeAll(root.dependencies.map((dep) => dep.name));
+    transitive.removeAll(root.dependencies.keys);
     if (_includeDev) {
-      transitive.removeAll(root.devDependencies.map((dep) => dep.name));
+      transitive.removeAll(root.devDependencies.keys);
     }
-    transitive.removeAll(root.dependencyOverrides.map((dep) => dep.name));
+    transitive.removeAll(root.dependencyOverrides.keys);
     return transitive;
   }
 
   Set<String> _getAllDependencies() {
     if (_includeDev) return entrypoint.packageGraph.packages.keys.toSet();
 
-    var nonDevDependencies = entrypoint.root.dependencies.toList()
-      ..addAll(entrypoint.root.dependencyOverrides);
+    var nonDevDependencies = entrypoint.root.dependencies.keys.toList()
+      ..addAll(entrypoint.root.dependencyOverrides.keys);
     return nonDevDependencies
-        .expand(
-            (dep) => entrypoint.packageGraph.transitiveDependencies(dep.name))
+        .expand((name) => entrypoint.packageGraph.transitiveDependencies(name))
         .map((package) => package.name)
         .toSet();
   }
@@ -260,7 +255,8 @@ class DepsCommand extends PubCommand {
       ..addAll((_includeDev
               ? entrypoint.root.immediateDependencies
               : entrypoint.root.dependencies)
-          .map((dep) => entrypoint.packageGraph.packages[dep.name]));
+          .keys
+          .map((name) => entrypoint.packageGraph.packages[name]));
 
     for (var package in packages) {
       var executables = _getExecutablesFor(package);

lib/src/entrypoint.dart

@@ -6,6 +6,7 @@ import 'dart:async';
 import 'dart:io';
 
 import 'package:barback/barback.dart';
+import 'package:collection/collection.dart';
 import 'package:package_config/packages_file.dart' as packages_file;
 import 'package:path/path.dart' as p;
 import 'package:pub_semver/pub_semver.dart';
@@ -372,10 +373,8 @@ class Entrypoint {
   Future precompileExecutables({Iterable<String> changed}) async {
     _deleteExecutableSnapshots(changed: changed);
 
-    var executables = new Map<String, List<AssetId>>.fromIterable(
-        root.immediateDependencies,
-        key: (dep) => dep.name,
-        value: (dep) => _executablesForPackage(dep.name));
+    var executables = mapMap(root.immediateDependencies,
+        value: (name, _) => _executablesForPackage(name));
 
     for (var package in executables.keys.toList()) {
       if (executables[package].isEmpty) executables.remove(package);
@@ -609,12 +608,12 @@ class Entrypoint {
   /// or that don't match what's in there, this will throw a [DataError]
   /// describing the issue.
   void _assertLockFileUpToDate() {
-    if (!root.immediateDependencies.every(_isDependencyUpToDate)) {
+    if (!root.immediateDependencies.values.every(_isDependencyUpToDate)) {
       dataError('The pubspec.yaml file has changed since the pubspec.lock '
           'file was generated, please run "pub get" again.');
     }
 
-    var overrides = root.dependencyOverrides.map((dep) => dep.name).toSet();
+    var overrides = new MapKeySet(root.dependencyOverrides);
 
     // Check that uncached dependencies' pubspecs are also still satisfied,
     // since they're mutable and may have changed since the last get.
@@ -623,7 +622,7 @@ class Entrypoint {
       if (source is CachedSource) continue;
 
       try {
-        if (cache.load(id).dependencies.every((dep) =>
+        if (cache.load(id).dependencies.values.every((dep) =>
             overrides.contains(dep.name) || _isDependencyUpToDate(dep))) {
           continue;
         }

lib/src/executable.dart

@@ -39,8 +39,7 @@ Future<int> runExecutable(Entrypoint entrypoint, String package,
   // Make sure the package is an immediate dependency of the entrypoint or the
   // entrypoint itself.
   if (entrypoint.root.name != package &&
-      !entrypoint.root.immediateDependencies
-          .any((dep) => dep.name == package)) {
+      !entrypoint.root.immediateDependencies.containsKey(package)) {
     if (entrypoint.packageGraph.packages.containsKey(package)) {
       dataError('Package "$package" is not an immediate dependency.\n'
           'Cannot run executables in transitive dependencies.');

lib/src/package.dart

@@ -49,31 +49,24 @@ class Package {
   final Pubspec pubspec;
 
   /// The immediate dependencies this package specifies in its pubspec.
-  List<PackageRange> get dependencies => pubspec.dependencies;
+  Map<String, PackageRange> get dependencies => pubspec.dependencies;
 
   /// The immediate dev dependencies this package specifies in its pubspec.
-  List<PackageRange> get devDependencies => pubspec.devDependencies;
+  Map<String, PackageRange> get devDependencies => pubspec.devDependencies;
 
   /// The dependency overrides this package specifies in its pubspec.
-  List<PackageRange> get dependencyOverrides => pubspec.dependencyOverrides;
+  Map<String, PackageRange> get dependencyOverrides =>
+      pubspec.dependencyOverrides;
 
   /// All immediate dependencies this package specifies.
   ///
   /// This includes regular, dev dependencies, and overrides.
-  List<PackageRange> get immediateDependencies {
-    var deps = <String, PackageRange>{};
-
-    addToMap(dep) {
-      deps[dep.name] = dep;
-    }
-
-    dependencies.forEach(addToMap);
-    devDependencies.forEach(addToMap);
-
-    // Make sure to add these last so they replace normal dependencies.
-    dependencyOverrides.forEach(addToMap);
-
-    return deps.values.toList();
+  Map<String, PackageRange> get immediateDependencies {
+    // Make sure to add overrides last so they replace normal dependencies.
+    return {}
+      ..addAll(dependencies)
+      ..addAll(devDependencies)
+      ..addAll(dependencyOverrides);
   }
 
   /// Returns a list of asset ids for all Dart executables in this package's bin

lib/src/package_graph.dart

@@ -82,8 +82,7 @@ class PackageGraph {
     if (_transitiveDependencies == null) {
       var closure = transitiveClosure(
           mapMap<String, Package, String, Iterable<String>>(packages,
-              value: (_, package) =>
-                  package.dependencies.map((dep) => dep.name)));
+              value: (_, package) => package.dependencies.keys));
       _transitiveDependencies =
           mapMap<String, Set<String>, String, Set<Package>>(closure,
               value: (depender, names) {

lib/src/pubspec.dart

@@ -163,38 +163,38 @@ class Pubspec {
   Version _version;
 
   /// The additional packages this package depends on.
-  List<PackageRange> get dependencies {
+  Map<String, PackageRange> get dependencies {
     if (_dependencies != null) return _dependencies;
     _dependencies =
         _parseDependencies('dependencies', fields.nodes['dependencies']);
     return _dependencies;
   }
 
-  List<PackageRange> _dependencies;
+  Map<String, PackageRange> _dependencies;
 
   /// The packages this package depends on when it is the root package.
-  List<PackageRange> get devDependencies {
+  Map<String, PackageRange> get devDependencies {
     if (_devDependencies != null) return _devDependencies;
     _devDependencies = _parseDependencies(
         'dev_dependencies', fields.nodes['dev_dependencies']);
     return _devDependencies;
   }
 
-  List<PackageRange> _devDependencies;
+  Map<String, PackageRange> _devDependencies;
 
   /// The dependency constraints that this package overrides when it is the
   /// root package.
   ///
   /// Dependencies here will replace any dependency on a package with the same
   /// name anywhere in the dependency graph.
-  List<PackageRange> get dependencyOverrides {
+  Map<String, PackageRange> get dependencyOverrides {
     if (_dependencyOverrides != null) return _dependencyOverrides;
     _dependencyOverrides = _parseDependencies(
         'dependency_overrides', fields.nodes['dependency_overrides']);
     return _dependencyOverrides;
   }
 
-  List<PackageRange> _dependencyOverrides;
+  Map<String, PackageRange> _dependencyOverrides;
 
   Map<String, Feature> get features {
     if (_features != null) return _features;
@@ -235,7 +235,7 @@ class Pubspec {
 
           var sdkConstraints = _parseEnvironment(specNode);
 
-          return new Feature(nameNode.value, dependencies,
+          return new Feature(nameNode.value, dependencies.values,
               requires: requires,
               dartSdkConstraint: sdkConstraints.first,
               flutterSdkConstraint: sdkConstraints.last,
@@ -597,11 +597,16 @@ class Pubspec {
       Map fields,
       SourceRegistry sources})
       : _version = version,
-        _dependencies = dependencies == null ? null : dependencies.toList(),
-        _devDependencies =
-            devDependencies == null ? null : devDependencies.toList(),
-        _dependencyOverrides =
-            dependencyOverrides == null ? null : dependencyOverrides.toList(),
+        _dependencies = dependencies == null
+            ? null
+            : new Map.fromIterable(dependencies, key: (range) => range.name),
+        _devDependencies = devDependencies == null
+            ? null
+            : new Map.fromIterable(devDependencies, key: (range) => range.name),
+        _dependencyOverrides = dependencyOverrides == null
+            ? null
+            : new Map.fromIterable(dependencyOverrides,
+                key: (range) => range.name),
         _dartSdkConstraint =
             dartSdkConstraint ?? includeDefaultSdkConstraint == true
                 ? _defaultUpperBoundSdkConstraint
@@ -618,8 +623,8 @@ class Pubspec {
       : _sources = null,
         _name = null,
         _version = Version.none,
-        _dependencies = <PackageRange>[],
-        _devDependencies = <PackageRange>[],
+        _dependencies = {},
+        _devDependencies = {},
         _dartSdkConstraint = VersionConstraint.any,
         _flutterSdkConstraint = null,
         _includeDefaultSdkConstraint = false,
@@ -704,9 +709,9 @@ class Pubspec {
   }
 
   /// Parses the dependency field named [field], and returns the corresponding
-  /// list of dependencies.
-  List<PackageRange> _parseDependencies(String field, YamlNode node) {
-    var dependencies = <PackageRange>[];
+  /// map of dependency names to dependencies.
+  Map<String, PackageRange> _parseDependencies(String field, YamlNode node) {
+    var dependencies = <String, PackageRange>{};
 
     // Allow an empty dependencies key.
     if (node == null || node.value == null) return dependencies;
@@ -788,8 +793,8 @@ class Pubspec {
             containingPath: pubspecPath);
       });
 
-      dependencies
-          .add(ref.withConstraint(versionConstraint).withFeatures(features));
+      dependencies[name] =
+          ref.withConstraint(versionConstraint).withFeatures(features);
     });
 
     return dependencies;