Skip to content

Conversation

@mattt
Copy link
Contributor

@mattt mattt commented Sep 16, 2021

This PR replaces the synthesized conformance to Equatable and Hashable and updates adoption of Comparable to use case-insensitive comparisons.

Motivation:

With SE-0292, package identifiers must be case-insensitive. For example, a dependency declared as mona.LinkedList is equivalent to MONA.LINKEDLIST.

This change also makes sense for packages identified by a remote URL* or local path**.

* Technically, URLs are case-sensitive. However, many if not most websites treat them as case-insensitive. This includes all of the popular code hosting services I'm aware of. I would be very surprised if any packages in the wild rely on remote packages differing only in case not being equivalent.

** Filesystems vary in case-sensitivity, so we should to normalize to a single case to make sure Swift packages behave the same on all platforms and don't cause vulnerabilities.

Modifications:

+ extension PackageIdentity: Equatable, Comparable {
+     private func compare(to other: PackageIdentity) -> ComparisonResult {
+         return self.description.caseInsensitiveCompare(other.description)
+     }
+ 
+     public static func == (lhs: PackageIdentity, rhs: PackageIdentity) -> Bool {
+         return lhs.compare(to: rhs) == .orderedSame
+     }
+ 
+     public static func < (lhs: PackageIdentity, rhs: PackageIdentity) -> Bool {
+         return lhs.compare(to: rhs) == .orderedAscending
+     }
+ 
+     public static func > (lhs: PackageIdentity, rhs: PackageIdentity) -> Bool {
+         return lhs.compare(to: rhs) == .orderedDescending
+     }
+ }

Result:

This shouldn't change any existing behavior. If it causes any new failures in our test suite, we should review to see if the expected behavior is correct.

@mattt
Copy link
Contributor Author

mattt commented Sep 16, 2021

@swift-ci Please smoke test

@tomerd tomerd merged commit 777a15a into swiftlang:main Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants