Show dependency type changes in report (#3954)

lib/src/entrypoint.dart

@@ -403,8 +403,7 @@ Try running `$topLevelProgram pub get` to create `$lockFilePath`.''');
       quiet: summaryOnly,
     );
 
-    final hasChanges = await report.show();
-    await report.summarize();
+    final hasChanges = await report.show(summary: true);
     if (enforceLockfile && hasChanges) {
       dataError('''
 Unable to satisfy `$pubspecPath` using `$lockFilePath`$suffix.

lib/src/global_packages.dart

@@ -189,7 +189,8 @@ class GlobalPackages {
     final tempDir = cache.createTempDir();
     // TODO(rnystrom): Look in "bin" and display list of binaries that
     // user can run.
-    LockFile([id]).writeToFile(p.join(tempDir, 'pubspec.lock'), cache);
+    LockFile([id], mainDependencies: {id.name})
+        .writeToFile(p.join(tempDir, 'pubspec.lock'), cache);
 
     tryDeleteEntry(_packageDir(name));
     tryRenameDir(tempDir, _packageDir(name));
@@ -270,7 +271,7 @@ To recompile executables, first run `$topLevelProgram pub global deactivate $nam
           dryRun: false,
           quiet: false,
           enforceLockfile: false,
-        ).show();
+        ).show(summary: false);
       }
 
       final tempDir = cache.createTempDir();

lib/src/lock_file.dart

@@ -223,9 +223,9 @@ class LockFile {
     return LockFile._(
       packages,
       sdkConstraints,
-      const UnmodifiableSetView.empty(),
-      const UnmodifiableSetView.empty(),
-      const UnmodifiableSetView.empty(),
+      mainDependencies,
+      devDependencies,
+      overriddenDependencies,
     );
   }
 

lib/src/solver/report.dart

@@ -63,12 +63,16 @@ class SolveReport {
   /// Displays a report of the results of the version resolution in
   /// [_newLockFile] relative to the [_previousLockFile] file.
   ///
+  /// If [summary] is `true` a count of changes and number of
+  /// discontinued/retracted packages will be shown at the end of the report.
+  ///
   /// Returns `true` if there was any change of dependencies relative to the old
   /// lockfile.
-  Future<bool> show() async {
-    final hasChanges = await _reportChanges();
+  Future<bool> show({required bool summary}) async {
+    final changes = await _reportChanges();
     _checkContentHashesMatchOldLockfile();
-    return hasChanges;
+    if (summary) await summarize(changes);
+    return changes != 0;
   }
 
   void _checkContentHashesMatchOldLockfile() {
@@ -139,24 +143,12 @@ $contentHashesDocumentationUrl
   /// If [type] is `SolveType.UPGRADE` it also shows the number of packages that
   /// are not at the latest available version and the number of outdated
   /// packages.
-  Future<void> summarize() async {
+  Future<void> summarize(int changes) async {
     // Count how many dependencies actually changed.
     var dependencies = _newLockFile.packages.keys.toSet();
     dependencies.addAll(_previousLockFile.packages.keys);
     dependencies.remove(_rootPubspec.name);
 
-    var numChanged = dependencies.where((name) {
-      var oldId = _previousLockFile.packages[name];
-      var newId = _newLockFile.packages[name];
-
-      // Added or removed dependencies count.
-      if (oldId == null) return true;
-      if (newId == null) return true;
-
-      // The dependency existed before, so see if it was modified.
-      return oldId != newId;
-    }).length;
-
     var suffix = '';
     final dir = _location;
     if (dir != null) {
@@ -169,40 +161,40 @@ $contentHashesDocumentationUrl
       if (_dryRun) {
         log.message('Would get dependencies$suffix.');
       } else if (_enforceLockfile) {
-        if (numChanged == 0) {
+        if (changes == 0) {
           log.message('Got dependencies$suffix.');
         }
       } else {
         log.message('Got dependencies$suffix.');
       }
     } else {
       if (_dryRun) {
-        if (numChanged == 0) {
+        if (changes == 0) {
           log.message('No dependencies would change$suffix.');
-        } else if (numChanged == 1) {
-          log.message('Would change $numChanged dependency$suffix.');
+        } else if (changes == 1) {
+          log.message('Would change $changes dependency$suffix.');
         } else {
-          log.message('Would change $numChanged dependencies$suffix.');
+          log.message('Would change $changes dependencies$suffix.');
         }
       } else if (_enforceLockfile) {
-        if (numChanged == 0) {
+        if (changes == 0) {
           log.message('Got dependencies$suffix!');
-        } else if (numChanged == 1) {
-          log.message('Would change $numChanged dependency$suffix.');
+        } else if (changes == 1) {
+          log.message('Would change $changes dependency$suffix.');
         } else {
-          log.message('Would change $numChanged dependencies$suffix.');
+          log.message('Would change $changes dependencies$suffix.');
         }
       } else {
-        if (numChanged == 0) {
+        if (changes == 0) {
           if (_type == SolveType.get) {
             log.message('Got dependencies$suffix!');
           } else {
             log.message('No dependencies changed$suffix.');
           }
-        } else if (numChanged == 1) {
-          log.message('Changed $numChanged dependency$suffix!');
+        } else if (changes == 1) {
+          log.message('Changed $changes dependency$suffix!');
         } else {
-          log.message('Changed $numChanged dependencies$suffix!');
+          log.message('Changed $changes dependencies$suffix!');
         }
       }
       await reportDiscontinued();
@@ -213,16 +205,16 @@ $contentHashesDocumentationUrl
   /// Displays a report of all of the previous and current dependencies and
   /// how they have changed.
   ///
-  /// Returns true if anything changed.
-  Future<bool> _reportChanges() async {
+  /// Returns the number of changes.
+  Future<int> _reportChanges() async {
     final output = StringBuffer();
     // Show the new set of dependencies ordered by name.
     var names = _newLockFile.packages.keys.toList();
     names.remove(_rootPubspec.name);
     names.sort();
-    var hasChanges = false;
+    var changes = 0;
     for (final name in names) {
-      hasChanges |= await _reportPackage(name, output);
+      changes += await _reportPackage(name, output) ? 1 : 0;
     }
     // Show any removed ones.
     var removed = _previousLockFile.packages.keys.toSet();
@@ -232,12 +224,12 @@ $contentHashesDocumentationUrl
       output.writeln('These packages are no longer being depended on:');
       for (var name in ordered(removed)) {
         await _reportPackage(name, output, alwaysShow: true);
+        changes += 1;
       }
-      hasChanges = true;
     }
 
     message(output.toString());
-    return hasChanges;
+    return changes;
   }
 
   /// Displays a single-line message, number of discontinued packages
@@ -290,6 +282,13 @@ $contentHashesDocumentationUrl
     }
   }
 
+  static DependencyType dependencyType(LockFile lockFile, String name) =>
+      lockFile.mainDependencies.contains(name)
+          ? DependencyType.direct
+          : lockFile.devDependencies.contains(name)
+              ? DependencyType.dev
+              : DependencyType.none;
+
   /// Reports the results of the upgrade on the package named [name].
   ///
   /// If [alwaysShow] is true, the package is reported even if it didn't change,
@@ -405,9 +404,17 @@ $contentHashesDocumentationUrl
       }
     }
 
+    final oldDependencyType = dependencyType(_previousLockFile, name);
+    final newDependencyType = dependencyType(_newLockFile, name);
+
+    var dependencyTypeChanged = oldId != null &&
+        newId != null &&
+        oldDependencyType != newDependencyType;
+
     if (!(alwaysShow ||
         changed ||
         addedOrRemoved ||
+        dependencyTypeChanged ||
         message != null ||
         isOverridden)) {
       return changed || addedOrRemoved;
@@ -425,6 +432,18 @@ $contentHashesDocumentationUrl
       output.write(')');
     }
 
+    if (dependencyTypeChanged) {
+      if (dependencyTypeChanged) {
+        output.write(' (from ');
+
+        _writeDependencyType(oldDependencyType, output);
+        output.write(' dependency to ');
+
+        _writeDependencyType(newDependencyType, output);
+        output.write(' dependency)');
+      }
+    }
+
     // Highlight overridden packages.
     if (isOverridden) {
       final location = _location;
@@ -438,7 +457,7 @@ $contentHashesDocumentationUrl
     if (message != null) output.write(' ${log.cyan(message)}');
 
     output.writeln();
-    return changed || addedOrRemoved;
+    return changed || addedOrRemoved || dependencyTypeChanged;
   }
 
   /// Writes a terse description of [id] (not including its name) to the output.
@@ -451,6 +470,18 @@ $contentHashesDocumentationUrl
     }
   }
 
+  void _writeDependencyType(DependencyType t, StringBuffer output) {
+    output.write(
+      log.bold(
+        switch (t) {
+          DependencyType.direct => 'direct',
+          DependencyType.dev => 'dev',
+          DependencyType.none => 'transitive',
+        },
+      ),
+    );
+  }
+
   void warning(String message) {
     if (_quiet) {
       log.fine(message);

test/add/common/add_test.dart

@@ -269,20 +269,66 @@ environment:
       await d.dir(appPath, [
         d.file('pubspec.yaml', '''
 name: myapp
-dependencies: 
+dependencies:
 
 dev_dependencies:
   foo: 1.2.2
 environment:
   sdk: '$defaultSdkConstraint'
 '''),
       ]).create();
+      await pubGet();
+      await pubAdd(
+        args: ['foo:1.2.3'],
+        output: allOf(
+          contains('"foo" was found in dev_dependencies. Removing "foo" and '
+              'adding it to dependencies instead.'),
+          contains(
+            '> foo 1.2.3 (was 1.2.2) (from dev dependency to direct dependency)',
+          ),
+        ),
+      );
+
+      await d.cacheDir({'foo': '1.2.3'}).validate();
+      await d.appPackageConfigFile([
+        d.packageConfigEntry(name: 'foo', version: '1.2.3'),
+      ]).validate();
+
+      await d.dir(appPath, [
+        d.pubspec({
+          'name': 'myapp',
+          'dependencies': {'foo': '1.2.3'},
+        }),
+      ]).validate();
+    });
+
+    test('changing from a dev to non-dev_dependency is considered a change',
+        () async {
+      (await servePackages()).serve('foo', '1.2.3');
+
+      await d.dir(appPath, [
+        d.file('pubspec.yaml', '''
+name: myapp
+dependencies:
+
+dev_dependencies:
+  foo: 1.2.3
+environment:
+  sdk: '$defaultSdkConstraint'
+'''),
+      ]).create();
+      await pubGet();
 
       await pubAdd(
         args: ['foo:1.2.3'],
-        output:
-            contains('"foo" was found in dev_dependencies. Removing "foo" and '
-                'adding it to dependencies instead.'),
+        output: allOf(
+          contains('"foo" was found in dev_dependencies. Removing "foo" and '
+              'adding it to dependencies instead.'),
+          contains(
+            ' foo 1.2.3 (from dev dependency to direct dependency)',
+          ),
+          contains('Changed 1 dependency!'),
+        ),
       );
 
       await d.cacheDir({'foo': '1.2.3'}).validate();