Delay error reporting for dependencies

lib/src/pubspec.dart

@@ -14,6 +14,7 @@ import 'language_version.dart';
 import 'package_name.dart';
 import 'pubspec_parse.dart';
 import 'sdk.dart';
+import 'source/error.dart';
 import 'system_cache.dart';
 
 export 'pubspec_parse.dart' hide PubspecBase;
@@ -442,17 +443,28 @@ class Pubspec extends PubspecBase {
   /// Returns a list of most errors in this pubspec.
   ///
   /// This will return at most one error for each field.
-  List<SourceSpanApplicationException> get allErrors => _collectErrorsFor([
-        () => name,
-        () => version,
-        () => dependencies,
-        () => devDependencies,
-        () => publishTo,
-        () => executables,
-        () => falseSecrets,
-        () => sdkConstraints,
-        () => ignoredAdvisories,
-      ]);
+  List<SourceSpanApplicationException> get allErrors {
+    void errorsOf(Map<String, PackageRange> m) {
+      for (final PackageRange(description: description) in m.values) {
+        if (description is ErrorDescription) {
+          throw description.exception;
+        }
+      }
+    }
+
+    return _collectErrorsFor([
+      () => name,
+      () => version,
+      () => errorsOf(dependencies),
+      () => errorsOf(devDependencies),
+      () => errorsOf(dependencyOverrides),
+      () => publishTo,
+      () => executables,
+      () => falseSecrets,
+      () => sdkConstraints,
+      () => ignoredAdvisories,
+    ]);
+  }
 
   /// Returns the type of dependency from this package onto [name].
   DependencyType dependencyType(String? name) {
@@ -562,28 +574,34 @@ Map<String, PackageRange> _parseDependencies(
           specNode.span,
         );
       }
-
-      // Let the source validate the description.
-      var ref = _wrapFormatException(
-        'description',
-        descriptionNode?.span,
-        () {
-          String? pubspecDir;
-          if (location != null && _isFileUri(location)) {
-            pubspecDir = path.dirname(path.fromUri(location));
-          }
-
-          return sources(sourceName).parseRef(
-            name,
-            descriptionNode?.value,
-            containingDir: pubspecDir,
-            languageVersion: languageVersion,
-          );
-        },
-        packageName,
-        fileType,
-        targetPackage: name,
-      );
+      String? pubspecDir;
+      if (location != null && _isFileUri(location)) {
+        pubspecDir = path.dirname(path.fromUri(location));
+      }
+      PackageRef ref;
+      try {
+        // Let the source validate the description.
+        ref = sources(sourceName).parseRef(
+          name,
+          descriptionNode?.value,
+          containingDir: pubspecDir,
+          languageVersion: languageVersion,
+        );
+      } on FormatException catch (e) {
+        // If we already have a pub exception with a span, re-use that
+        if (e is SourceSpanApplicationException) rethrow;
+        final typeName = _fileTypeName(fileType);
+
+        final msg =
+            'Invalid description in the "$packageName" $typeName on the "$name" '
+            'dependency: ${e.message}';
+        ref = PackageRef(
+          name,
+          ErrorDescription(
+            SourceSpanApplicationException(msg, descriptionNode?.span),
+          ),
+        );
+      }
 
       dependencies[name] = ref.withConstraint(versionConstraint);
     },

lib/src/solver/package_lister.dart

@@ -15,6 +15,7 @@ import '../package.dart';
 import '../package_name.dart';
 import '../pubspec.dart';
 import '../sdk.dart';
+import '../source/error.dart';
 import '../system_cache.dart';
 import '../utils.dart';
 import 'incompatibility.dart';
@@ -253,7 +254,11 @@ class PackageLister {
               .where((range) => !_overriddenPackages.contains(range.name)),
         if (id.isRoot) ...pubspec.dependencyOverrides.values,
       ];
-
+      for (final PackageRange(description: description) in entries) {
+        if (description is ErrorDescription) {
+          throw description.exception;
+        }
+      }
       return entries.map((range) => _dependency(depender, range)).toList();
     }
 
@@ -274,12 +279,16 @@ class PackageLister {
 
     // Don't recompute dependencies that have already been emitted.
     var dependencies = Map<String, PackageRange>.from(pubspec.dependencies);
-    for (var package in dependencies.keys.toList()) {
+    for (final MapEntry(key: package, value: range)
+        in Map<String, PackageRange>.from(dependencies).entries) {
       if (_overriddenPackages.contains(package)) {
         dependencies.remove(package);
         continue;
       }
-
+      final description = range.description;
+      if (description is ErrorDescription) {
+        throw description.exception;
+      }
       var constraint = _alreadyListedDependencies[package];
       if (constraint != null && constraint.allows(id.version)) {
         dependencies.remove(package);

lib/src/source/error.dart

@@ -0,0 +1,40 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// 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 '../language_version.dart';
+import '../source.dart';
+import 'unknown.dart';
+
+/// Represents a bad dependency description.
+///
+/// Allows for postponing the error until the dependency is actually read.
+class ErrorDescription extends Description {
+  final Exception exception;
+
+  ErrorDescription(this.exception);
+
+  @override
+  String format() {
+    throw UnimplementedError();
+  }
+
+  @override
+  bool operator ==(Object other) {
+    return identical(other, this);
+  }
+
+  @override
+  int get hashCode => identityHashCode(this);
+
+  @override
+  Object? serializeForPubspec({
+    required String? containingDir,
+    required LanguageVersion languageVersion,
+  }) {
+    throw UnimplementedError();
+  }
+
+  @override
+  Source get source => UnknownSource('Error');
+}

lib/src/system_cache.dart

@@ -17,6 +17,7 @@ import 'package_name.dart';
 import 'pubspec.dart';
 import 'source.dart';
 import 'source/cached.dart';
+import 'source/error.dart';
 import 'source/git.dart';
 import 'source/hosted.dart';
 import 'source/path.dart';
@@ -185,6 +186,10 @@ Consider setting the `PUB_CACHE` variable manually.
     Duration? maxAge,
     Version? allowedRetractedVersion,
   }) async {
+    final description = ref.description;
+    if (description is ErrorDescription) {
+      throw description.exception;
+    }
     if (ref.isRoot) {
       throw ArgumentError('Cannot get versions for the root package.');
     }

test/dependency_override_test.dart

@@ -133,4 +133,23 @@ void main() {
       );
     });
   });
+
+  test('can override package with faulty description', () async {
+    final server = await servePackages();
+    server.serve(
+      'foo',
+      '1.0.0',
+      deps: {
+        'bar': {'path': '../abc'},
+      },
+    );
+    server.serve('bar', '1.0.0');
+    await d.appDir(
+      dependencies: {'foo': '^1.0.0'},
+      pubspec: {
+        'dependency_overrides': {'bar': '1.0.0'},
+      },
+    ).create();
+    await pubGet();
+  });
 }

test/pubspec_test.dart

@@ -5,7 +5,9 @@
 import 'dart:io';
 
 import 'package:pub/src/exceptions.dart';
+import 'package:pub/src/package_name.dart';
 import 'package:pub/src/pubspec.dart';
+import 'package:pub/src/source/error.dart';
 import 'package:pub/src/source/hosted.dart';
 import 'package:pub/src/system_cache.dart';
 import 'package:pub_semver/pub_semver.dart';
@@ -246,7 +248,7 @@ dependencies:
       name: foo
       url: '::'
 ''',
-        (pubspec) => pubspec.dependencies,
+        (pubspec) => errorsOf(pubspec.dependencies),
       );
     });
 
@@ -428,7 +430,7 @@ name: pkg
 dependencies:
   from_path: {path: non_local_path}
 ''',
-          (pubspec) => pubspec.dependencies,
+          (pubspec) => errorsOf(pubspec.dependencies),
           'Invalid description in the "pkg" pubspec on the "from_path" '
               'dependency: "non_local_path" is a relative path, but this isn\'t a '
               'local pubspec.');
@@ -603,7 +605,7 @@ dependencies:
     hosted:
       url: https://example.org/pub/
 ''',
-              (pubspec) => pubspec.dependencies,
+              (pubspec) => errorsOf(pubspec.dependencies),
               "The 'name' key must have a string value without a minimum Dart "
                   'SDK constraint of 2.15.');
         });
@@ -618,7 +620,7 @@ dependencies:
   foo:
     hosted: http://pub.example.org
 ''',
-              (pubspec) => pubspec.dependencies,
+              (pubspec) => errorsOf(pubspec.dependencies),
               'Using `hosted: <url>` is only supported with a minimum SDK constraint of 2.15.',
             );
           },
@@ -636,7 +638,7 @@ dependencies:
       url: git://github.com/dart-lang/foo
       path: 12
 ''',
-          (pubspec) => pubspec.dependencies,
+          (pubspec) => errorsOf(pubspec.dependencies),
         );
       });
 
@@ -649,7 +651,7 @@ dependencies:
       url: git://github.com/dart-lang/foo
       path: git://github.com/dart-lang/foo/bar
 ''',
-          (pubspec) => pubspec.dependencies,
+          (pubspec) => errorsOf(pubspec.dependencies),
         );
 
         expectPubspecException(
@@ -660,7 +662,7 @@ dependencies:
       url: git://github.com/dart-lang/foo
       path: /foo
 ''',
-          (pubspec) => pubspec.dependencies,
+          (pubspec) => errorsOf(pubspec.dependencies),
         );
       });
 
@@ -673,7 +675,7 @@ dependencies:
       url: git://github.com/dart-lang/foo
       path: foo/../../bar
 ''',
-          (pubspec) => pubspec.dependencies,
+          (pubspec) => errorsOf(pubspec.dependencies),
         );
       });
     });
@@ -1005,7 +1007,7 @@ dependency_overrides:
       name: foo
       url: '::'
 ''',
-          (pubspecOverrides) => pubspecOverrides.dependencyOverrides,
+          (pubspecOverrides) => errorsOf(pubspecOverrides.dependencyOverrides),
           'Error on line 4, column 7 of ${Platform.pathSeparator}pubspec_overrides.yaml',
         );
       });
@@ -1030,3 +1032,11 @@ name: 'foo'
     });
   });
 }
+
+void errorsOf(Map<String, PackageRange> m) {
+  for (final PackageRange(description: description) in m.values) {
+    if (description is ErrorDescription) {
+      throw description.exception;
+    }
+  }
+}