Revert "Validate pubspec names after resolution, instead of when parsing (#3956)" (#4006)
This reverts commit 037138e6ea82488a2ec2ca4e190680506fe634db.
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart
index 8d6dd67..efba10c 100644
--- a/lib/src/entrypoint.dart
+++ b/lib/src/entrypoint.dart
@@ -87,6 +87,7 @@
///
/// For a global package, this is the activated package.
Package get root => _root ??= Package.load(
+ null,
rootDir,
cache.sources,
withPubspecOverrides: true,
@@ -177,6 +178,7 @@
var packages = {
for (var packageEntry in packageConfig.nonInjectedPackages)
packageEntry.name: Package.load(
+ packageEntry.name,
packageEntry.resolvedRootDir(packageConfigPath),
cache.sources,
),
diff --git a/lib/src/package.dart b/lib/src/package.dart
index eaa41bf..8058dc3 100644
--- a/lib/src/package.dart
+++ b/lib/src/package.dart
@@ -116,13 +116,14 @@
/// Loads the package whose root directory is [packageDir].
///
- /// [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.
+ /// [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.
///
/// `pubspec_overrides.yaml` is only loaded if [withPubspecOverrides] is
/// `true`.
factory Package.load(
+ String? name,
String dir,
SourceRegistry sources, {
bool withPubspecOverrides = false,
@@ -130,6 +131,7 @@
final pubspec = Pubspec.load(
dir,
sources,
+ expectedName: name,
allowOverridesFile: withPubspecOverrides,
);
return Package._(dir, pubspec);
diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart
index f1ec472..f8bfe8c 100644
--- a/lib/src/pubspec.dart
+++ b/lib/src/pubspec.dart
@@ -207,11 +207,15 @@
/// 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);
@@ -233,6 +237,7 @@
return Pubspec.parse(
readTextFile(pubspecPath),
sources,
+ expectedName: expectedName,
location: path.toUri(pubspecPath),
overridesFileContents: overridesFileContents,
overridesLocation: path.toUri(overridesPath),
@@ -273,11 +278,15 @@
/// 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,
@@ -288,7 +297,18 @@
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].
///
@@ -297,6 +317,7 @@
factory Pubspec.parse(
String contents,
SourceRegistry sources, {
+ String? expectedName,
Uri? location,
String? overridesFileContents,
Uri? overridesLocation,
@@ -318,6 +339,7 @@
pubspecMap,
sources,
overridesFields: overridesFileMap,
+ expectedName: expectedName,
location: location,
);
}
diff --git a/lib/src/solver/result.dart b/lib/src/solver/result.dart
index bfb7dbe..fb8f2e5 100644
--- a/lib/src/solver/result.dart
+++ b/lib/src/solver/result.dart
@@ -4,9 +4,7 @@
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';
@@ -78,7 +76,6 @@
return id;
}),
);
- await _validatePubspecNames(cache);
// Invariant: the content-hashes in PUB_CACHE matches those provided by the
// server.
@@ -110,39 +107,6 @@
);
}
- /// 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.
diff --git a/lib/src/source/cached.dart b/lib/src/source/cached.dart
index b57f3b5..933b08c 100644
--- a/lib/src/source/cached.dart
+++ b/lib/src/source/cached.dart
@@ -29,7 +29,7 @@
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);
+ return Pubspec.load(packageDir, cache.sources, expectedName: id.name);
}
return await describeUncached(id, cache);
diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart
index bc5470b..367f7a9 100644
--- a/lib/src/source/git.dart
+++ b/lib/src/source/git.dart
@@ -336,6 +336,7 @@
cache,
),
cache.sources,
+ expectedName: ref.name,
);
}
@@ -427,7 +428,7 @@
var packageDir = p.join(revisionCachePath, relative);
try {
- return Package.load(packageDir, cache.sources);
+ return Package.load(null, packageDir, cache.sources);
} catch (error, stackTrace) {
log.error('Failed to load package', error, stackTrace);
var name = p.basename(revisionCachePath).split('-').first;
diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart
index 6d3d0bd..e99ee52 100644
--- a/lib/src/source/hosted.dart
+++ b/lib/src/source/hosted.dart
@@ -396,6 +396,7 @@
var pubspec = Pubspec.fromMap(
pubspecData,
cache.sources,
+ expectedName: ref.name,
location: location,
);
final archiveSha256 = map['archive_sha256'];
@@ -956,7 +957,7 @@
var packages = <Package>[];
for (var entry in listDir(serverDir)) {
try {
- packages.add(Package.load(entry, cache.sources));
+ packages.add(Package.load(null, entry, cache.sources));
} catch (error, stackTrace) {
log.error('Failed to load package', error, stackTrace);
final id = _idForBasename(
@@ -1063,7 +1064,7 @@
.where(_looksLikePackageDir)
.map((entry) {
try {
- return Package.load(entry, cache.sources);
+ return Package.load(null, entry, cache.sources);
} catch (error, stackTrace) {
log.fine('Failed to load package from $entry:\n'
'$error\n'
diff --git a/lib/src/source/path.dart b/lib/src/source/path.dart
index e2b3ef9..f881b8e 100644
--- a/lib/src/source/path.dart
+++ b/lib/src/source/path.dart
@@ -168,7 +168,7 @@
throw ArgumentError('Wrong source');
}
var dir = _validatePath(ref.name, description);
- return Pubspec.load(dir, cache.sources);
+ return Pubspec.load(dir, cache.sources, expectedName: ref.name);
}
@override
diff --git a/lib/src/source/sdk.dart b/lib/src/source/sdk.dart
index 3b391f6..d37c13e 100644
--- a/lib/src/source/sdk.dart
+++ b/lib/src/source/sdk.dart
@@ -89,8 +89,11 @@
///
/// 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);
+ Pubspec _loadPubspec(PackageRef ref, SystemCache cache) => Pubspec.load(
+ _verifiedPackagePath(ref),
+ cache.sources,
+ expectedName: ref.name,
+ );
/// Returns the path for the given [ref].
///
diff --git a/lib/src/system_cache.dart b/lib/src/system_cache.dart
index 10145fe..ff2b535 100644
--- a/lib/src/system_cache.dart
+++ b/lib/src/system_cache.dart
@@ -112,13 +112,17 @@
///
/// Throws an [ArgumentError] if [id] has an invalid source.
Package load(PackageId id) {
- return Package.load(getDirectory(id), sources);
+ return Package.load(id.name, getDirectory(id), sources);
}
Package loadCached(PackageId id) {
final source = id.description.description.source;
if (source is CachedSource) {
- return Package.load(source.getDirectoryInCache(id, this), sources);
+ return Package.load(
+ id.name,
+ source.getDirectoryInCache(id, this),
+ sources,
+ );
} else {
throw ArgumentError('Call only on Cached ids.');
}
diff --git a/test/ascii_tree_test.dart b/test/ascii_tree_test.dart
index b406f72..d0a921b 100644
--- a/test/ascii_tree_test.dart
+++ b/test/ascii_tree_test.dart
@@ -61,9 +61,11 @@
file('path.dart', bytes(100)),
]),
]).create();
- var files =
- Package.load(path(appPath), (name) => throw UnimplementedError())
- .listFiles();
+ var files = Package.load(
+ null,
+ path(appPath),
+ (name) => throw UnimplementedError(),
+ ).listFiles();
ctx.expectNextSection(
tree.fromFiles(files, baseDir: path(appPath), showFileSizes: true),
);
diff --git a/test/directory_option_test.dart b/test/directory_option_test.dart
index 4568eac..da0505e 100644
--- a/test/directory_option_test.dart
+++ b/test/directory_option_test.dart
@@ -50,7 +50,7 @@
pubspec({
'name': 'example',
'dependencies': {
- 'test_pkg': {'path': '../'},
+ 'myapp': {'path': '../'}, // Wrong name of dependency
},
}),
]),
diff --git a/test/get/git/dependency_name_match_pubspec_test.dart b/test/get/git/dependency_name_match_pubspec_test.dart
index b048f86..2dcf169 100644
--- a/test/get/git/dependency_name_match_pubspec_test.dart
+++ b/test/get/git/dependency_name_match_pubspec_test.dart
@@ -28,9 +28,8 @@
]).create();
await pubGet(
- error: contains(
- 'Expected to find package "weirdname", found package "foo".',
- ),
+ error: contains('"name" field doesn\'t match expected name '
+ '"weirdname".'),
exitCode: exit_codes.DATA,
);
});
diff --git a/test/pubspec_test.dart b/test/pubspec_test.dart
index c06ca21..7af4b2d 100644
--- a/test/pubspec_test.dart
+++ b/test/pubspec_test.dart
@@ -15,6 +15,9 @@
group('parse()', () {
final sources = SystemCache().sources;
+ var throwsPubspecException =
+ throwsA(const TypeMatcher<SourceSpanApplicationException>());
+
void expectPubspecException(
String contents,
void Function(Pubspec) fn, [
@@ -38,6 +41,24 @@
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(
'''
diff --git a/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt b/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt
index c69e9e2..e4da8be 100644
--- a/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt
+++ b/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt
@@ -59,9 +59,12 @@
## Section 6
$ pub get bar -C 'myapp/example2'
Resolving dependencies in myapp/example2...
-+ foo 1.0.0
-+ test_pkg 1.0.0 from path myapp
-Changed 2 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
-------------------------------- END OF OUTPUT ---------------------------------
diff --git a/test/upgrade/git/upgrade_to_incompatible_pubspec_test.dart b/test/upgrade/git/upgrade_to_incompatible_pubspec_test.dart
index 711c344..b45a56f 100644
--- a/test/upgrade/git/upgrade_to_incompatible_pubspec_test.dart
+++ b/test/upgrade/git/upgrade_to_incompatible_pubspec_test.dart
@@ -42,7 +42,7 @@
).commit();
await pubUpgrade(
- error: contains('Expected to find package "foo", found package "zoo".'),
+ error: contains('"name" field doesn\'t match expected name "foo".'),
exitCode: exit_codes.DATA,
);
