allow null values in ISemanticVersion compare methods (#837)

3 files changed, 52 insertions, 15 deletions

diff --git a/docs/release-notes.md b/docs/release-notes.md
index 732cd530..96a236ad 100644
--- a/docs/release-notes.md
+++ b/docs/release-notes.md
@@ -32,6 +32,7 @@
     _This adds methods for working with asset names, parsed locales, etc._
   * Added `helper.Content.ParseAssetName` to get an `IAssetName` for an arbitrary asset key.
   * If an asset is loaded multiple times in the same tick, `IAssetLoader.CanLoad` and `IAssetEditor.CanEdit` are now cached unless invalidated by `helper.Content.InvalidateCache`.
+  * The `ISemanticVersion` comparison methods (`CompareTo`, `IsBetween`, `IsNewerThan`, and `IsOlderThan`) now allow null values. A null version is always considered smaller than any non-null version per [best practices](https://docs.microsoft.com/en-us/dotnet/api/system.icomparable-1.compareto#remarks).
   * Fixed the `SDate` constructor being case-sensitive.
   * Fixed support for using locale codes from custom languages in asset names (e.g. `Data/Achievements.eo-EU`).
   * Fixed issue where suppressing `[Left|Right]Thumbstick[Down|Left]` keys would suppress the opposite direction instead.
diff --git a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs
index 66181ea6..599ac839 100644
--- a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs
+++ b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs
@@ -219,11 +219,16 @@ namespace SMAPI.Tests.Utilities
         [TestCase("1.0-beta.2", "1.0-beta.1", ExpectedResult = 1)]
         [TestCase("1.0-beta.10", "1.0-beta.2", ExpectedResult = 1)]
         [TestCase("1.0-beta-10", "1.0-beta-2", ExpectedResult = 1)]
+
+        // null
+        [TestCase("1.0.0", null, ExpectedResult = 1)] // null is always less than any value per CompareTo remarks
         public int CompareTo(string versionStrA, string versionStrB)
         {
             // arrange
             ISemanticVersion versionA = new SemanticVersion(versionStrA);
-            ISemanticVersion versionB = new SemanticVersion(versionStrB);
+            ISemanticVersion versionB = versionStrB != null
+                ? new SemanticVersion(versionStrB)
+                : null;
 
             // assert
             return versionA.CompareTo(versionB);
@@ -262,14 +267,19 @@ namespace SMAPI.Tests.Utilities
         [TestCase("1.0-beta.2", "1.0-beta.1", ExpectedResult = false)]
         [TestCase("1.0-beta.10", "1.0-beta.2", ExpectedResult = false)]
         [TestCase("1.0-beta-10", "1.0-beta-2", ExpectedResult = false)]
+
+        // null
+        [TestCase("1.0.0", null, ExpectedResult = false)] // null is always less than any value per CompareTo remarks
         public bool IsOlderThan(string versionStrA, string versionStrB)
         {
             // arrange
             ISemanticVersion versionA = new SemanticVersion(versionStrA);
-            ISemanticVersion versionB = new SemanticVersion(versionStrB);
+            ISemanticVersion versionB = versionStrB != null
+                ? new SemanticVersion(versionStrB)
+                : null;
 
             // assert
-            Assert.AreEqual(versionA.IsOlderThan(versionB), versionA.IsOlderThan(versionB.ToString()), "The two signatures returned different results.");
+            Assert.AreEqual(versionA.IsOlderThan(versionB), versionA.IsOlderThan(versionB?.ToString()), "The two signatures returned different results.");
             return versionA.IsOlderThan(versionB);
         }
 
@@ -306,14 +316,19 @@ namespace SMAPI.Tests.Utilities
         [TestCase("1.0-beta.2", "1.0-beta.1", ExpectedResult = true)]
         [TestCase("1.0-beta.10", "1.0-beta.2", ExpectedResult = true)]
         [TestCase("1.0-beta-10", "1.0-beta-2", ExpectedResult = true)]
+
+        // null
+        [TestCase("1.0.0", null, ExpectedResult = true)] // null is always less than any value per CompareTo remarks
         public bool IsNewerThan(string versionStrA, string versionStrB)
         {
             // arrange
             ISemanticVersion versionA = new SemanticVersion(versionStrA);
-            ISemanticVersion versionB = new SemanticVersion(versionStrB);
+            ISemanticVersion versionB = versionStrB != null
+                ? new SemanticVersion(versionStrB)
+                : null;
 
             // assert
-            Assert.AreEqual(versionA.IsNewerThan(versionB), versionA.IsNewerThan(versionB.ToString()), "The two signatures returned different results.");
+            Assert.AreEqual(versionA.IsNewerThan(versionB), versionA.IsNewerThan(versionB?.ToString()), "The two signatures returned different results.");
             return versionA.IsNewerThan(versionB);
         }
 
@@ -324,7 +339,7 @@ namespace SMAPI.Tests.Utilities
         /// <param name="versionStr">The main version.</param>
         /// <param name="lowerStr">The lower version number.</param>
         /// <param name="upperStr">The upper version number.</param>
-        [Test(Description = "Assert that version.IsNewerThan returns the expected value.")]
+        [Test(Description = "Assert that version.IsBetween returns the expected value.")]
         // is between
         [TestCase("0.5.7-beta.3", "0.5.7-beta.3", "0.5.7-beta.3", ExpectedResult = true)]
         [TestCase("1.0", "1.0", "1.1", ExpectedResult = true)]
@@ -332,6 +347,7 @@ namespace SMAPI.Tests.Utilities
         [TestCase("1.0", "0.5", "1.1", ExpectedResult = true)]
         [TestCase("1.0-beta.2", "1.0-beta.1", "1.0-beta.3", ExpectedResult = true)]
         [TestCase("1.0-beta-2", "1.0-beta-1", "1.0-beta-3", ExpectedResult = true)]
+        [TestCase("1.0.0", null, "1.0.0", ExpectedResult = true)] // null is always less than any value per CompareTo remarks
 
         // is not between
         [TestCase("1.0-beta", "1.0", "1.1", ExpectedResult = false)]
@@ -339,15 +355,20 @@ namespace SMAPI.Tests.Utilities
         [TestCase("1.0-beta.2", "1.1", "1.0", ExpectedResult = false)]
         [TestCase("1.0-beta.2", "1.0-beta.10", "1.0-beta.3", ExpectedResult = false)]
         [TestCase("1.0-beta-2", "1.0-beta-10", "1.0-beta-3", ExpectedResult = false)]
+        [TestCase("1.0.0", "1.0.0", null, ExpectedResult = false)] // null is always less than any value per CompareTo remarks
         public bool IsBetween(string versionStr, string lowerStr, string upperStr)
         {
             // arrange
-            ISemanticVersion lower = new SemanticVersion(lowerStr);
-            ISemanticVersion upper = new SemanticVersion(upperStr);
+            ISemanticVersion lower = lowerStr != null
+                ? new SemanticVersion(lowerStr)
+                : null;
+            ISemanticVersion upper = upperStr != null
+                ? new SemanticVersion(upperStr)
+                : null;
             ISemanticVersion version = new SemanticVersion(versionStr);
 
             // assert
-            Assert.AreEqual(version.IsBetween(lower, upper), version.IsBetween(lower.ToString(), upper.ToString()), "The two signatures returned different results.");
+            Assert.AreEqual(version.IsBetween(lower, upper), version.IsBetween(lower?.ToString(), upper?.ToString()), "The two signatures returned different results.");
             return version.IsBetween(lower, upper);
         }
 
diff --git a/src/SMAPI.Toolkit/SemanticVersion.cs b/src/SMAPI.Toolkit/SemanticVersion.cs
index 97b92555..41ae12eb 100644
--- a/src/SMAPI.Toolkit/SemanticVersion.cs
+++ b/src/SMAPI.Toolkit/SemanticVersion.cs
@@ -108,9 +108,9 @@ namespace StardewModdingAPI.Toolkit
         /// <inheritdoc />
         public int CompareTo(ISemanticVersion other)
         {
-            if (other == null)
-                throw new ArgumentNullException(nameof(other));
-            return this.CompareTo(other.MajorVersion, other.MinorVersion, other.PatchVersion, (other as SemanticVersion)?.PlatformRelease ?? 0, other.PrereleaseTag);
+            return other == null
+                ? 1
+                : this.CompareTo(other.MajorVersion, other.MinorVersion, other.PatchVersion, (other as SemanticVersion)?.PlatformRelease ?? 0, other.PrereleaseTag);
         }
 
         /// <inheritdoc />
@@ -134,7 +134,11 @@ namespace StardewModdingAPI.Toolkit
         /// <inheritdoc />
         public bool IsOlderThan(string other)
         {
-            return this.IsOlderThan(new SemanticVersion(other, allowNonStandard: true));
+            ISemanticVersion otherVersion = other != null
+                ? new SemanticVersion(other, allowNonStandard: true)
+                : null;
+
+            return this.IsOlderThan(otherVersion);
         }
 
         /// <inheritdoc />
@@ -146,7 +150,11 @@ namespace StardewModdingAPI.Toolkit
         /// <inheritdoc />
         public bool IsNewerThan(string other)
         {
-            return this.IsNewerThan(new SemanticVersion(other, allowNonStandard: true));
+            ISemanticVersion otherVersion = other != null
+                ? new SemanticVersion(other, allowNonStandard: true)
+                : null;
+
+            return this.IsNewerThan(otherVersion);
         }
 
         /// <inheritdoc />
@@ -158,7 +166,14 @@ namespace StardewModdingAPI.Toolkit
         /// <inheritdoc />
         public bool IsBetween(string min, string max)
         {
-            return this.IsBetween(new SemanticVersion(min, allowNonStandard: true), new SemanticVersion(max, allowNonStandard: true));
+            ISemanticVersion minVersion = min != null
+                ? new SemanticVersion(min, allowNonStandard: true)
+                : null;
+            ISemanticVersion maxVersion = max != null
+                ? new SemanticVersion(max, allowNonStandard: true)
+                : null;
+
+            return this.IsBetween(minVersion, maxVersion);
         }
 
         /// <inheritdoc cref="ISemanticVersion.ToString" />