Adapt PackageManager.get[Best]Package to PackageName API

This paves the way to be able to automatically return subpackages.

source/dub/commandline.d

@@ -1379,7 +1379,8 @@ abstract class PackageBuildCommand : Command {
 
 		enforce(udesc.name.length, "No valid root package found - aborting.");
 
-		auto pack = dub.packageManager.getBestPackage(udesc.name, udesc.range);
+		auto pack = dub.packageManager.getBestPackage(
+			PackageName(udesc.name), udesc.range);
 
 		enforce(pack, format!"Failed to find package '%s' locally."(udesc));
 		logInfo("Building package %s in %s", pack.name, pack.path.toNativeString());
@@ -1559,7 +1560,7 @@ class BuildCommand : GenerateCommand {
 
 		const baseName = PackageName(packageParts.name).main;
 		// Found locally
-		if (dub.packageManager.getBestPackage(baseName.toString(), packageParts.range))
+		if (dub.packageManager.getBestPackage(baseName, packageParts.range))
 			return 0;
 
 		// Non-interactive, either via flag, or because a version was provided
@@ -2221,7 +2222,8 @@ class FetchCommand : FetchRemoveCommand {
             break;
         }
         if (this.recursive) {
-            auto pack = dub.packageManager.getBestPackage(udesc.name, udesc.range);
+            auto pack = dub.packageManager.getBestPackage(
+				PackageName(udesc.name), udesc.range);
             auto proj = new Project(dub.packageManager, pack);
             if (!proj.hasAllDependencies) {
 				logInfo("Resolving", Color.green, "missing dependencies for project");
@@ -2234,7 +2236,8 @@ class FetchCommand : FetchRemoveCommand {
 	/// Implementation for argument version
 	private FetchStatus fetchSinglePackage(Dub dub, UserPackageDesc udesc)
 	{
-		auto fspkg = dub.packageManager.getBestPackage(udesc.name, udesc.range);
+		auto fspkg = dub.packageManager.getBestPackage(
+			PackageName(udesc.name), udesc.range);
 		// Avoid dub fetch if the package is present on the filesystem.
 		if (fspkg !is null && udesc.range.isExactVersion())
 			return FetchStatus.Present;

source/dub/dub.d

@@ -642,7 +642,7 @@ class Dub {
 					if (m_packageManager.loadSCMPackage(name, dep.repository))
 						continue;
 				} else {
-					if (m_packageManager.getPackage(p, dep.version_)) continue;
+					if (m_packageManager.getPackage(name, dep.version_)) continue;
 					foreach (ps; m_packageSuppliers) {
 						try {
 							auto versions = ps.getVersions(name);
@@ -706,7 +706,7 @@ class Dub {
 				pack = m_packageManager.loadSCMPackage(name, ver.repository);
 			} else {
 				assert(ver.isExactVersion, "Resolved dependency is neither path, nor repository, nor exact version based!?");
-				pack = m_packageManager.getPackage(p, ver.version_);
+				pack = m_packageManager.getPackage(name, ver.version_);
 				if (pack && m_packageManager.isManagedPackage(pack)
 					&& ver.version_.isBranch && (options & UpgradeOptions.upgrade) != 0)
 				{
@@ -797,7 +797,7 @@ class Dub {
 
 		auto tool = PackageName("dscanner");
 
-		auto tool_pack = m_packageManager.getBestPackage(tool.toString());
+		auto tool_pack = m_packageManager.getBestPackage(tool);
 		if (!tool_pack) {
 			logInfo("Hint", Color.light_blue, "%s is not present, getting and storing it locally", tool);
 			tool_pack = this.fetch(tool);
@@ -989,7 +989,7 @@ class Dub {
 		Version ver = Version(pinfo["version"].get!string);
 
 		// always upgrade branch based versions - TODO: actually check if there is a new commit available
-		Package existing = m_packageManager.getPackage(name.toString(), ver, location);
+		Package existing = m_packageManager.getPackage(name, ver, location);
 		if (options & FetchOptions.printOnly) {
 			if (existing && existing.version_ != ver)
 				logInfo("A new version for %s is available (%s -> %s). Run \"%s\" to switch.",
@@ -1029,7 +1029,7 @@ class Dub {
 			logDiagnostic("Placing to %s...", location.toString());
 
 			try {
-				return m_packageManager.store(path, location, name.main.toString(), ver);
+				return m_packageManager.store(path, location, name.main, ver);
 			} catch (ZipException e) {
 				logInfo("Failed to extract zip archive for %s@%s...", name, ver);
 				// re-throw the exception at the end of the loop
@@ -1403,7 +1403,7 @@ class Dub {
 	private void runCustomInitialization(NativePath path, string name, string[] runArgs)
 	{
 		auto name_ = PackageName(name);
-		auto template_pack = m_packageManager.getBestPackage(name);
+		auto template_pack = m_packageManager.getBestPackage(name_);
 		if (!template_pack) {
 			logInfo("%s is not present, getting and storing it locally", name);
 			template_pack = fetch(name_);
@@ -1471,7 +1471,7 @@ class Dub {
 			? PackageName("ddox")
 			: PackageName(m_project.rootPackage.recipe.ddoxTool);
 
-		auto tool_pack = m_packageManager.getBestPackage(tool.toString());
+		auto tool_pack = m_packageManager.getBestPackage(tool);
 		if (!tool_pack) {
 			logInfo("%s is not present, getting and storing it user wide", tool);
 			tool_pack = this.fetch(tool);
@@ -1948,7 +1948,7 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
 		}
 		const vers = dep.version_;
 
-		if (auto ret = m_dub.m_packageManager.getBestPackage(name.toString(), vers))
+		if (auto ret = m_dub.m_packageManager.getBestPackage(name, vers))
 			return ret;
 
 		auto key = name.toString() ~ ":" ~ vers.toString();
@@ -1978,7 +1978,7 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
 					FetchOptions fetchOpts;
 					fetchOpts |= prerelease ? FetchOptions.usePrerelease : FetchOptions.none;
 					m_dub.fetch(name.main, vers, fetchOpts, m_dub.defaultPlacementLocation, "need sub package description");
-					auto ret = m_dub.m_packageManager.getBestPackage(name.toString(), vers);
+					auto ret = m_dub.m_packageManager.getBestPackage(name, vers);
 					if (!ret) {
 						logWarn("Package %s %s doesn't have a sub package %s", name.main, dep, name);
 						return null;

source/dub/packagemanager.d

@@ -219,12 +219,12 @@ class PackageManager {
 		Returns:
 			The matching package or null if no match was found.
 	*/
-	Package getPackage(string name, Version ver, bool enable_overrides = true)
+	Package getPackage(in PackageName name, in Version ver, bool enable_overrides = true)
 	{
 		if (enable_overrides) {
 			foreach (ref repo; m_repositories)
 				foreach (ovr; repo.overrides)
-					if (ovr.package_ == name && ovr.source.matches(ver)) {
+					if (ovr.package_ == name.toString() && ovr.source.matches(ver)) {
 						Package pack = ovr.target.match!(
 							(NativePath path) => getOrLoadPackage(path),
 							(Version	vers) => getPackage(name, vers, false),
@@ -240,7 +240,13 @@ class PackageManager {
 					}
 		}
 
-		return this.lookup(PackageName(name), ver);
+		return this.lookup(name, ver);
+	}
+
+	deprecated("Use the overload that accepts a `PackageName` instead")
+	Package getPackage(string name, Version ver, bool enable_overrides = true)
+	{
+		return this.getPackage(PackageName(name), ver, enable_overrides);
 	}
 
 	/// ditto
@@ -263,12 +269,19 @@ class PackageManager {
 	}
 
 	/// Ditto
+	deprecated("Use the overload that accepts a `PackageName` instead")
 	Package getPackage(string name, Version ver, PlacementLocation loc)
+	{
+		return this.getPackage(PackageName(name), ver, loc);
+	}
+
+	/// Ditto
+	Package getPackage(in PackageName name, in Version ver, PlacementLocation loc)
 	{
 		// Bare mode
 		if (loc >= this.m_repositories.length)
 			return null;
-		return this.m_repositories[loc].load(PackageName(name), ver, this);
+		return this.m_repositories[loc].load(name, ver, this);
 	}
 
 	/// ditto
@@ -475,13 +488,27 @@ class PackageManager {
 	 * Returns:
 	 *	 The best package matching the parameters, or `null` if none was found.
 	 */
+	deprecated("Use the overload that accepts a `PackageName` instead")
 	Package getBestPackage(string name, Version vers)
+	{
+		return this.getBestPackage(PackageName(name), vers);
+	}
+
+	/// Ditto
+	Package getBestPackage(in PackageName name, in Version vers)
 	{
 		return this.getBestPackage(name, VersionRange(vers, vers));
 	}
 
 	/// Ditto
+	deprecated("Use the overload that accepts a `PackageName` instead")
 	Package getBestPackage(string name, VersionRange range = VersionRange.Any)
+	{
+		return this.getBestPackage(PackageName(name), range);
+	}
+
+	/// Ditto
+	Package getBestPackage(in PackageName name, in VersionRange range = VersionRange.Any)
 	{
 		return this.getBestPackage_(name, Dependency(range));
 	}
@@ -497,14 +524,15 @@ class PackageManager {
 	deprecated("`getBestPackage` should only be used with a `Version` or `VersionRange` argument")
 	Package getBestPackage(string name, Dependency version_spec, bool enable_overrides = true)
 	{
-		return this.getBestPackage_(name, version_spec, enable_overrides);
+		return this.getBestPackage_(PackageName(name), version_spec, enable_overrides);
 	}
 
 	// TODO: Merge this into `getBestPackage(string, VersionRange)`
-	private Package getBestPackage_(string name, Dependency version_spec, bool enable_overrides = true)
+	private Package getBestPackage_(in PackageName name, in Dependency version_spec,
+		bool enable_overrides = true)
 	{
 		Package ret;
-		foreach (p; getPackageIterator(name)) {
+		foreach (p; getPackageIterator(name.toString())) {
 			auto vmm = isManagedPackage(p) ? VersionMatchMode.strict : VersionMatchMode.standard;
 			if (version_spec.matches(p.version_, vmm) && (!ret || p.version_ > ret.version_))
 				ret = p;
@@ -696,7 +724,8 @@ class PackageManager {
 	deprecated("Use `store(NativePath source, PlacementLocation dest, string name, Version vers)`")
 	Package storeFetchedPackage(NativePath zip_file_path, Json package_info, NativePath destination)
 	{
-		return this.store_(zip_file_path, destination, package_info["name"].get!string,
+		return this.store_(zip_file_path, destination,
+			PackageName(package_info["name"].get!string),
 			Version(package_info["version"].get!string));
 	}
 
@@ -721,11 +750,18 @@ class PackageManager {
 	 *   If the package cannot be loaded / the zip is corrupted / the package
 	 *   already exists, etc...
 	 */
+	deprecated("Use the overload that accepts a `PackageName` instead")
 	Package store(NativePath src, PlacementLocation dest, string name, Version vers)
 	{
-		const name_ = PackageName(name);
-		assert(!name_.sub.length, "Cannot store a subpackage, use main package instead");
-		NativePath dstpath = this.getPackagePath(dest, name_, vers.toString());
+		return this.store(src, dest, PackageName(name), vers);
+	}
+
+	/// Ditto
+	Package store(NativePath src, PlacementLocation dest, in PackageName name,
+		in Version vers)
+	{
+		assert(!name.sub.length, "Cannot store a subpackage, use main package instead");
+		NativePath dstpath = this.getPackagePath(dest, name, vers.toString());
 		ensureDirectory(dstpath.parentPath());
 		const lockPath = dstpath.parentPath() ~ ".lock";
 
@@ -740,7 +776,8 @@ class PackageManager {
 
 	/// Backward-compatibility for deprecated overload, simplify once `storeFetchedPatch`
 	/// is removed
-	private Package store_(NativePath src, NativePath destination, string name, Version vers)
+	private Package store_(NativePath src, NativePath destination,
+		in PackageName name, in Version vers)
 	{
 		import std.range : walkLength;
 
@@ -1436,7 +1473,7 @@ package struct Location {
 	 * Returns:
 	 *	 A `Package` if one was found, `null` if none exists.
 	 */
-	Package load (PackageName name, Version vers, PackageManager mgr)
+	Package load (in PackageName name, Version vers, PackageManager mgr)
 	{
 		if (auto pkg = this.lookup(name, vers))
 			return pkg;

source/dub/project.d

@@ -568,7 +568,7 @@ class Project {
 					(VersionRange range) {
 						// See `dub.recipe.selection : SelectedDependency.fromYAML`
 						assert(range.isExactVersion());
-						return m_packageManager.getPackage(dep.name.toString(), vspec.version_);
+						return m_packageManager.getPackage(dep.name, vspec.version_);
 					},
 				);
 			} else if (m_dependencies.canFind!(d => PackageName(d.name).main == basename)) {

source/dub/test/base.d

@@ -283,6 +283,7 @@ package class TestPackageManager : PackageManager
 
     // Re-introduce hidden/deprecated overloads
     public alias loadSCMPackage = PackageManager.loadSCMPackage;
+    public alias store = PackageManager.store;
 
     /// Disabled as semantic are not implementable unless a virtual FS is created
 	public override @property void customCachePaths(NativePath[] custom_cache_paths)
@@ -291,7 +292,7 @@ package class TestPackageManager : PackageManager
     }
 
     /// Ditto
-    public override Package store(NativePath src, PlacementLocation dest, string name, Version vers)
+    public override Package store(NativePath src, PlacementLocation dest, in PackageName name, in Version vers)
     {
         assert(0, "Function not implemented");
     }