Use PackageIdentity.Scope instead of String where applicable

Author: mattt

Sources/Commands/SwiftPackageRegistryTool.swift

@@ -24,7 +24,7 @@ import Foundation
 import PackageRegistry
 
 private enum RegistryConfigurationError: Swift.Error {
-    case missingScope(String? = nil)
+    case missingScope(PackageIdentity.Scope? = nil)
     case invalidURL(String)
 }
 
@@ -90,6 +90,8 @@ public struct SwiftPackageRegistryTool: ParsableCommand {
                 throw RegistryConfigurationError.invalidURL(self.url)
             }
 
+            let scope = try scope.map(PackageIdentity.Scope.init(validating:))
+
             // TODO: Require login if password is specified
 
             let set: (inout RegistryConfiguration) throws -> Void = { configuration in
@@ -125,6 +127,8 @@ public struct SwiftPackageRegistryTool: ParsableCommand {
         var scope: String?
 
         func run(_ swiftTool: SwiftTool) throws {
+            let scope = try scope.map(PackageIdentity.Scope.init(validating:))
+
             let unset: (inout RegistryConfiguration) throws -> Void = { configuration in
                 if let scope = scope {
                     guard let _ = configuration.scopedRegistries[scope] else {
@@ -151,7 +155,6 @@ public struct SwiftPackageRegistryTool: ParsableCommand {
 
 // MARK: -
 
-
 private extension SwiftTool {
     func getRegistriesConfig() throws -> Workspace.Configuration.Registries {
         let localRegistriesFile = try Workspace.DefaultLocations.registriesConfigurationFile(forRootPackage: self.getPackageRoot())

Sources/PackageModel/PackageIdentity.swift

@@ -59,14 +59,14 @@ public struct PackageIdentity: Hashable, CustomStringConvertible {
     }
 
     // TODO: formalize package registry identifier
-    public var scopeAndName: (String, String)? {
+    public var scopeAndName: (Scope, Name)? {
         let components = description.split(separator: ".", maxSplits: 1, omittingEmptySubsequences: true)
-        guard let scope = components.first,
-              let name = components.last,
-              components.count == 2
+        guard components.count == 2,
+              let scope = Scope(components.first),
+              let name = Name(components.last)
         else { return nil }
 
-        return (String(scope), String(name))
+        return (scope, name)
     }
 }
 
@@ -136,6 +136,11 @@ extension PackageIdentity {
             self = scope
         }
 
+        fileprivate init?(_ substring: String.SubSequence?) {
+            guard let substring = substring else { return nil }
+            self.init(String(substring))
+        }
+
         // MARK: - Equatable & Comparable
 
         private func compare(to other: Scope) -> ComparisonResult {
@@ -213,6 +218,11 @@ extension PackageIdentity {
             self = name
         }
 
+        fileprivate init?(_ substring: String.SubSequence?) {
+            guard let substring = substring else { return nil }
+            self.init(String(substring))
+        }
+
         // MARK: - Equatable & Comparable
 
         private func compare(to other: Name) -> ComparisonResult {

Sources/PackageRegistry/RegistryConfiguration.swift

@@ -9,18 +9,17 @@
 */
 
 import Foundation
+import PackageModel
 
 public struct RegistryConfiguration: Hashable {
-    public typealias Scope = String
-
     public enum Version: Int, Codable {
         case v1 = 1
     }
 
     public static let version: Version = .v1
 
     public var defaultRegistry: Registry?
-    public var scopedRegistries: [Scope: Registry]
+    public var scopedRegistries: [PackageIdentity.Scope: Registry]
 
     public init() {
         self.defaultRegistry = nil
@@ -41,7 +40,7 @@ public struct RegistryConfiguration: Hashable {
         }
     }
 
-    public func registry(for scope: Scope) -> Registry? {
+    public func registry(for scope: PackageIdentity.Scope) -> Registry? {
         return scopedRegistries[scope] ?? defaultRegistry
     }
 }
@@ -79,9 +78,10 @@ extension RegistryConfiguration: Codable {
 
             self.defaultRegistry = try nestedContainer.decodeIfPresent(Registry.self, forKey: .default)
 
-            var scopedRegistries: [Scope: Registry] = [:]
+            var scopedRegistries: [PackageIdentity.Scope: Registry] = [:]
             for key in nestedContainer.allKeys where key != .default {
-                scopedRegistries[key.stringValue] = try nestedContainer.decode(Registry.self, forKey: key)
+                let scope = try PackageIdentity.Scope(validating: key.stringValue)
+                scopedRegistries[scope] = try nestedContainer.decode(Registry.self, forKey: key)
             }
             self.scopedRegistries = scopedRegistries
         case nil:
@@ -99,7 +99,7 @@ extension RegistryConfiguration: Codable {
         try nestedContainer.encodeIfPresent(defaultRegistry, forKey: .default)
 
         for (scope, registry) in scopedRegistries {
-            let key = ScopeCodingKey(stringValue: scope)
+            let key = ScopeCodingKey(stringValue: scope.description)
             try nestedContainer.encode(registry, forKey: key)
         }
     }

Sources/PackageRegistry/RegistryManager.swift

@@ -22,7 +22,7 @@ import struct Foundation.URLQueryItem
 import Dispatch
 
 public enum RegistryError: Error {
-    case registryNotConfigured(scope: String)
+    case registryNotConfigured(scope: PackageIdentity.Scope)
     case invalidPackage(PackageReference)
     case invalidOperation
     case invalidResponse
@@ -69,7 +69,7 @@ public final class RegistryManager {
         }
 
         var components = URLComponents(url: registry.url, resolvingAgainstBaseURL: true)
-        components?.appendPathComponents(scope, name)
+        components?.appendPathComponents("\(scope)", "\(name)")
 
         guard let url = components?.url else {
             return completion(.failure(RegistryError.invalidURL))
@@ -123,7 +123,7 @@ public final class RegistryManager {
         }
 
         var components = URLComponents(url: registry.url, resolvingAgainstBaseURL: true)
-        components?.appendPathComponents(scope, name, "\(version)", "Package.swift")
+        components?.appendPathComponents("\(scope)", "\(name)", "\(version)", "Package.swift")
         if let swiftLanguageVersion = swiftLanguageVersion {
             components?.queryItems = [
                 URLQueryItem(name: "swift-version", value: swiftLanguageVersion.rawValue)
@@ -207,7 +207,7 @@ public final class RegistryManager {
         }
 
         var components = URLComponents(url: registry.url, resolvingAgainstBaseURL: true)
-        components?.appendPathComponents(scope, name, "\(version).zip")
+        components?.appendPathComponents("\(scope)", "\(name)", "\(version).zip")
 
         guard let url = components?.url else {
             return completion(.failure(RegistryError.invalidURL))

Tests/CommandsTests/PackageRegistryToolTests.swift

@@ -161,6 +161,23 @@ final class PackageRegistryToolTests: XCTestCase {
         }
     }
 
+    func testSetInvalidScope() throws {
+        fixture(name: "DependencyResolution/External/Simple") { prefix in
+            let packageRoot = prefix.appending(component: "Bar")
+            let configurationFilePath = packageRoot.appending(RelativePath(".swiftpm/configuration/registries.json"))
+
+            XCTAssertFalse(localFileSystem.exists(configurationFilePath))
+
+            // Set default registry
+            do {
+                let result = try execute(["set", "--scope", "_invalid_", "\(defaultRegistryBaseURL)"], packagePath: packageRoot)
+                XCTAssertNotEqual(result.exitStatus, .terminated(code: 0))
+            }
+
+            XCTAssertFalse(localFileSystem.exists(configurationFilePath))
+        }
+    }
+
     func testUnsetMissingEntry() throws {
         fixture(name: "DependencyResolution/External/Simple") { prefix in
             let packageRoot = prefix.appending(component: "Bar")