From a67dd6dc5ee9a994124acddb8ce6e0a4cee0d964 Mon Sep 17 00:00:00 2001 From: Chris Eplett Date: Mon, 12 Jun 2023 11:47:43 -0400 Subject: [PATCH 1/2] Fix comparison issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As implemented, there were a couple of problems where the library was not performing comparisons between semvers correctly. The first issue is that the presence of buildmetadata was causing the library to treat the version as unstable, which also meant that, given the comparison implementation, the buildmetadata was being considered when comparing two semvers. This is a violation of Section 10 of the semver.org spec, which specifies that “Build metadata MUST be ignored when determining version precedence”. Additionally, it is not specified by semver.org that that the presence of buildmetadata should be involved in determining the stability of a semver. The second problem is that the library uses simple ASCII lexical ordering to compare the pre-release string of two semvers, which does not match the rules defined in Section 11.4 of the semver specification. This change addresses these problems with the following changes 1) Remove the build.isEmpty check from the implementation of the isStable property 2) Implement a PreReleaseIdentifier enum and add Comparable conformance to an array of these types, per the rules laid out in Section 11.4 of the semver specification 3) Add a computed property that returns an array of PreReleaseIndentifiers by parsing the preRelease string 4) Update the less than operator implementation to apply the above changes appropriately 5) Add additional test cases to verify the new behavior Testing: 1) Existing unit tests pass 2) New unit tests pass --- Sources/SemanticVersion/SemanticVersion.swift | 80 ++++++++++++++++--- .../SemanticVersionTests.swift | 11 ++- 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/Sources/SemanticVersion/SemanticVersion.swift b/Sources/SemanticVersion/SemanticVersion.swift index fb4a69d..f772b81 100644 --- a/Sources/SemanticVersion/SemanticVersion.swift +++ b/Sources/SemanticVersion/SemanticVersion.swift @@ -43,6 +43,11 @@ public struct SemanticVersion: Codable, Equatable, Hashable { self.preRelease = preRelease self.build = build } + + public enum PreReleaseIdentifier: Equatable { + case alphanumeric(String) + case numeric(Int) + } } @@ -74,26 +79,81 @@ extension SemanticVersion: Comparable { if lhs.major != rhs.major { return lhs.major < rhs.major } if lhs.minor != rhs.minor { return lhs.minor < rhs.minor } if lhs.patch != rhs.patch { return lhs.patch < rhs.patch } - if lhs.preRelease != rhs.preRelease { - // Ensure stable versions sort after their betas ... - if lhs.isStable { return false } - if rhs.isStable { return true } - // ... otherwise sort by preRelease - return lhs.preRelease < rhs.preRelease - } - // ... and build - return lhs.build < rhs.build + + // A stable release takes precedence over a pre-release + if lhs.isStable != rhs.isStable { return rhs.isStable } + + // Otherwise compare the pre-releases per section 11.4 + // See: https://semver.org/#spec-item-11 + return lhs.preReleaseIdentifiers < rhs.preReleaseIdentifiers + + // Note that per section 10, buildmetadata MUST not be + // considered when determining precedence + // See: https://semver.org/#spec-item-10 } } extension SemanticVersion { - public var isStable: Bool { return preRelease.isEmpty && build.isEmpty } + public var isStable: Bool { return preRelease.isEmpty } public var isPreRelease: Bool { return !isStable } public var isMajorRelease: Bool { return isStable && (major > 0 && minor == 0 && patch == 0) } public var isMinorRelease: Bool { return isStable && (minor > 0 && patch == 0) } public var isPatchRelease: Bool { return isStable && patch > 0 } public var isInitialRelease: Bool { return self == .init(0, 0, 0) } + + public var preReleaseIdentifiers: [PreReleaseIdentifier] { + return preRelease + .split(separator: ".") + .map { PreReleaseIdentifier(String($0)) } + } +} + +extension SemanticVersion.PreReleaseIdentifier { + init(_ rawValue: String) { + if let number = Int(rawValue) { + self = .numeric(number) + } else { + self = .alphanumeric(rawValue) + } + } +} + +extension SemanticVersion.PreReleaseIdentifier: Comparable { + public static func < (lhs: Self, rhs: Self) -> Bool { + // These rules are laid out in section 11.4 of the semver spec + // See: https://semver.org/#spec-item-11 + switch (lhs, rhs) { + case (.numeric, .alphanumeric): + // 11.4.3 - Numeric identifiers always have lower precedence than non-numeric identifiers + return true + case (.alphanumeric, .numeric): + // 11.4.3 - Numeric identifiers always have lower precedence than non-numeric identifiers + return false + case (.numeric(let lhInt), .numeric(let rhInt)): + // 11.4.1 - Identifiers consisting of only digits are compared numerically + return lhInt < rhInt + case (.alphanumeric(let lhString), .alphanumeric(let rhString)): + // 11.4.2 - Identifiers with letters or hyphens are compared lexically in ASCII sort order + return lhString < rhString + } + } +} + + +extension Array: Comparable where Element == SemanticVersion.PreReleaseIdentifier { + public static func < (lhs: Self, rhs: Self) -> Bool { + // Per section 11.4 of the semver spec, compare left to right until a + // difference is found. + // See: https://semver.org/#spec-item-11 + for (lhIdentifier, rhIdentifier) in zip(lhs, rhs) { + if lhIdentifier != rhIdentifier { return lhIdentifier < rhIdentifier } + } + + // 11.4.4 - A larger set of identifiers will have a higher precendence + // than a smaller set, if all the preceding identifiers are equal. + return lhs.count < rhs.count + } } #if swift(>=5.5) diff --git a/Tests/SemanticVersionTests/SemanticVersionTests.swift b/Tests/SemanticVersionTests/SemanticVersionTests.swift index 1accd26..ba26755 100644 --- a/Tests/SemanticVersionTests/SemanticVersionTests.swift +++ b/Tests/SemanticVersionTests/SemanticVersionTests.swift @@ -124,7 +124,8 @@ final class SemanticVersionTests: XCTestCase { XCTAssert(SemanticVersion(1, 0, 0) < SemanticVersion(1, 1, 0)) XCTAssert(SemanticVersion(1, 0, 0) < SemanticVersion(1, 0, 1)) XCTAssert(SemanticVersion(1, 0, 0, "a") < SemanticVersion(1, 0, 0, "b")) - XCTAssert(SemanticVersion(1, 0, 0, "a", "a") < SemanticVersion(1, 0, 0, "a", "b")) + XCTAssertLessThan(SemanticVersion(1, 0, 0, "alpha.2"), SemanticVersion(1, 0, 0, "alpha.11")) + XCTAssertLessThan(SemanticVersion(1, 0, 0, "alpha.2"), SemanticVersion(1, 0, 0, "alpha.2.1")) // ensure betas come before releases XCTAssert(SemanticVersion(1, 0, 0, "b1") < SemanticVersion(1, 0, 0)) @@ -133,6 +134,11 @@ final class SemanticVersionTests: XCTestCase { XCTAssert(SemanticVersion(1, 0, 0) < SemanticVersion(1, 0, 1, "b1")) // once the patch bumps up to the beta level again, it sorts higher XCTAssert(SemanticVersion(1, 0, 1) > SemanticVersion(1, 0, 1, "b1")) + + // Ensure metadata is not considered + XCTAssertFalse(SemanticVersion(1, 0, 0, "a", "a") < SemanticVersion(1, 0, 0, "a", "b")) + XCTAssertFalse(SemanticVersion(1, 0, 0, "a", "a") > SemanticVersion(1, 0, 0, "a", "b")) + XCTAssertLessThan(SemanticVersion(1, 0, 0, "alpha", "build1"), SemanticVersion(1, 0, 0, "", "build1")) } func test_isStable() throws { @@ -140,7 +146,8 @@ final class SemanticVersionTests: XCTestCase { XCTAssert(SemanticVersion(1, 0, 0, "").isStable) XCTAssert(SemanticVersion(1, 0, 0, "", "").isStable) XCTAssertFalse(SemanticVersion(1, 0, 0, "a").isStable) - XCTAssertFalse(SemanticVersion(1, 0, 0, "", "a").isStable) + XCTAssertTrue(SemanticVersion(1, 0, 0, "", "a").isStable) + XCTAssertFalse(SemanticVersion(1, 0, 0, "a", "b").isStable) } func test_isMajorRelease() throws { From 85444439453b1b05d3d5820807f2052cd541f88d Mon Sep 17 00:00:00 2001 From: Chris Eplett Date: Mon, 12 Jun 2023 12:02:51 -0400 Subject: [PATCH 2/2] Fix a unit test --- Tests/SemanticVersionTests/SemanticVersionTests.swift | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Tests/SemanticVersionTests/SemanticVersionTests.swift b/Tests/SemanticVersionTests/SemanticVersionTests.swift index ba26755..a3ad6c6 100644 --- a/Tests/SemanticVersionTests/SemanticVersionTests.swift +++ b/Tests/SemanticVersionTests/SemanticVersionTests.swift @@ -124,8 +124,7 @@ final class SemanticVersionTests: XCTestCase { XCTAssert(SemanticVersion(1, 0, 0) < SemanticVersion(1, 1, 0)) XCTAssert(SemanticVersion(1, 0, 0) < SemanticVersion(1, 0, 1)) XCTAssert(SemanticVersion(1, 0, 0, "a") < SemanticVersion(1, 0, 0, "b")) - XCTAssertLessThan(SemanticVersion(1, 0, 0, "alpha.2"), SemanticVersion(1, 0, 0, "alpha.11")) - XCTAssertLessThan(SemanticVersion(1, 0, 0, "alpha.2"), SemanticVersion(1, 0, 0, "alpha.2.1")) + XCTAssertFalse(SemanticVersion(1, 0, 0, "a", "a") < SemanticVersion(1, 0, 0, "a", "b")) // ensure betas come before releases XCTAssert(SemanticVersion(1, 0, 0, "b1") < SemanticVersion(1, 0, 0)) @@ -135,10 +134,8 @@ final class SemanticVersionTests: XCTestCase { // once the patch bumps up to the beta level again, it sorts higher XCTAssert(SemanticVersion(1, 0, 1) > SemanticVersion(1, 0, 1, "b1")) - // Ensure metadata is not considered - XCTAssertFalse(SemanticVersion(1, 0, 0, "a", "a") < SemanticVersion(1, 0, 0, "a", "b")) - XCTAssertFalse(SemanticVersion(1, 0, 0, "a", "a") > SemanticVersion(1, 0, 0, "a", "b")) - XCTAssertLessThan(SemanticVersion(1, 0, 0, "alpha", "build1"), SemanticVersion(1, 0, 0, "", "build1")) + // Ensure a release with build metadata sorts above a pre-release + XCTAssert(SemanticVersion(1, 0, 1, "alpha") < SemanticVersion(1, 0, 1, "", "build.14")) } func test_isStable() throws { @@ -147,7 +144,6 @@ final class SemanticVersionTests: XCTestCase { XCTAssert(SemanticVersion(1, 0, 0, "", "").isStable) XCTAssertFalse(SemanticVersion(1, 0, 0, "a").isStable) XCTAssertTrue(SemanticVersion(1, 0, 0, "", "a").isStable) - XCTAssertFalse(SemanticVersion(1, 0, 0, "a", "b").isStable) } func test_isMajorRelease() throws {