Update for XcodeProj with strongly typed settings

- Updates to xcdiff for compatibility with tuist/XcodeProj#903 - Build setting values are no longer type erased and instead can now either be a `String` or `[String]` - This alleviates the need to casting values and throwing errors when dealing with build settings Test Plan: - Verify CI passes Signed-off-by: Kassem Wridan <[email protected]>
bloomberg · Feb 19, 2025 · 1b2ac14 · 1b2ac14
1 parent 99301ee
commit 1b2ac14
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 58 deletions.
diff --git a/Package.resolved b/Package.resolved
@@ -59,9 +59,9 @@
         "package": "XcodeProj",
         "repositoryURL": "https://github.com/tuist/xcodeproj.git",
         "state": {
-          "branch": null,
-          "revision": "6f90427e172da66336739801c84b9cef3e17367b",
-          "version": "8.26.6"
+          "branch": "waltflanagan/StrongTypes",
+          "revision": "bf96639f81663c8c032a0296e75c911041c95220",
+          "version": null
         }
       }
     ]

diff --git a/Package.swift b/Package.swift
@@ -25,7 +25,7 @@ let package = Package(
     dependencies: [
         .package(url: "https://github.com/apple/swift-tools-support-core.git", from: "0.3.0"),
         .package(url: "https://github.com/kylef/PathKit", from: "1.0.0"),
-        .package(url: "https://github.com/tuist/xcodeproj.git", from: "8.26.6"),
+        .package(url: "https://github.com/tuist/xcodeproj.git", branch: "waltflanagan/StrongTypes"),
         .package(url: "https://github.com/apple/swift-argument-parser", from: "1.5.0"),
     ],
     targets: [

diff --git a/Sources/XCDiffCore/Comparator/CopyFilesComparator.swift b/Sources/XCDiffCore/Comparator/CopyFilesComparator.swift
@@ -141,7 +141,7 @@ final class CopyFilesComparator: Comparator {
                 ) ?? buildFile.file?.path else {
                     return nil
                 }
-                let attributes = buildFile.settings?["ATTRIBUTES"] as? [String] ?? []
+                let attributes = buildFile.settings?["ATTRIBUTES"]?.arrayValue ?? []
 
                 return BuildFileDescriptor(
                     name: path,

diff --git a/Sources/XCDiffCore/Comparator/ResolvedSettingsComparator.swift b/Sources/XCDiffCore/Comparator/ResolvedSettingsComparator.swift
@@ -67,7 +67,7 @@ final class ResolvedSettingsComparator: Comparator {
                                                         secondBuildSettings)
     }
 
-    private func buildSettings(path: String, target: String, configuration: String) throws -> [String: String] {
+    private func buildSettings(path: String, target: String, configuration: String) throws -> [String: BuildSetting] {
         let command = extractBuildSettingsCommand(path: path, target: target, config: configuration)
         let output = try system.execute(arguments: command)
         do {
@@ -100,24 +100,36 @@ final class ResolvedSettingsComparator: Comparator {
         return arguments
     }
 
-    private func parseRawJsonBuildSettings(output: String) throws -> [String: String] {
+    private func parseRawJsonBuildSettings(output: String) throws -> [String: BuildSetting] {
         let data = output.data(using: .utf8)!
         let result = try jsonDecoder.decode([RawShowBuildSettingsItem].self, from: data)
         var buildSettings = result[0].buildSettings
         let replacementKeys: [String] = ["PROJECT_TEMP_DIR", "PROJECT_TEMP_ROOT", "BUILD_DIR", "PROJECT_FILE_PATH"]
-        let replacementValues = replacementKeys.compactMap { (key: String) -> (key: String, value: String)? in
+        let replacementValues = replacementKeys.compactMap { (key: String) -> (key: String, value: BuildSetting)? in
             buildSettings[key].map { (key: key, value: $0) }
         }
         replacementValues.forEach { replacement in
             buildSettings.forEach { setting in
-                buildSettings[setting.key] =
-                    setting.value.replacingOccurrences(of: replacement.value, with: "$(\(replacement.key))")
+                switch setting.value {
+                case let .string(stringValue):
+                    buildSettings[setting.key] =
+                        .string(
+                            stringValue.replacingOccurrences(of: "\(replacement.value)", with: "$(\(replacement.key))")
+                        )
+                case let .array(arrayValue):
+                    buildSettings[setting.key] =
+                        .array(
+                            arrayValue.map {
+                                $0.replacingOccurrences(of: "\(replacement.value)", with: "$(\(replacement.key))")
+                            }
+                        )
+                }
             }
         }
         return buildSettings
     }
 }
 
 private struct RawShowBuildSettingsItem: Decodable {
-    let buildSettings: [String: String]
+    let buildSettings: [String: BuildSetting]
 }
diff --git a/Sources/XCDiffCore/Library/PBX+Extensions.swift b/Sources/XCDiffCore/Library/PBX+Extensions.swift
@@ -61,3 +61,34 @@ extension XCRemoteSwiftPackageReference.VersionRequirement: CustomStringConverti
         }
     }
 }
+
+extension BuildSetting {
+    var stringValue: String {
+        switch self {
+        case let .string(value):
+            value
+        case let .array(value):
+            value.joined(separator: ", ")
+        }
+    }
+}
+
+extension BuildFileSetting {
+    var stringValue: String {
+        switch self {
+        case let .string(value):
+            value
+        case let .array(value):
+            value.joined(separator: ", ")
+        }
+    }
+
+    var arrayValue: [String] {
+        switch self {
+        case let .string(value):
+            [value]
+        case let .array(value):
+            value
+        }
+    }
+}
diff --git a/Sources/XCDiffCore/Library/SettingsHelper.swift b/Sources/XCDiffCore/Library/SettingsHelper.swift
@@ -29,16 +29,16 @@ class SettingsHelper {
         let onlyInSecond = secondKeys.subtractingAndSorted(firstKeys).map { keyAndValue($0, buildSettings: second) }
 
         // we attempt to ignore differences that are a result of different project names
-        let firstProjectName = first["PROJECT_NAME"] as? String
-        let secondProjectName = second["PROJECT_NAME"] as? String
+        let firstProjectName = first["PROJECT_NAME"]?.stringValue
+        let secondProjectName = second["PROJECT_NAME"]?.stringValue
         let settingValueComparator = SettingValueComparator(firstProjectName: firstProjectName,
                                                             secondProjectName: secondProjectName)
 
-        let valueDifferences: [CompareResult.DifferentValues] = try commonKeys.compactMap { settingName in
+        let valueDifferences: [CompareResult.DifferentValues] = commonKeys.compactMap { settingName in
             let firstSetting = first[settingName]
             let secondSettings = second[settingName]
-            let firstString = try stringFromBuildSetting(firstSetting)
-            let secondString = try stringFromBuildSetting(secondSettings)
+            let firstString = stringFromBuildSetting(firstSetting)
+            let secondString = stringFromBuildSetting(secondSettings)
             guard settingValueComparator.compare(firstString, secondString) == .orderedSame else {
                 return .init(context: settingName,
                              first: firstString,
@@ -57,25 +57,21 @@ class SettingsHelper {
     // MARK: - Private
 
     private func keyAndValue(_ key: String, buildSettings: BuildSettings) -> String {
-        return "\(key) = \(buildSettings[key] ?? "nil")"
+        return "\(key) = \(buildSettings[key]?.settingDiffValue ?? "nil")"
     }
 
-    private func stringFromBuildSetting(_ buildSetting: Any?) throws -> String {
-        // try to unwrap
-        guard let buildSetting else {
-            return "nil"
-        }
-
-        // try to cast to string
-        if let buildSettingString = buildSetting as? String {
-            return buildSettingString
-        }
+    private func stringFromBuildSetting(_ buildSetting: BuildSetting?) -> String {
+        buildSetting?.stringValue ?? "nil"
+    }
+}
 
-        // try to case to array
-        if let buildSettingArray = buildSetting as? NSArray {
-            return buildSettingArray.compactMap { $0 as? String }.joined(separator: " ")
+extension BuildSetting {
+    var settingDiffValue: String {
+        switch self {
+        case let .string(value):
+            value
+        case let .array(value):
+            "\(value)"
         }
-
-        throw ComparatorError.generic("Cannot convert build setting to string")
     }
 }
diff --git a/Sources/XCDiffCore/Library/TargetsHelper.swift b/Sources/XCDiffCore/Library/TargetsHelper.swift
@@ -318,30 +318,11 @@ final class TargetsHelper {
 
 private extension PBXBuildFile {
     func compilerFlags() -> String? {
-        guard let flags = settings?["COMPILER_FLAGS"] else {
-            return nil
-        }
-        if let flagsString = flags as? String {
-            return flagsString
-        }
-        if let flagsArray = flags as? [String] {
-            return flagsArray.joined(separator: ", ")
-        }
-
-        return nil
+        settings?["COMPILER_FLAGS"]?.stringValue
     }
 
     func attributes() -> String? {
-        guard let anyAttributes = settings?["ATTRIBUTES"] else {
-            return nil
-        }
-        if let attributes = anyAttributes as? [String] {
-            return attributes.joined(separator: ", ")
-        }
-        if let attributes = anyAttributes as? String {
-            return attributes
-        }
-        return String(describing: anyAttributes)
+        settings?["ATTRIBUTES"]?.stringValue
     }
 }
 

diff --git a/Tests/XCDiffCoreTests/Helpers/PBXBuildConfigurationBuilder.swift b/Tests/XCDiffCoreTests/Helpers/PBXBuildConfigurationBuilder.swift
@@ -19,15 +19,15 @@ import XcodeProj
 
 final class PBXBuildConfigurationBuilder {
     private let name: String
-    private var buildSettings: [String: Any] = [:]
+    private var buildSettings: [String: BuildSetting] = [:]
     private var baseConfiguration: PBXFileReference?
 
     init(name: String) {
         self.name = name
     }
 
     @discardableResult
-    func setValue(_ value: Any, forKey key: String) -> PBXBuildConfigurationBuilder {
+    func setValue(_ value: BuildSetting, forKey key: String) -> PBXBuildConfigurationBuilder {
         buildSettings[key] = value
         return self
     }

diff --git a/Tests/XCDiffCoreTests/Helpers/PBXBuildFileBuilder.swift b/Tests/XCDiffCoreTests/Helpers/PBXBuildFileBuilder.swift
@@ -22,7 +22,7 @@ final class PBXBuildFileBuilder {
     private var name: String?
     private var platformFilter: String?
     private var platformFilters: [String]?
-    private var settings: [String: Any]?
+    private var settings: [String: BuildFileSetting]?
     private var packageProduct: SwiftPackageProductDependencyData?
 
     @discardableResult
@@ -51,13 +51,13 @@ final class PBXBuildFileBuilder {
 
     @discardableResult
     func setSettings(_ settings: [String: [String]]) -> PBXBuildFileBuilder {
-        self.settings = settings
+        self.settings = settings.mapValues { .array($0) }
         return self
     }
 
     @discardableResult
     func setSettings(_ settings: [String: String]) -> PBXBuildFileBuilder {
-        self.settings = settings
+        self.settings = settings.mapValues { .string($0) }
         return self
     }