Dependency: Move spec / git ref inside of Repository

The `Dependency` struct is logically acting as an `enum`,
with 3 discrete possibilities. However, some of the logic is
piggybacking on the (original) version logic.
`Repository` is one of them, and by separating things,
we can make stronger assumptions in `Version` and `Dependency`.

Author: Geod24

source/dub/dependency.d

@@ -98,9 +98,20 @@ struct Dependency {
 	/** Constructs a new dependency specification that matches a specific
 		Git reference.
 	*/
-	this(Repository repository, string spec) {
-		this.versionSpec = spec;
+	this(Repository repository)
+	{
 		this.repository = repository;
+		// Set for backward compatibility
+		auto ver = Version(repository.m_ref);
+		this.m_range = VersionRange(ver, ver, true, true);
+	}
+
+	deprecated("Instantiate the `Repository` struct with the string directy")
+	this(Repository repository, string spec)
+	{
+		assert(repository.m_ref is null);
+		repository.m_ref = spec;
+		this(repository);
 	}
 
 	/// If set, overrides any version based dependency selection.
@@ -112,6 +123,9 @@ struct Dependency {
 	@property void repository(Repository value)
 	{
 		m_repository = value;
+		// Set for backward compatibility
+		auto ver = Version(value.m_ref);
+		this.m_range = VersionRange(ver, ver, true, true);
 	}
 
 	/// ditto
@@ -246,7 +260,7 @@ struct Dependency {
 	}
 
 	@trusted unittest {
-		Dependency dependency = Dependency(Repository("git+http://localhost"), "1.0.0");
+		Dependency dependency = Dependency(Repository("git+http://localhost", "1.0.0"));
 		Json expected = Json([
 			"repository": Json("git+http://localhost"),
 			"version": Json("1.0.0")
@@ -272,8 +286,8 @@ struct Dependency {
 				enforce("version" in verspec, "No version field specified!");
 				enforce(repository.length > 0, "No repository field specified!");
 
-				dep = Dependency(Repository(repository.get!string),
-						verspec["version"].get!string);
+				dep = Dependency(Repository(
+                                     repository.get!string, verspec["version"].get!string));
 			} else {
 				enforce("version" in verspec, "No version field specified!");
 				auto ver = verspec["version"].get!string;
@@ -606,6 +620,7 @@ unittest {
 struct Repository
 {
 	private string m_remote;
+	private string m_ref;
 
 	private Kind m_kind;
 
@@ -617,18 +632,28 @@ struct Repository
 	/**
 		Params:
 			remote = Repository remote.
+			ref_   = Reference to use (SHA1, tag, branch name...)
 	 */
+	this(string remote, string ref_)
+	{
+		enforce(remote.startsWith("git+"), "Unsupported repository type");
+
+		m_remote = remote["git+".length .. $];
+		m_kind = Kind.git;
+		m_ref = ref_;
+		assert(m_remote.length);
+		assert(m_ref.length);
+	}
+
+	/// Ditto
+	deprecated("Use the constructor accepting a second parameter named `ref_`")
 	this(string remote)
 	{
-		if (remote.startsWith("git+"))
-		{
-			m_remote = remote["git+".length .. $];
-			m_kind = Kind.git;
-		}
-		else
-		{
-			throw new Exception("Unsupported repository type");
-		}
+		enforce(remote.startsWith("git+"), "Unsupported repository type");
+
+		m_remote = remote["git+".length .. $];
+		m_kind = Kind.git;
+		assert(m_remote.length);
 	}
 
 	string toString() nothrow pure @safe
@@ -655,6 +680,18 @@ struct Repository
 		return m_remote;
 	}
 
+	/**
+		Returns:
+			The reference (commit hash, branch name, tag) we are targeting
+	*/
+	@property string ref_() @nogc nothrow pure @safe
+	in { assert(m_remote !is null); }
+	in { assert(m_ref !is null); }
+	do
+	{
+		return m_ref;
+	}
+
 	/**
 		Returns:
 			Repository type.

source/dub/dub.d

@@ -612,7 +612,7 @@ class Dub {
 			if (!pack) fetch(p, ver, defaultPlacementLocation, fetchOpts, "getting selected version");
 			if ((options & UpgradeOptions.select) && p != m_project.rootPackage.name) {
 				if (!ver.repository.empty) {
-					m_project.selections.selectVersionWithRepository(p, ver.repository, ver.versionSpec);
+					m_project.selections.selectVersionWithRepository(p, ver.repository);
 				} else if (ver.path.empty) {
 					m_project.selections.selectVersion(p, ver.version_);
 				} else {

source/dub/project.d

@@ -1764,9 +1764,9 @@ final class SelectedVersions {
 	}
 
 	/// Selects a certain Git reference for a specific package.
-	void selectVersionWithRepository(string package_id, Repository repository, string spec)
+	void selectVersionWithRepository(string package_id, Repository repository)
 	{
-		const dependency = Dependency(repository, spec);
+		const dependency = Dependency(repository);
 		if (auto ps = package_id in m_selections) {
 			if (ps.dep == dependency)
 				return;
@@ -1775,6 +1775,12 @@ final class SelectedVersions {
 		m_dirty = true;
 	}
 
+	deprecated("Move `spec` inside of the `repository` parameter")
+	void selectVersionWithRepository(string package_id, Repository repository, string spec)
+	{
+		this.selectVersionWithRepository(package_id, Repository(repository.remote(), spec));
+	}
+
 	/// Removes the selection for a particular package.
 	void deselectVersion(string package_id)
 	{
@@ -1853,8 +1859,8 @@ final class SelectedVersions {
 		else if (j.type == Json.Type.object && "path" in j)
 			return Dependency(NativePath(j["path"].get!string));
 		else if (j.type == Json.Type.object && "repository" in j)
-			return Dependency(Repository(j["repository"].get!string),
-				enforce("version" in j, "Expected \"version\" field in repository version object").get!string);
+			return Dependency(Repository(j["repository"].get!string,
+				enforce("version" in j, "Expected \"version\" field in repository version object").get!string));
 		else throw new Exception(format("Unexpected type for dependency: %s", j));
 	}
 

source/dub/recipe/sdl.d

@@ -194,8 +194,8 @@ private void parseDependency(Tag t, ref BuildSettingsTemplate bs, string package
 	} else if ("repository" in attrs) {
 		enforceSDL("version" in attrs, "Missing version specification.", t);
 
-		dep.repository = Repository(attrs["repository"][0].value.get!string);
-		dep.versionSpec = attrs["version"][0].value.get!string;
+		dep.repository = Repository(attrs["repository"][0].value.get!string,
+                                    attrs["version"][0].value.get!string);
 	} else {
 		enforceSDL("version" in attrs, "Missing version specification.", t);
 		dep.versionSpec = attrs["version"][0].value.get!string;
@@ -672,8 +672,8 @@ unittest {
 	PackageRecipe p;
 	p.name = "test";
 
-	auto repository = Repository("git+https://some.url");
-	p.buildSettings.dependencies["package"] = Dependency(repository, "12345678");
+	auto repository = Repository("git+https://some.url", "12345678");
+	p.buildSettings.dependencies["package"] = Dependency(repository);
 	auto sdl = toSDL(p).toSDLDocument();
 	assert(sdl ==
 `name "test"