[Swift migrate] Handle migration for optional features (#8711)

In addition to upcoming and experimental features, handle migration for
"optional" features (the only one now being StrictMemorySafety). Read
optional features from the supported-features JSON coming from the
compiler and use their provided flags.

While here, use the newly-introduced "categories" field from the JSON to
filter the list of categories rather than assuming it's the same as the
feature name.

Author: DougGregor

Fixtures/SwiftMigrate/InferIsolatedConformancesMigration/Package.swift

@@ -0,0 +1,10 @@
+// swift-tools-version:6.2
+
+import PackageDescription
+
+let package = Package(
+    name: "InferIsolatedConformancesMigration",
+    targets: [
+        .target(name: "Diagnostics", path: "Sources", exclude: ["Fixed"]),
+    ]
+)

Fixtures/SwiftMigrate/InferIsolatedConformancesMigration/Sources/Fixed/Test.swift

@@ -0,0 +1,8 @@
+@MainActor
+class C: nonisolated Equatable {
+  let name = "Hello"
+
+  nonisolated static func ==(lhs: C, rhs: C) -> Bool {
+    lhs.name == rhs.name
+  }
+}

Fixtures/SwiftMigrate/InferIsolatedConformancesMigration/Sources/Test.swift

@@ -0,0 +1,8 @@
+@MainActor
+class C: Equatable {
+  let name = "Hello"
+
+  nonisolated static func ==(lhs: C, rhs: C) -> Bool {
+    lhs.name == rhs.name
+  }
+}

Fixtures/SwiftMigrate/StrictMemorySafetyMigration/Package.swift

@@ -0,0 +1,10 @@
+// swift-tools-version:6.2
+
+import PackageDescription
+
+let package = Package(
+    name: "StrictMemorySafetyMigration",
+    targets: [
+        .target(name: "Diagnostics", path: "Sources", exclude: ["Fixed"]),
+    ]
+)

Fixtures/SwiftMigrate/StrictMemorySafetyMigration/Sources/Fixed/Test.swift

@@ -0,0 +1,5 @@
+@unsafe func f() { }
+
+func g() {
+  unsafe f()
+}

Fixtures/SwiftMigrate/StrictMemorySafetyMigration/Sources/Test.swift

@@ -0,0 +1,5 @@
+@unsafe func f() { }
+
+func g() {
+  f()
+}

Sources/Commands/PackageCommands/AddSetting.swift

@@ -29,7 +29,7 @@ extension SwiftPackageCommand {
             case experimentalFeature
             case upcomingFeature
             case languageMode
-            case strictMemorySafety
+            case strictMemorySafety = "StrictMemorySafety"
         }
 
         package static let configuration = CommandConfiguration(
@@ -150,8 +150,8 @@ extension SwiftPackageCommand {
                         manifest: manifestSyntax
                     )
                 case .strictMemorySafety:
-                    guard value.isEmpty else {
-                        throw ValidationError("'strictMemorySafety' doesn't have an argument")
+                    guard value.isEmpty || value == SwiftSetting.strictMemorySafety.rawValue else {
+                        throw ValidationError("'strictMemorySafety' does not support argument '\(value)'")
                     }
 
                     editResult = try AddSwiftSetting.strictMemorySafety(

Sources/Commands/PackageCommands/Migrate.swift

@@ -127,7 +127,7 @@ extension SwiftPackageCommand {
             for module in modules {
                 let fixit = try SwiftFixIt(
                     diagnosticFiles: module.diagnosticFiles,
-                    categories: Set(features.map(\.name)),
+                    categories: Set(features.flatMap(\.categories)),
                     fileSystem: swiftCommandState.fileSystem
                 )
                 try fixit.applyFixIts()
@@ -139,7 +139,7 @@ extension SwiftPackageCommand {
             print("> Updating manifest.")
             for module in modules.map(\.module) {
                 swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(module.name)'.")
-                self.updateManifest(
+                try self.updateManifest(
                     for: module.name,
                     add: features,
                     using: swiftCommandState
@@ -159,12 +159,7 @@ extension SwiftPackageCommand {
 
             let addFeaturesToModule = { (module: ResolvedModule) in
                 for feature in features {
-                    module.underlying.buildSettings.add(.init(values: [
-                        "-Xfrontend",
-                        "-enable-\(feature.upcoming ? "upcoming" : "experimental")-feature",
-                        "-Xfrontend",
-                        "\(feature.name):migrate",
-                    ]), for: .OTHER_SWIFT_FLAGS)
+                    module.underlying.buildSettings.add(.init(values: feature.migrationFlags), for: .OTHER_SWIFT_FLAGS)
                 }
             }
 
@@ -198,16 +193,11 @@ extension SwiftPackageCommand {
             for target: String,
             add features: [SwiftCompilerFeature],
             using swiftCommandState: SwiftCommandState
-        ) {
+        ) throws {
             typealias SwiftSetting = SwiftPackageCommand.AddSetting.SwiftSetting
 
-            let settings: [(SwiftSetting, String)] = features.map {
-                switch $0 {
-                case .upcoming(name: let name, migratable: _, enabledIn: _):
-                    (.upcomingFeature, "\(name)")
-                case .experimental(name: let name, migratable: _):
-                    (.experimentalFeature, "\(name)")
-                }
+            let settings: [(SwiftSetting, String)] = try features.map {
+                (try $0.swiftSetting, $0.name)
             }
 
             do {
@@ -218,10 +208,68 @@ extension SwiftPackageCommand {
                     verbose: !self.globalOptions.logging.quiet
                 )
             } catch {
-                swiftCommandState.observabilityScope.emit(error: "Could not update manifest for '\(target)' (\(error)). Please enable '\(features.map(\.name).joined(separator: ", "))' features manually.")
+                swiftCommandState.observabilityScope.emit(error: "Could not update manifest for '\(target)' (\(error)). Please enable '\(try features.map { try $0.swiftSettingDescription }.joined(separator: ", "))' features manually.")
             }
         }
 
         public init() {}
     }
 }
+
+fileprivate extension SwiftCompilerFeature {
+    /// Produce the set of command-line flags to pass to the compiler to enable migration for this feature.
+    var migrationFlags: [String] {
+        precondition(migratable)
+
+        switch self {
+        case .upcoming(name: let name, migratable: _, categories: _, enabledIn: _):
+            return ["-Xfrontend", "-enable-upcoming-feature", "-Xfrontend", "\(name):migrate"]
+        case .experimental(name: let name, migratable: _, categories: _):
+            return ["-Xfrontend", "-enable-experimental-feature", "-Xfrontend", "\(name):migrate"]
+        case .optional(name: _, migratable: _, categories: _, flagName: let flagName):
+            let flags = flagName.split(separator: " ")
+            var resultFlags: [String] = []
+            for (index, flag) in flags.enumerated() {
+                resultFlags.append("-Xfrontend")
+                if index == flags.endIndex - 1 {
+                    resultFlags.append(String(flag) + ":migrate")
+                } else {
+                    resultFlags.append(String(flag))
+                }
+            }
+
+            return resultFlags
+        }
+    }
+
+    /// Produce the Swift setting corresponding to this compiler feature.
+    var swiftSetting: SwiftPackageCommand.AddSetting.SwiftSetting {
+        get throws {
+            switch self {
+            case .upcoming:
+                return .upcomingFeature
+            case .experimental:
+                return .experimentalFeature
+            case .optional(name: "StrictMemorySafety", migratable: _, categories: _, flagName: _):
+                return .strictMemorySafety
+            case .optional(name: let name, migratable: _, categories: _, flagName: _):
+                throw InternalError("Unsupported optional feature: \(name)")
+            }
+        }
+    }
+
+    var swiftSettingDescription: String {
+        get throws {
+            switch self {
+            case .upcoming(name: let name, migratable: _, categories: _, enabledIn: _):
+                return #".enableUpcomingFeature("\#(name)")"#
+            case .experimental(name: let name, migratable: _, categories: _):
+                return #".enableExperimentalFeature("\#(name)")"#
+            case .optional(name: "StrictMemorySafety", migratable: _, categories: _, flagName: _):
+                return ".strictMemorySafety()"
+            case .optional(name: let name, migratable: _, categories: _, flagName: _):
+                throw InternalError("Unsupported optional feature: \(name)")
+            }
+        }
+    }
+}

Sources/PackageModel/Toolchain+SupportedFeatures.swift

@@ -13,43 +13,60 @@
 import Basics
 
 import enum TSCBasic.JSON
+import protocol TSCBasic.JSONMappable
 import TSCUtility
 
 public enum SwiftCompilerFeature {
-    case upcoming(name: String, migratable: Bool, enabledIn: SwiftLanguageVersion)
-    case experimental(name: String, migratable: Bool)
+    case optional(name: String, migratable: Bool, categories: [String], flagName: String)
+    case upcoming(name: String, migratable: Bool, categories: [String], enabledIn: SwiftLanguageVersion)
+    case experimental(name: String, migratable: Bool, categories: [String])
 
+    public var optional: Bool {
+        switch self {
+        case .optional: true
+        case .upcoming, .experimental: false
+        }
+    }
     public var upcoming: Bool {
         switch self {
         case .upcoming: true
-        case .experimental: false
+        case .optional, .experimental: false
         }
     }
 
     public var experimental: Bool {
         switch self {
-        case .upcoming: false
+        case .optional, .upcoming: false
         case .experimental: true
         }
     }
 
     public var name: String {
         switch self {
-        case .upcoming(name: let name, migratable: _, enabledIn: _):
-            name
-        case .experimental(name: let name, migratable: _):
+        case .optional(name: let name, migratable: _, categories: _, flagName: _),
+                .upcoming(name: let name, migratable: _, categories: _, enabledIn: _),
+                .experimental(name: let name, migratable: _, categories: _):
             name
         }
     }
 
     public var migratable: Bool {
         switch self {
-        case .upcoming(name: _, migratable: let migratable, enabledIn: _):
-            migratable
-        case .experimental(name: _, migratable: let migratable):
+        case .optional(name: _, migratable: let migratable, categories: _, flagName: _),
+             .upcoming(name: _, migratable: let migratable, categories: _, enabledIn: _),
+             .experimental(name: _, migratable: let migratable, categories: _):
             migratable
         }
     }
+
+    public var categories: [String] {
+        switch self {
+        case .optional(name: _, migratable: _, categories: let categories, flagName: _),
+             .upcoming(name: _, migratable: _, categories: let categories, enabledIn: _),
+             .experimental(name: _, migratable: _, categories: let categories):
+            categories
+        }
+    }
 }
 
 extension Toolchain {
@@ -85,8 +102,23 @@ extension Toolchain {
 
             let features: JSON = try parsedSupportedFeatures.get("features")
 
+            let optional: [SwiftCompilerFeature] = try (features.get("optional") as [JSON]?)?.map { (json: JSON) in
+                let name: String = try json.get("name")
+                let categories: [String]? = try json.getArrayIfAvailable("categories")
+                let migratable: Bool? = json.get("migratable")
+                let flagName: String = try json.get("flag_name")
+
+                return .optional(
+                    name: name,
+                    migratable: migratable ?? false,
+                    categories: categories ?? [name],
+                    flagName: flagName
+                )
+            } ?? []
+
             let upcoming: [SwiftCompilerFeature] = try features.getArray("upcoming").map {
                 let name: String = try $0.get("name")
+                let categories: [String]? = try $0.getArrayIfAvailable("categories")
                 let migratable: Bool? = $0.get("migratable")
                 let enabledIn: String = try $0.get("enabled_in")
 
@@ -97,21 +129,34 @@ extension Toolchain {
                 return .upcoming(
                     name: name,
                     migratable: migratable ?? false,
+                    categories: categories ?? [name],
                     enabledIn: mode
                 )
             }
 
             let experimental: [SwiftCompilerFeature] = try features.getArray("experimental").map {
                 let name: String = try $0.get("name")
+                let categories: [String]? = try $0.getArrayIfAvailable("categories")
                 let migratable: Bool? = $0.get("migratable")
 
                 return .experimental(
                     name: name,
-                    migratable: migratable ?? false
+                    migratable: migratable ?? false,
+                    categories: categories ?? [name]
                 )
             }
 
-            return upcoming + experimental
+            return optional + upcoming + experimental
+        }
+    }
+}
+
+fileprivate extension JSON {
+    func getArrayIfAvailable<T: JSONMappable>(_ key: String) throws -> [T]? {
+        do {
+            return try get(key)
+        } catch MapError.missingKey(key) {
+            return nil
         }
     }
 }

Sources/PackageModelSyntax/AddSwiftSetting.swift

@@ -73,7 +73,7 @@ public enum AddSwiftSetting {
         manifest: SourceFileSyntax
     ) throws -> PackageEditResult {
         try self.addToTarget(
-            target, name: "strictMemorySafety",
+            target, name: "strictMemorySafety()",
             value: String?.none,
             firstIntroduced: .v6_2,
             manifest: manifest