Validate pubspec names after resolution, instead of when parsing (#3956)

lib/src/entrypoint.dart

@@ -87,7 +87,6 @@ class Entrypoint {
   ///
   /// For a global package, this is the activated package.
   Package get root => _root ??= Package.load(
-        null,
         rootDir,
         cache.sources,
         withPubspecOverrides: true,
@@ -178,7 +177,6 @@ class Entrypoint {
     var packages = {
       for (var packageEntry in packageConfig.nonInjectedPackages)
         packageEntry.name: Package.load(
-          packageEntry.name,
           packageEntry.resolvedRootDir(packageConfigPath),
           cache.sources,
         ),

lib/src/package.dart

@@ -116,22 +116,20 @@ class Package {
 
   /// Loads the package whose root directory is [packageDir].
   ///
-  /// [name] is the expected name of that package (e.g. the name given in the
-  /// dependency), or `null` if the package being loaded is the entrypoint
-  /// package.
+  /// [expectedName] is the expected name of that package (e.g. the name given
+  /// in the dependency), or `null` if the package being loaded is the
+  /// entrypoint package.
   ///
   /// `pubspec_overrides.yaml` is only loaded if [withPubspecOverrides] is
   /// `true`.
   factory Package.load(
-    String? name,
     String dir,
     SourceRegistry sources, {
     bool withPubspecOverrides = false,
   }) {
     final pubspec = Pubspec.load(
       dir,
       sources,
-      expectedName: name,
       allowOverridesFile: withPubspecOverrides,
     );
     return Package._(dir, pubspec);

lib/src/pubspec.dart

@@ -207,15 +207,11 @@ class Pubspec extends PubspecBase {
 
   /// Loads the pubspec for a package located in [packageDir].
   ///
-  /// If [expectedName] is passed and the pubspec doesn't have a matching name
-  /// field, this will throw a [SourceSpanApplicationException].
-  ///
   /// If [allowOverridesFile] is `true` [pubspecOverridesFilename] is loaded and
   /// is allowed to override dependency_overrides from `pubspec.yaml`.
   factory Pubspec.load(
     String packageDir,
     SourceRegistry sources, {
-    String? expectedName,
     bool allowOverridesFile = false,
   }) {
     var pubspecPath = path.join(packageDir, pubspecYamlFilename);
@@ -237,7 +233,6 @@ class Pubspec extends PubspecBase {
     return Pubspec.parse(
       readTextFile(pubspecPath),
       sources,
-      expectedName: expectedName,
       location: path.toUri(pubspecPath),
       overridesFileContents: overridesFileContents,
       overridesLocation: path.toUri(overridesPath),
@@ -278,15 +273,11 @@ class Pubspec extends PubspecBase {
   /// Returns a Pubspec object for an already-parsed map representing its
   /// contents.
   ///
-  /// If [expectedName] is passed and the pubspec doesn't have a matching name
-  /// field, this will throw a [PubspecError].
-  ///
   /// [location] is the location from which this pubspec was loaded.
   Pubspec.fromMap(
     Map fields,
     this._sources, {
     YamlMap? overridesFields,
-    String? expectedName,
     Uri? location,
   })  : _overridesFileFields = overridesFields,
         _includeDefaultSdkConstraint = true,
@@ -297,18 +288,7 @@ class Pubspec extends PubspecBase {
           fields is YamlMap
               ? fields
               : YamlMap.wrap(fields, sourceUrl: location),
-        ) {
-    // If [expectedName] is passed, ensure that the actual 'name' field exists
-    // and matches the expectation.
-    if (expectedName == null) return;
-    if (name == expectedName) return;
-
-    throw SourceSpanApplicationException(
-      '"name" field doesn\'t match expected name '
-      '"$expectedName".',
-      this.fields.nodes['name']!.span,
-    );
-  }
+        );
 
   /// Parses the pubspec stored at [location] whose text is [contents].
   ///
@@ -317,7 +297,6 @@ class Pubspec extends PubspecBase {
   factory Pubspec.parse(
     String contents,
     SourceRegistry sources, {
-    String? expectedName,
     Uri? location,
     String? overridesFileContents,
     Uri? overridesLocation,
@@ -339,7 +318,6 @@ class Pubspec extends PubspecBase {
       pubspecMap,
       sources,
       overridesFields: overridesFileMap,
-      expectedName: expectedName,
       location: location,
     );
   }

lib/src/solver/result.dart

@@ -4,7 +4,9 @@
 
 import 'package:collection/collection.dart';
 import 'package:pub_semver/pub_semver.dart';
+import 'package:yaml/yaml.dart';
 
+import '../exceptions.dart';
 import '../http.dart';
 import '../io.dart';
 import '../lock_file.dart';
@@ -76,6 +78,7 @@ class SolveResult {
         return id;
       }),
     );
+    await _validatePubspecNames(cache);
 
     // Invariant: the content-hashes in PUB_CACHE matches those provided by the
     // server.
@@ -107,6 +110,39 @@ class SolveResult {
     );
   }
 
+  /// Validate that all dependencies in [packages] refer to a package that has the
+  /// expected name.
+  ///
+  /// Throws an ApplicationException if there is a mismatch.
+  Future<void> _validatePubspecNames(
+    SystemCache cache,
+  ) async {
+    for (final id in packages) {
+      final pubspec = pubspecs[id.name]!;
+      final validatedNames = <String>{};
+      for (final dependency in [
+        ...pubspec.dependencies.keys,
+        if (id.isRoot) ...pubspec.devDependencies.keys,
+      ]) {
+        if (!validatedNames.add(dependency)) continue;
+        final dependencyPubspec = pubspecs[dependency]!;
+
+        if (dependencyPubspec.name != dependency) {
+          // Find the span for the reference.
+          final key = pubspec.dependencies.containsKey(dependency)
+              ? 'dependencies'
+              : 'dev_dependencies';
+          final dependencyNode = (pubspec.fields.nodes[key] as YamlMap)
+              .nodes[dependency] as YamlNode;
+          throw SourceSpanApplicationException(
+            'Expected to find package "${log.bold(dependency)}", found package "${log.bold(dependencyPubspec.name)}".',
+            dependencyNode.span,
+          );
+        }
+      }
+    }
+  }
+
   final LockFile _previousLockFile;
 
   /// Returns the names of all packages that were changed.

lib/src/source/cached.dart

@@ -29,7 +29,7 @@ abstract class CachedSource extends Source {
   Future<Pubspec> doDescribe(PackageId id, SystemCache cache) async {
     var packageDir = getDirectoryInCache(id, cache);
     if (fileExists(path.join(packageDir, 'pubspec.yaml'))) {
-      return Pubspec.load(packageDir, cache.sources, expectedName: id.name);
+      return Pubspec.load(packageDir, cache.sources);
     }
 
     return await describeUncached(id, cache);

lib/src/source/git.dart

@@ -336,7 +336,6 @@ class GitSource extends CachedSource {
         cache,
       ),
       cache.sources,
-      expectedName: ref.name,
     );
   }
 
@@ -428,7 +427,7 @@ class GitSource extends CachedSource {
 
             var packageDir = p.join(revisionCachePath, relative);
             try {
-              return Package.load(null, packageDir, cache.sources);
+              return Package.load(packageDir, cache.sources);
             } catch (error, stackTrace) {
               log.error('Failed to load package', error, stackTrace);
               var name = p.basename(revisionCachePath).split('-').first;

lib/src/source/hosted.dart

@@ -396,7 +396,6 @@ class HostedSource extends CachedSource {
       var pubspec = Pubspec.fromMap(
         pubspecData,
         cache.sources,
-        expectedName: ref.name,
         location: location,
       );
       final archiveSha256 = map['archive_sha256'];
@@ -954,7 +953,7 @@ class HostedSource extends CachedSource {
         var packages = <Package>[];
         for (var entry in listDir(serverDir)) {
           try {
-            packages.add(Package.load(null, entry, cache.sources));
+            packages.add(Package.load(entry, cache.sources));
           } catch (error, stackTrace) {
             log.error('Failed to load package', error, stackTrace);
             final id = _idForBasename(
@@ -1061,7 +1060,7 @@ class HostedSource extends CachedSource {
         .where(_looksLikePackageDir)
         .map((entry) {
           try {
-            return Package.load(null, entry, cache.sources);
+            return Package.load(entry, cache.sources);
           } catch (error, stackTrace) {
             log.fine('Failed to load package from $entry:\n'
                 '$error\n'

lib/src/source/path.dart

@@ -168,7 +168,7 @@ class PathSource extends Source {
       throw ArgumentError('Wrong source');
     }
     var dir = _validatePath(ref.name, description);
-    return Pubspec.load(dir, cache.sources, expectedName: ref.name);
+    return Pubspec.load(dir, cache.sources);
   }
 
   @override

lib/src/source/sdk.dart

@@ -89,11 +89,8 @@ class SdkSource extends Source {
   ///
   /// Throws a [PackageNotFoundException] if [ref]'s SDK is unavailable or
   /// doesn't contain the package.
-  Pubspec _loadPubspec(PackageRef ref, SystemCache cache) => Pubspec.load(
-        _verifiedPackagePath(ref),
-        cache.sources,
-        expectedName: ref.name,
-      );
+  Pubspec _loadPubspec(PackageRef ref, SystemCache cache) =>
+      Pubspec.load(_verifiedPackagePath(ref), cache.sources);
 
   /// Returns the path for the given [ref].
   ///

lib/src/system_cache.dart

@@ -112,17 +112,13 @@ Consider setting the `PUB_CACHE` variable manually.
   ///
   /// Throws an [ArgumentError] if [id] has an invalid source.
   Package load(PackageId id) {
-    return Package.load(id.name, getDirectory(id), sources);
+    return Package.load(getDirectory(id), sources);
   }
 
   Package loadCached(PackageId id) {
     final source = id.description.description.source;
     if (source is CachedSource) {
-      return Package.load(
-        id.name,
-        source.getDirectoryInCache(id, this),
-        sources,
-      );
+      return Package.load(source.getDirectoryInCache(id, this), sources);
     } else {
       throw ArgumentError('Call only on Cached ids.');
     }

test/ascii_tree_test.dart

@@ -61,11 +61,9 @@ void main() {
         file('path.dart', bytes(100)),
       ]),
     ]).create();
-    var files = Package.load(
-      null,
-      path(appPath),
-      (name) => throw UnimplementedError(),
-    ).listFiles();
+    var files =
+        Package.load(path(appPath), (name) => throw UnimplementedError())
+            .listFiles();
     ctx.expectNextSection(
       tree.fromFiles(files, baseDir: path(appPath), showFileSizes: true),
     );

test/directory_option_test.dart

@@ -50,7 +50,7 @@ main() => print('Hi');
         pubspec({
           'name': 'example',
           'dependencies': {
-            'myapp': {'path': '../'}, // Wrong name of dependency
+            'test_pkg': {'path': '../'},
           },
         }),
       ]),

test/get/git/dependency_name_match_pubspec_test.dart

@@ -28,8 +28,9 @@ void main() {
     ]).create();
 
     await pubGet(
-      error: contains('"name" field doesn\'t match expected name '
-          '"weirdname".'),
+      error: contains(
+        'Expected to find package "weirdname", found package "foo".',
+      ),
       exitCode: exit_codes.DATA,
     );
   });

test/pubspec_test.dart

@@ -15,9 +15,6 @@ void main() {
   group('parse()', () {
     final sources = SystemCache().sources;
 
-    var throwsPubspecException =
-        throwsA(const TypeMatcher<SourceSpanApplicationException>());
-
     void expectPubspecException(
       String contents,
       void Function(Pubspec) fn, [
@@ -41,24 +38,6 @@ void main() {
       Pubspec.parse('version: not a semver', sources);
     });
 
-    test(
-        "eagerly throws an error if the pubspec name doesn't match the "
-        'expected name', () {
-      expect(
-        () => Pubspec.parse('name: foo', sources, expectedName: 'bar'),
-        throwsPubspecException,
-      );
-    });
-
-    test(
-        "eagerly throws an error if the pubspec doesn't have a name and an "
-        'expected name is passed', () {
-      expect(
-        () => Pubspec.parse('{}', sources, expectedName: 'bar'),
-        throwsPubspecException,
-      );
-    });
-
     test('allows a version constraint for dependencies', () {
       var pubspec = Pubspec.parse(
         '''

test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt

@@ -59,12 +59,9 @@ Got dependencies in myapp/example!
 ## Section 6
 $ pub get bar -C 'myapp/example2'
 Resolving dependencies in myapp/example2...
-[STDERR] Error on line 1, column 9 of myapp/pubspec.yaml: "name" field doesn't match expected name "myapp".
-[STDERR]   ╷
-[STDERR] 1 │ {"name":"test_pkg","version":"1.0.0","homepage":"https://pub.dev","description":"A package, I guess.","environment":{"sdk":">=3.1.2 <=3.2.0"}, dependencies: { foo: ^1.0.0}}
-[STDERR]   │         ^^^^^^^^^^
-[STDERR]   ╵
-[EXIT CODE] 65
++ foo 1.0.0
++ test_pkg 1.0.0 from path myapp
+Changed 2 dependencies in myapp/example2!
 
 -------------------------------- END OF OUTPUT ---------------------------------
 

test/upgrade/git/upgrade_to_incompatible_pubspec_test.dart

@@ -42,7 +42,7 @@ void main() {
     ).commit();
 
     await pubUpgrade(
-      error: contains('"name" field doesn\'t match expected name "foo".'),
+      error: contains('Expected to find package "foo", found package "zoo".'),
       exitCode: exit_codes.DATA,
     );