From 4b4d81ba9380a1a47c5cab94e654858dafb64482 Mon Sep 17 00:00:00 2001
From: Daniel Chao <dan.chao@apple.com>
Date: Sun, 3 Nov 2024 20:31:04 -0800
Subject: [PATCH] Fix semver comparison logic (#773)

Fix `isLessThan` logic between semvers
---
 .../input/api/semverModule.pkl                | 16 +++++++++++++++
 .../output/api/semverModule.pcf               | 20 +++++++++++++++++++
 stdlib/semver.pkl                             | 20 +++++++++++--------
 3 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/api/semverModule.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/api/semverModule.pkl
index 204417699..f1c886084 100644
--- a/pkl-core/src/test/files/LanguageSnippetTests/input/api/semverModule.pkl
+++ b/pkl-core/src/test/files/LanguageSnippetTests/input/api/semverModule.pkl
@@ -15,6 +15,15 @@ examples {
     test.catch(() -> semver.Version("1.02.3"))
     test.catch(() -> semver.Version("aaa"))
   }
+
+  ["List.sortWith()"] {
+    List(
+      semver.Version("100.0.0"),
+      semver.Version("0.100.0"),
+      semver.Version("0.0.100")
+    )
+      .sortWith(semver.comparator)
+  }
 }
 
 facts {
@@ -123,6 +132,9 @@ facts {
 
     !semver.Version("1.2.3-alpha+def").isLessThan(semver.Version("1.2.3-alpha+abc"))
     !semver.Version("1.2.3-alpha+abc").isLessThan(semver.Version("1.2.3-alpha+def"))
+
+    semver.Version("1.2.3").isLessThan(semver.Version("2.0.100"))
+    semver.Version("1.2.3").isLessThan(semver.Version("2.100.0"))
   }
 
   ["Version.isLessThanOrEquals()"] {
@@ -141,6 +153,10 @@ facts {
     semver.Version("1.3.3").isGreaterThan(semver.Version("1.2.3"))
     semver.Version("1.2.4").isGreaterThan(semver.Version("1.2.3"))
     semver.Version("1.2.3-beta").isGreaterThan(semver.Version("1.2.3-alpha"))
+
+    semver.Version("2.0.100").isGreaterThan(semver.Version("1.2.3"))
+    semver.Version("2.100.0").isGreaterThan(semver.Version("1.2.3"))
+    semver.Version("2.0.100").isGreaterThan(semver.Version("1.2.3"))
   }
 
   ["Version.isGreaterThanOrEquals()"] {
diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/api/semverModule.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/api/semverModule.pcf
index 0b3e92958..82ab0dc57 100644
--- a/pkl-core/src/test/files/LanguageSnippetTests/output/api/semverModule.pcf
+++ b/pkl-core/src/test/files/LanguageSnippetTests/output/api/semverModule.pcf
@@ -92,6 +92,8 @@ facts {
     true
     true
     true
+    true
+    true
   }
   ["Version.isLessThanOrEquals()"] {
     true
@@ -108,6 +110,9 @@ facts {
     true
     true
     true
+    true
+    true
+    true
   }
   ["Version.isGreaterThanOrEquals()"] {
     true
@@ -172,4 +177,19 @@ examples {
     "`1.02.3` is not a valid semantic version number."
     "`aaa` is not a valid semantic version number."
   }
+  ["List.sortWith()"] {
+    List(new {
+      major = 0
+      minor = 0
+      patch = 100
+    }, new {
+      major = 0
+      minor = 100
+      patch = 0
+    }, new {
+      major = 100
+      minor = 0
+      patch = 0
+    })
+  }
 }
diff --git a/stdlib/semver.pkl b/stdlib/semver.pkl
index 56fe93b25..16dafe143 100644
--- a/stdlib/semver.pkl
+++ b/stdlib/semver.pkl
@@ -133,10 +133,10 @@ class Version {
   ///
   /// Note: `version1.equals(version2)` differs from `version1 == version2` in that it ignores [build].
   function equals(other: Version): Boolean =
-    major == other.major &&
-      minor == other.minor &&
-      patch == other.patch &&
-      preRelease == other.preRelease
+    major == other.major
+      && minor == other.minor
+      && patch == other.patch
+      && preRelease == other.preRelease
 
   /// Tells whether this version is less than [other] according to semantic versioning rules.
   ///
@@ -157,10 +157,14 @@ class Version {
   /// semver.Version("1.0.0-rc.1").isLessThan(semver.Version("1.0.0"))
   /// ```
   function isLessThan(other: Version): Boolean =
-    major < other.major ||
-    minor < other.minor ||
-    patch < other.patch ||
-    isPreReleaseLessThan(other)
+    major < other.major
+      || major == other.major && minor < other.minor
+      || major == other.major && minor == other.minor && patch < other.patch
+      ||
+        major == other.major
+          && minor == other.minor
+          && patch == other.patch
+          && isPreReleaseLessThan(other)
 
   /// Tells whether this version is less than or equal to [other] according to semantic versioning rules.
   function isLessThanOrEquals(other: Version): Boolean =