Fix unstable reads for XCSchemeManagement (#758)

Resolves: #756

- Reading the same `xcschememanagement.plist` file was resulting in different `XCSchemeManagement` objects
- This was due to the internal conversion of the `schemeUserState` from a dictionary to an array without sorting the dictionary key
- This lead to having differently ordered `XCSchemeManagement` arrays each time the plist was read
- Wrties were stable (going from `XCSchemeManagement` > `plist`) is most likely why this wasn't previously noticed
- To address this, they dictionary elements are sorted by key name
- Read stability tests have also been added and fixture updated to include more entries to aid with testing

Note: it's unclear why the `schemeUserState` is stored as an array, it's a candidate to be changed to dictionary in the next major release as it would be a breaking change to do so now without any compatibility accessors to maintain the same public API.

Test Plan:

- Verify unit tests pass
- Verify writing an `xcschememanagement.plist` file remains unchanged by these changes

Author: kwridan

Fixtures/Schemes/xcschememanagement.plist

@@ -4,17 +4,36 @@
 <dict>
 	<key>SchemeUserState</key>
 	<dict>
+		<key>App.xcscheme</key>
+		<dict>
+			<key>isShown</key>
+			<false/>
+			<key>orderHint</key>
+			<integer>0</integer>
+		</dict>
+		<key>Test 0.xcscheme</key>
+		<dict>
+			<key>orderHint</key>
+			<integer>3</integer>
+		</dict>
+		<key>Test 1.xcscheme</key>
+		<dict>
+			<key>isShown</key>
+			<false/>
+			<key>orderHint</key>
+			<integer>4</integer>
+		</dict>
 		<key>Tuist.xcscheme_^#shared#^_</key>
 		<dict>
 			<key>isShown</key>
 			<true/>
 			<key>orderHint</key>
-			<integer>0</integer>
+			<integer>1</integer>
 		</dict>
 		<key>XcodeProj.xcscheme</key>
 		<dict>
 			<key>orderHint</key>
-			<integer>1</integer>
+			<integer>2</integer>
 		</dict>
 	</dict>
 	<key>SuppressBuildableAutocreation</key>

Sources/XcodeProj/Scheme/XCSchemeManagement.swift

@@ -150,7 +150,9 @@ public struct XCSchemeManagement: Codable, Equatable, Writable {
         let container = try decoder.container(keyedBy: CodingKeys.self)
         self.suppressBuildableAutocreation = try container.decodeIfPresent(.suppressBuildableAutocreation)
         if let schemeUserStateDictionary = try container.decodeIfPresent([String: Any].self, forKey: .schemeUserState) {
-            self.schemeUserState = try schemeUserStateDictionary.compactMap({ (key, value) -> XCSchemeManagement.UserStateScheme? in
+            self.schemeUserState = try schemeUserStateDictionary
+                .sorted(by: { $0.key < $1.key })
+                .compactMap({ (key, value) -> XCSchemeManagement.UserStateScheme? in
                 var name = key
                 guard var valueDictionary = value as? [String: Any] else { return nil }
                 if key.contains("_^#shared#^_") {

Tests/XcodeProjTests/Scheme/XCSchemeManagementTests.swift

@@ -13,26 +13,37 @@ final class XCSchemeManagementTests: XCTestCase {
         let got = try XCSchemeManagement.init(path: path)
 
         // Then
-        let autocreationTarget = try XCTUnwrap(got.suppressBuildableAutocreation?["E525238B16245A900012E2BA"])
-        XCTAssertEqual(autocreationTarget.primary, true)
-        
-        let tuistScheme = try XCTUnwrap(got.schemeUserState?.first(where: {$0.name == "Tuist.xcscheme"}))
-        XCTAssertEqual(tuistScheme.name, "Tuist.xcscheme")
-        XCTAssertTrue(tuistScheme.shared)
-        XCTAssertEqual(tuistScheme.isShown, true)
-        XCTAssertEqual(tuistScheme.orderHint, 0)
-        
-        let xcodeprojScheme = try XCTUnwrap(got.schemeUserState?.first(where: {$0.name == "XcodeProj.xcscheme"}))
-        XCTAssertEqual(xcodeprojScheme.name, "XcodeProj.xcscheme")
-        XCTAssertFalse(xcodeprojScheme.shared)
-        XCTAssertNil(xcodeprojScheme.isShown)
-        XCTAssertEqual(xcodeprojScheme.orderHint, 1)
+        XCTAssertEqual(got.suppressBuildableAutocreation, [
+            "E525238B16245A900012E2BA": .init(primary: true),
+        ])
+
+        XCTAssertEqual(got.schemeUserState, [
+            .init(name: "App.xcscheme", shared: false, orderHint: 0, isShown: false),
+            .init(name: "Test 0.xcscheme", shared: false, orderHint: 3, isShown: nil),
+            .init(name: "Test 1.xcscheme", shared: false, orderHint: 4, isShown: false),
+            .init(name: "Tuist.xcscheme", shared: true, orderHint: 1, isShown: true),
+            .init(name: "XcodeProj.xcscheme", shared: false, orderHint: 2, isShown: nil),
+        ])
     }
 
     func test_read_write_produces_no_diff() throws {
         try testReadWriteProducesNoDiff(from: xcschememanagementPath, initModel: XCSchemeManagement.init(path:))
     }
 
+    func test_read_is_stable() throws {
+        // Given
+        let path = xcschememanagementPath
+
+        // When
+        let reads = try (0..<10).map { _ in
+            try XCSchemeManagement(path: path)
+        }
+
+        // Then
+        let unstableReads = reads.dropFirst().filter { $0 != reads.first }
+        XCTAssertTrue(unstableReads.isEmpty)
+    }
+
     func test_write_produces_no_diff() throws {
         let tmpDir = try Path.uniqueTemporary()
         defer {
@@ -42,8 +53,17 @@ final class XCSchemeManagementTests: XCTestCase {
         try tmpDir.chdir {
             // Write
             let plistPath = tmpDir + "xcschememanagement.plist"
-            let subject = XCSchemeManagement(schemeUserState: [.init(name: "Test.xcscheme", shared: true, orderHint: 0, isShown: true)],
-                                             suppressBuildableAutocreation: ["E525238B16245A900012E2BA": .init(primary: true)])
+            let subject = XCSchemeManagement(
+                schemeUserState: [
+                    .init(name: "Test 0.xcscheme", shared: true, orderHint: 0, isShown: true),
+                    .init(name: "Test 1.xcscheme", shared: true, orderHint: 1, isShown: true),
+                    .init(name: "Test 2.xcscheme", shared: true, orderHint: 2, isShown: false),
+                    .init(name: "Test 3.xcscheme", shared: true, orderHint: 3, isShown: true),
+                ],
+                suppressBuildableAutocreation: [
+                    "E525238B16245A900012E2BA": .init(primary: true),
+                ]
+            )
             try subject.write(path: plistPath, override: true)
 
             // Create a commit