diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 8d6dd67c2..efba10c2a 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -87,6 +87,7 @@ class Entrypoint { /// /// 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 @@ class Entrypoint { 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 eaa41bf7a..8058dc3bb 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -116,13 +116,14 @@ class Package { /// 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 @@ class Package { 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 f1ec4729d..f8bfe8ca5 100644 --- a/lib/src/pubspec.dart +++ b/lib/src/pubspec.dart @@ -207,11 +207,15 @@ 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); @@ -233,6 +237,7 @@ class Pubspec extends PubspecBase { return Pubspec.parse( readTextFile(pubspecPath), sources, + expectedName: expectedName, location: path.toUri(pubspecPath), overridesFileContents: overridesFileContents, overridesLocation: path.toUri(overridesPath), @@ -273,11 +278,15 @@ 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, @@ -288,7 +297,18 @@ 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]. /// @@ -297,6 +317,7 @@ class Pubspec extends PubspecBase { factory Pubspec.parse( String contents, SourceRegistry sources, { + String? expectedName, Uri? location, String? overridesFileContents, Uri? overridesLocation, @@ -318,6 +339,7 @@ class Pubspec extends PubspecBase { pubspecMap, sources, overridesFields: overridesFileMap, + expectedName: expectedName, location: location, ); } diff --git a/lib/src/solver/result.dart b/lib/src/solver/result.dart index bfb7dbead..fb8f2e584 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 @@ class SolveResult { return id; }), ); - await _validatePubspecNames(cache); // Invariant: the content-hashes in PUB_CACHE matches those provided by the // server. @@ -110,39 +107,6 @@ 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 _validatePubspecNames( - SystemCache cache, - ) async { - for (final id in packages) { - final pubspec = pubspecs[id.name]!; - final validatedNames = {}; - 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 b57f3b530..933b08c9a 100644 --- a/lib/src/source/cached.dart +++ b/lib/src/source/cached.dart @@ -29,7 +29,7 @@ abstract class CachedSource extends Source { Future 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 bc5470bc1..367f7a92d 100644 --- a/lib/src/source/git.dart +++ b/lib/src/source/git.dart @@ -336,6 +336,7 @@ class GitSource extends CachedSource { cache, ), cache.sources, + expectedName: ref.name, ); } @@ -427,7 +428,7 @@ class GitSource extends CachedSource { 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 6d3d0bdad..e99ee523a 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -396,6 +396,7 @@ class HostedSource extends CachedSource { var pubspec = Pubspec.fromMap( pubspecData, cache.sources, + expectedName: ref.name, location: location, ); final archiveSha256 = map['archive_sha256']; @@ -956,7 +957,7 @@ class HostedSource extends CachedSource { var packages = []; 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 @@ class HostedSource extends CachedSource { .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 e2b3ef993..f881b8e9e 100644 --- a/lib/src/source/path.dart +++ b/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); + 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 3b391f68d..d37c13ef9 100644 --- a/lib/src/source/sdk.dart +++ b/lib/src/source/sdk.dart @@ -89,8 +89,11 @@ 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); + 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 10145febf..ff2b53537 100644 --- a/lib/src/system_cache.dart +++ b/lib/src/system_cache.dart @@ -112,13 +112,17 @@ Consider setting the `PUB_CACHE` variable manually. /// /// 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 b406f725e..d0a921bd4 100644 --- a/test/ascii_tree_test.dart +++ b/test/ascii_tree_test.dart @@ -61,9 +61,11 @@ void main() { 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 4568eac5f..da0505e19 100644 --- a/test/directory_option_test.dart +++ b/test/directory_option_test.dart @@ -50,7 +50,7 @@ main() => print('Hi'); 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 b048f86ba..2dcf1692b 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 @@ void main() { ]).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 c06ca2140..7af4b2dda 100644 --- a/test/pubspec_test.dart +++ b/test/pubspec_test.dart @@ -15,6 +15,9 @@ void main() { group('parse()', () { final sources = SystemCache().sources; + var throwsPubspecException = + throwsA(const TypeMatcher()); + void expectPubspecException( String contents, void Function(Pubspec) fn, [ @@ -38,6 +41,24 @@ 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( ''' 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 c69e9e259..e4da8be6c 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 @@ Got dependencies in myapp/example! ## 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 711c3448d..b45a56fe9 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 @@ void main() { ).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, );