Indicate the source of a dependency in the lockfile (#1722)

Indicate the source of a dependency in the lockfile

Closes #1721

Author: nex3

lib/src/lock_file.dart

@@ -4,6 +4,7 @@
 
 import 'dart:collection';
 
+import 'package:collection/collection.dart';
 import 'package:path/path.dart' as p;
 import 'package:package_config/packages_file.dart' as packages_file;
 import 'package:pub_semver/pub_semver.dart';
@@ -28,30 +29,62 @@ class LockFile {
   /// or `null` if no packages require the Flutter SDK.
   final VersionConstraint flutterSdkConstraint;
 
+  /// Dependency names that appeared in the root package's `dependencies`
+  /// section.
+  final Set<String> _mainDependencies;
+
+  /// Dependency names that appeared in the root package's `dev_dependencies`
+  /// section.
+  final Set<String> _devDependencies;
+
+  /// Dependency names that appeared in the root package's
+  /// `dependency_overrides` section.
+  final Set<String> _overriddenDependencies;
+
   /// Creates a new lockfile containing [ids].
   ///
   /// If passed, [dartSdkConstraint] represents the intersection of all Dart SDK
   /// constraints for all locked packages. It defaults to
   /// [VersionConstraint.any]. Similarly, [flutterSdkConstraint] represents the
   /// intersection of all Flutter SDK constraints; however, it defaults to
   /// `null`.
+  ///
+  /// If passed, [mainDependencies], [devDependencies], and
+  /// [overriddenDependencies] indicate which dependencies should be marked as
+  /// being listed in the main package's `dependencies`, `dev_dependencies`, and
+  /// `dependency_overrides` sections, respectively. These are consumed by the
+  /// analysis server to provide better auto-completion.
   LockFile(Iterable<PackageId> ids,
       {VersionConstraint dartSdkConstraint,
-      VersionConstraint flutterSdkConstraint})
+      VersionConstraint flutterSdkConstraint,
+      Set<String> mainDependencies,
+      Set<String> devDependencies,
+      Set<String> overriddenDependencies})
       : this._(
             new Map.fromIterable(ids.where((id) => !id.isRoot),
                 key: (id) => id.name),
             dartSdkConstraint ?? VersionConstraint.any,
-            flutterSdkConstraint);
-
-  LockFile._(Map<String, PackageId> packages, this.dartSdkConstraint,
-      this.flutterSdkConstraint)
+            flutterSdkConstraint,
+            mainDependencies ?? const UnmodifiableSetView.empty(),
+            devDependencies ?? const UnmodifiableSetView.empty(),
+            overriddenDependencies ?? const UnmodifiableSetView.empty());
+
+  LockFile._(
+      Map<String, PackageId> packages,
+      this.dartSdkConstraint,
+      this.flutterSdkConstraint,
+      this._mainDependencies,
+      this._devDependencies,
+      this._overriddenDependencies)
       : packages = new UnmodifiableMapView(packages);
 
   LockFile.empty()
       : packages = const {},
         dartSdkConstraint = VersionConstraint.any,
-        flutterSdkConstraint = null;
+        flutterSdkConstraint = null,
+        _mainDependencies = const UnmodifiableSetView.empty(),
+        _devDependencies = const UnmodifiableSetView.empty(),
+        _overriddenDependencies = const UnmodifiableSetView.empty();
 
   /// Loads a lockfile from [filePath].
   factory LockFile.load(String filePath, SourceRegistry sources) {
@@ -135,7 +168,13 @@ class LockFile {
       });
     }
 
-    return new LockFile._(packages, dartSdkConstraint, flutterSdkConstraint);
+    return new LockFile._(
+        packages,
+        dartSdkConstraint,
+        flutterSdkConstraint,
+        const UnmodifiableSetView.empty(),
+        const UnmodifiableSetView.empty(),
+        const UnmodifiableSetView.empty());
   }
 
   /// Asserts that [node] is a version constraint, and parses it.
@@ -170,18 +209,6 @@ class LockFile {
     throw new SourceSpanFormatException(message, node.span);
   }
 
-  /// Returns a copy of this LockFile with [id] added.
-  ///
-  /// If there's already an ID with the same name as [id] in the LockFile, it's
-  /// overwritten.
-  LockFile setPackage(PackageId id) {
-    if (id.isRoot) return this;
-
-    var packages = new Map<String, PackageId>.from(this.packages);
-    packages[id.name] = id;
-    return new LockFile._(packages, dartSdkConstraint, flutterSdkConstraint);
-  }
-
   /// Returns a copy of this LockFile with a package named [name] removed.
   ///
   /// Returns an identical [LockFile] if there's no package named [name].
@@ -190,7 +217,8 @@ class LockFile {
 
     var packages = new Map<String, PackageId>.from(this.packages);
     packages.remove(name);
-    return new LockFile._(packages, dartSdkConstraint, flutterSdkConstraint);
+    return new LockFile._(packages, dartSdkConstraint, flutterSdkConstraint,
+        _mainDependencies, _devDependencies, _overriddenDependencies);
   }
 
   /// Returns the contents of the `.packages` file generated from this lockfile.
@@ -228,7 +256,8 @@ class LockFile {
       packageMap[name] = {
         'version': package.version.toString(),
         'source': package.source.name,
-        'description': description
+        'description': description,
+        'dependency': _dependencyType(package.name)
       };
     });
 
@@ -244,4 +273,17 @@ class LockFile {
 ${yamlToString(data)}
 """;
   }
+
+  /// Returns the dependency classification for [package].
+  String _dependencyType(String package) {
+    if (_mainDependencies.contains(package)) return 'direct main';
+    if (_devDependencies.contains(package)) return 'direct dev';
+
+    // If a package appears in `dependency_overrides` and another dependency
+    // section, the main section it appears in takes precedence.
+    if (_overriddenDependencies.contains(package)) {
+      return 'direct overridden';
+    }
+    return 'transitive';
+  }
 }

lib/src/solver/version_solver.dart

@@ -99,7 +99,13 @@ class SolveResult {
         : new VersionConstraint.intersection(flutterConstraints);
 
     return new LockFile(packages,
-        dartSdkConstraint: dartMerged, flutterSdkConstraint: flutterMerged);
+        dartSdkConstraint: dartMerged,
+        flutterSdkConstraint: flutterMerged,
+        mainDependencies: _root.dependencies.map((range) => range.name).toSet(),
+        devDependencies:
+            _root.devDependencies.map((range) => range.name).toSet(),
+        overriddenDependencies:
+            _root.dependencyOverrides.map((range) => range.name).toSet());
   }
 
   final SourceRegistry _sources;

test/get/hosted/get_test.dart

@@ -2,9 +2,12 @@
 // 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:path/path.dart' as p;
 import 'package:test/test.dart';
+import 'package:yaml/yaml.dart';
 
 import 'package:pub/src/exit_codes.dart' as exit_codes;
+import 'package:pub/src/io.dart';
 
 import '../../descriptor.dart' as d;
 import '../../test_pub.dart';
@@ -53,4 +56,67 @@ main() {
     await d.cacheDir({"foo": "1.2.3"}, port: server.port).validate();
     await d.appPackagesFile({"foo": "1.2.3"}).validate();
   });
+
+  group('categorizes dependency types in the lockfile', () {
+    setUp(() => servePackages((builder) {
+          builder.serve("foo", "1.2.3", deps: {"bar": "any"});
+          builder.serve("bar", "1.2.3");
+          builder.serve("baz", "1.2.3", deps: {"qux": "any"});
+          builder.serve("qux", "1.2.3");
+          builder.serve("zip", "1.2.3", deps: {"zap": "any"});
+          builder.serve("zap", "1.2.3");
+        }));
+
+    test('for main, dev, and overridden dependencies', () async {
+      await d.dir(appPath, [
+        d.pubspec({
+          "name": "myapp",
+          "dependencies": {"foo": "any"},
+          "dev_dependencies": {"baz": "any"},
+          "dependency_overrides": {"zip": "any"}
+        })
+      ]).create();
+
+      await pubGet();
+
+      var packages = loadYaml(
+          readTextFile(p.join(d.sandbox, appPath, 'pubspec.lock')))['packages'];
+      expect(packages,
+          containsPair('foo', containsPair('dependency', 'direct main')));
+      expect(packages,
+          containsPair('bar', containsPair('dependency', 'transitive')));
+      expect(packages,
+          containsPair('baz', containsPair('dependency', 'direct dev')));
+      expect(packages,
+          containsPair('qux', containsPair('dependency', 'transitive')));
+      expect(packages,
+          containsPair('zip', containsPair('dependency', 'direct overridden')));
+      expect(packages,
+          containsPair('zap', containsPair('dependency', 'transitive')));
+    });
+
+    test('for overridden main and dev dependencies', () async {
+      await d.dir(appPath, [
+        d.pubspec({
+          "name": "myapp",
+          "dependencies": {"foo": "any"},
+          "dev_dependencies": {"baz": "any"},
+          "dependency_overrides": {"foo": "any", "baz": "any"}
+        })
+      ]).create();
+
+      await pubGet();
+
+      var packages = loadYaml(
+          readTextFile(p.join(d.sandbox, appPath, 'pubspec.lock')))['packages'];
+      expect(packages,
+          containsPair('foo', containsPair('dependency', 'direct main')));
+      expect(packages,
+          containsPair('bar', containsPair('dependency', 'transitive')));
+      expect(packages,
+          containsPair('baz', containsPair('dependency', 'direct dev')));
+      expect(packages,
+          containsPair('qux', containsPair('dependency', 'transitive')));
+    });
+  });
 }

test/lock_file_test.dart

@@ -244,7 +244,7 @@ packages:
         new PackageId(
             'foo', mockSource, new Version.parse('1.2.3'), 'foo desc'),
         new PackageId('bar', mockSource, new Version.parse('3.2.1'), 'bar desc')
-      ]);
+      ], devDependencies: ['bar'].toSet());
 
       expect(
           loadYaml(lockfile.serialize(null)),
@@ -254,12 +254,14 @@ packages:
               'foo': {
                 'version': '1.2.3',
                 'source': 'mock',
-                'description': 'foo desc'
+                'description': 'foo desc',
+                'dependency': 'transitive'
               },
               'bar': {
                 'version': '3.2.1',
                 'source': 'mock',
-                'description': 'bar desc'
+                'description': 'bar desc',
+                'dependency': 'direct dev'
               }
             }
           }));