Various code simplification around dub.selections.json (dlang#2311)

* Deprecate unused Json constructor

* Trivial: Fix import order in dub.project

* Simplify DependencyResolver filtering

DependencyResolver is only instantiated from one place, and never re-used.
It makes more sense to set those variables in the constructor,
and avoid repeatedly calling a single function.

* Move file format to dub.selections.json to its own struct

Our goal is to replace the handcrafted JSON code with configy.

Author: Geod24

build-files.txt

@@ -51,4 +51,5 @@ source/dub/internal/vibecompat/inet/url.d
 source/dub/recipe/io.d
 source/dub/recipe/json.d
 source/dub/recipe/packagerecipe.d
+source/dub/recipe/selection.d
 source/dub/recipe/sdl.d

source/dub/dub.d

@@ -549,11 +549,9 @@ class Dub {
 			}
 		}
 
-		Dependency[string] versions;
-		auto resolver = new DependencyVersionResolver(this, options);
-		foreach (p; packages_to_upgrade)
-			resolver.addPackageToUpgrade(p);
-		versions = resolver.resolve(m_project.rootPackage, m_project.selections);
+		auto resolver = new DependencyVersionResolver(
+			this, options, m_project.rootPackage, m_project.selections);
+		Dependency[string] versions = resolver.resolve(packages_to_upgrade);
 
 		if (options & UpgradeOptions.dryRun) {
 			bool any = false;
@@ -1509,22 +1507,20 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
 	}
 
 
-	this(Dub dub, UpgradeOptions options)
+	this(Dub dub, UpgradeOptions options, Package root, SelectedVersions selected_versions)
 	{
 		m_dub = dub;
 		m_options = options;
+		m_rootPackage = root;
+		m_selectedVersions = selected_versions;
 	}
 
-	void addPackageToUpgrade(string name)
-	{
-		m_packagesToUpgrade[name] = true;
-	}
-
-	Dependency[string] resolve(Package root, SelectedVersions selected_versions)
+	Dependency[string] resolve(string[] filter)
 	{
-		m_rootPackage = root;
-		m_selectedVersions = selected_versions;
-		return super.resolve(TreeNode(root.name, Dependency(root.version_)), (m_options & UpgradeOptions.printUpgradesOnly) == 0);
+		foreach (name; filter)
+			m_packagesToUpgrade[name] = true;
+		return super.resolve(TreeNode(m_rootPackage.name, Dependency(m_rootPackage.version_)),
+			(m_options & UpgradeOptions.printUpgradesOnly) == 0);
 	}
 
 	protected bool isFixedPackage(string pack)

source/dub/project.d

@@ -10,22 +10,23 @@ module dub.project;
 import dub.compilers.compiler;
 import dub.dependency;
 import dub.description;
+import dub.generators.generator;
 import dub.internal.utils;
 import dub.internal.vibecompat.core.file;
 import dub.internal.vibecompat.core.log;
 import dub.internal.vibecompat.data.json;
 import dub.internal.vibecompat.inet.path;
 import dub.package_;
 import dub.packagemanager;
-import dub.generators.generator;
+import dub.recipe.selection;
 
 import std.algorithm;
 import std.array;
 import std.conv : to;
 import std.datetime;
+import std.encoding : sanitize;
 import std.exception : enforce;
 import std.string;
-import std.encoding : sanitize;
 
 /**
 	Represents a full project, a root package with its dependencies and package
@@ -1680,13 +1681,9 @@ unittest
 	"dub.selections.json" within a package's directory.
 */
 final class SelectedVersions {
-	private struct Selected {
-		Dependency dep;
-		//Dependency[string] packages;
-	}
 	private {
 		enum FileVersion = 1;
-		Selected[string] m_selections;
+		Selected m_selections;
 		bool m_dirty = false; // has changes since last save
 		bool m_bare = true;
 	}
@@ -1695,13 +1692,17 @@ final class SelectedVersions {
 	enum defaultFile = "dub.selections.json";
 
 	/// Constructs a new empty version selection.
-	this() {}
+	public this(Selected data = Selected(FileVersion)) @safe pure nothrow @nogc
+	{
+		this.m_selections = data;
+	}
 
 	/** Constructs a new version selection from JSON data.
 
 		The structure of the JSON document must match the contents of the
 		"dub.selections.json" file.
 	*/
+	deprecated("Pass a `dub.recipe.selection : Selected` directly")
 	this(Json data)
 	{
 		deserialize(data);
@@ -1719,7 +1720,7 @@ final class SelectedVersions {
 	}
 
 	/// Returns a list of names for all packages that have a version selection.
-	@property string[] selectedPackages() const { return m_selections.keys; }
+	@property string[] selectedPackages() const { return m_selections.versions.keys; }
 
 	/// Determines if any changes have been made after loading the selections from a file.
 	@property bool dirty() const { return m_dirty; }
@@ -1730,48 +1731,49 @@ final class SelectedVersions {
 	/// Removes all selections.
 	void clear()
 	{
-		m_selections = null;
+		m_selections.versions = null;
 		m_dirty = true;
 	}
 
 	/// Duplicates the set of selected versions from another instance.
 	void set(SelectedVersions versions)
 	{
-		m_selections = versions.m_selections.dup;
+		m_selections.fileVersion = versions.m_selections.fileVersion;
+		m_selections.versions = versions.m_selections.versions.dup;
 		m_dirty = true;
 	}
 
 	/// Selects a certain version for a specific package.
 	void selectVersion(string package_id, Version version_)
 	{
-		if (auto ps = package_id in m_selections) {
-			if (ps.dep == Dependency(version_))
+		if (auto pdep = package_id in m_selections.versions) {
+			if (*pdep == Dependency(version_))
 				return;
 		}
-		m_selections[package_id] = Selected(Dependency(version_)/*, issuer*/);
+		m_selections.versions[package_id] = Dependency(version_);
 		m_dirty = true;
 	}
 
 	/// Selects a certain path for a specific package.
 	void selectVersion(string package_id, NativePath path)
 	{
-		if (auto ps = package_id in m_selections) {
-			if (ps.dep == Dependency(path))
+		if (auto pdep = package_id in m_selections.versions) {
+			if (*pdep == Dependency(path))
 				return;
 		}
-		m_selections[package_id] = Selected(Dependency(path));
+		m_selections.versions[package_id] = Dependency(path);
 		m_dirty = true;
 	}
 
 	/// Selects a certain Git reference for a specific package.
 	void selectVersionWithRepository(string package_id, Repository repository)
 	{
 		const dependency = Dependency(repository);
-		if (auto ps = package_id in m_selections) {
-			if (ps.dep == dependency)
+		if (auto pdep = package_id in m_selections.versions) {
+			if (*pdep == dependency)
 				return;
 		}
-		m_selections[package_id] = Selected(dependency);
+		m_selections.versions[package_id] = dependency;
 		m_dirty = true;
 	}
 
@@ -1784,14 +1786,14 @@ final class SelectedVersions {
 	/// Removes the selection for a particular package.
 	void deselectVersion(string package_id)
 	{
-		m_selections.remove(package_id);
+		m_selections.versions.remove(package_id);
 		m_dirty = true;
 	}
 
 	/// Determines if a particular package has a selection set.
 	bool hasSelectedVersion(string packageId)
 	const {
-		return (packageId in m_selections) !is null;
+		return (packageId in m_selections.versions) !is null;
 	}
 
 	/** Returns the selection for a particular package.
@@ -1804,7 +1806,7 @@ final class SelectedVersions {
 	Dependency getSelectedVersion(string packageId)
 	const {
 		enforce(hasSelectedVersion(packageId));
-		return m_selections[packageId].dep;
+		return m_selections.versions[packageId];
 	}
 
 	/** Stores the selections to disk.
@@ -1868,10 +1870,10 @@ final class SelectedVersions {
 	const {
 		Json json = serializeToJson(m_selections);
 		Json serialized = Json.emptyObject;
-		serialized["fileVersion"] = FileVersion;
+		serialized["fileVersion"] = m_selections.fileVersion;
 		serialized["versions"] = Json.emptyObject;
-		foreach (p, v; m_selections)
-			serialized["versions"][p] = dependencyToJson(v.dep);
+		foreach (p, dep; m_selections.versions)
+			serialized["versions"][p] = dependencyToJson(dep);
 		return serialized;
 	}
 
@@ -1880,7 +1882,7 @@ final class SelectedVersions {
 		enforce(cast(int)json["fileVersion"] == FileVersion, "Mismatched dub.select.json version: " ~ to!string(cast(int)json["fileVersion"]) ~ "vs. " ~to!string(FileVersion));
 		clear();
 		scope(failure) clear();
-		foreach (string p, v; json["versions"])
-			m_selections[p] = Selected(dependencyFromJson(v));
+		foreach (string p, dep; json["versions"])
+			m_selections.versions[p] = dependencyFromJson(dep);
 	}
 }

source/dub/recipe/selection.d

@@ -0,0 +1,15 @@
+/**
+ * Contains type definition for `dub.selections.json`
+ */
+module dub.recipe.selection;
+
+import dub.dependency;
+
+public struct Selected
+{
+    /// The current version of the file format
+    public uint fileVersion;
+
+    /// The selected package and their matching versions
+    public Dependency[string] versions;
+}