Fix more sources of non-determinism (#118)

Author: rockbruno

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelQueryParser.swift

@@ -59,7 +59,8 @@ enum BazelQueryParser {
     /// and converts them into BSP-compatible build targets with associated source files.
     ///
     /// - Parameters:
-    ///   - targets: Array of `BlazeQuery_Target` protobuf objects from Bazel query output
+    ///   - targets: Array of `BlazeQuery_Target` protobuf objects from Bazel query output (type: rule)
+    ///   - allSrcs: Array of `BlazeQuery_Target` protobuf objects from Bazel query output (type: source_file)
     ///   - rootUri: Absolute path to the project root directory
     ///   - toolchainPath: Absolute path to the development toolchain
     ///
@@ -68,6 +69,7 @@ enum BazelQueryParser {
     ///   - `[URI]`: Array of source file URIs associated with the target
     static func parseTargetsWithProto(
         from targets: [BlazeQuery_Target],
+        allSrcs: [BlazeQuery_Target],
         rootUri: String,
         toolchainPath: String,
     ) throws -> [(BuildTarget, [URI])] {
@@ -78,7 +80,7 @@ enum BazelQueryParser {
         // parse this info.
 
         var result: [(BuildTarget, [URI])] = []
-        let srcMap = buildSourceFilesMap(targets)
+        let srcMap = buildSourceFilesMap(allSrcs)
 
         for target in targets {
             guard target.type == .rule else {
@@ -108,7 +110,7 @@ enum BazelQueryParser {
                     let _deps: [BuildTargetIdentifier] = try attr.stringListValue.map {
                         let id = try $0.toTargetId(rootUri: rootUri)
                         return .init(uri: id)
-                    }
+                    }.sorted(by: { $0.uri.stringValue < $1.uri.stringValue })
                     deps = _deps
                 }
                 if attr.name == "srcs" {
@@ -122,7 +124,7 @@ enum BazelQueryParser {
                             return nil
                         }
                         return try URI(string: path)
-                    }
+                    }.sorted(by: { $0.stringValue < $1.stringValue })
                     srcs = _srcs
                 }
             }
@@ -162,10 +164,10 @@ enum BazelQueryParser {
     /// The `srcs` attribute is a list of source_file labels instead of URI, thus we need
     /// a hashmap to reduce the time complexity.
     private static func buildSourceFilesMap(
-        _ targets: [BlazeQuery_Target]
+        _ allSrcs: [BlazeQuery_Target]
     ) -> [String: String] {
         var srcMap: [String: String] = [:]
-        for target in targets {
+        for target in allSrcs {
             // making sure the target is a source_file type
             guard target.type == .sourceFile else {
                 continue

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelTargetDiscoverer.swift

@@ -65,6 +65,7 @@ public enum BazelTargetDiscoverer {
             .map { $0.trimmingCharacters(in: .whitespacesAndNewlines) }
             .filter { !$0.isEmpty }
             .map { $0.components(separatedBy: " (")[0] }
+            .sorted()
 
         if discoveredTargets.isEmpty {
             throw BazelTargetDiscovererError.noTargetsDiscovered

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelTargetQuerier.swift

@@ -39,7 +39,7 @@ final class BazelTargetQuerier {
 
     private let commandRunner: CommandRunner
 
-    private var queryCache = [String: [BlazeQuery_Target]]()
+    private var queryCache = [String: ([BlazeQuery_Target], [BlazeQuery_Target])]()
     private var dependencyGraphCache = [String: [String: [String]]]()
 
     static func queryDepsString(forTargets targets: [String]) -> String {
@@ -61,7 +61,7 @@ final class BazelTargetQuerier {
     func queryTargets(
         config: InitializedServerConfig,
         dependencyKinds: Set<String>,
-    ) throws -> [BlazeQuery_Target] {
+    ) throws -> (rules: [BlazeQuery_Target], srcs: [BlazeQuery_Target]) {
         if dependencyKinds.isEmpty {
             throw BazelTargetQuerierError.noKinds
         }
@@ -102,14 +102,31 @@ final class BazelTargetQuerier {
             rootUri: config.rootUri
         )
 
-        let result = try BazelProtobufBindings.parseCqueryResult(data: output)
+        let queryResult = try BazelProtobufBindings.parseCqueryResult(data: output)
 
-        let targets = result.results.map { $0.target }
+        let targets = queryResult.results.map { $0.target }
 
         logger.debug("Parsed \(targets.count, privacy: .public) targets for cache key: \(cacheKey, privacy: .public)")
-        queryCache[cacheKey] = targets
 
-        return targets
+        var rules = [BlazeQuery_Target]()
+        var srcs = [BlazeQuery_Target]()
+        for target in targets {
+            if target.type == .rule {
+                rules.append(target)
+            } else if target.type == .sourceFile {
+                srcs.append(target)
+            } else {
+                logger.error("Parsed unexpected target type: \(target.type.rawValue)")
+            }
+        }
+
+        // Sort for determinism
+        rules = rules.sorted(by: { $0.rule.name < $1.rule.name })
+        srcs = srcs.sorted(by: { $0.sourceFile.name < $1.sourceFile.name })
+
+        let result = (rules, srcs)
+        queryCache[cacheKey] = result
+        return result
     }
 
     func queryDependencyGraph(
@@ -168,6 +185,9 @@ final class BazelTargetQuerier {
             graph[source, default: []].append(contentsOf: targets)
         }
 
+        // Sort for determinism
+        graph = graph.mapValues { $0.sorted() }
+
         dependencyGraphCache[cacheKey] = graph
         return graph
     }

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelTargetStore.swift

@@ -214,7 +214,8 @@ final class BazelTargetStoreImpl: BazelTargetStore {
         // Query all the targets we are interested in one invocation:
         //  - Top-level targets (e.g. `ios_application`, `ios_unit_test`, etc.)
         //  - Dependencies of the top-level targets (e.g. `swift_library`, `objc_library`, etc.)
-        let allTargets =
+        //  - Source files connected to these targets
+        let (allTargets, allSrcs) =
             try bazelTargetQuerier
             .queryTargets(
                 config: initializedConfig,
@@ -254,6 +255,7 @@ final class BazelTargetStoreImpl: BazelTargetStore {
 
         let targetData = try BazelQueryParser.parseTargetsWithProto(
             from: dependencyTargets,
+            allSrcs: allSrcs,
             rootUri: initializedConfig.rootUri,
             toolchainPath: initializedConfig.devToolchainPath,
         )

Sources/SourceKitBazelBSP/RequestHandlers/SKOptions/AqueryResult.swift

@@ -51,6 +51,11 @@ struct AqueryResult: Hashable {
         let configurations: [UInt32: Analysis_Configuration] = results.configuration.reduce(into: [:]) {
             result,
             configuration in
+            if result.keys.contains(configuration.id) {
+                logger.error(
+                    "Duplicate configuration found when aquerying (\(configuration.id))! This is unexpected. Will ignore the duplicate."
+                )
+            }
             result[configuration.id] = configuration
         }
         self.targets = targets

Tests/SourceKitBazelBSPTests/BazelTargetDiscovererTests.swift

@@ -38,7 +38,7 @@ struct BazelTargetDiscovererTests {
             commandRunner: commandRunner,
         )
 
-        #expect(targets == ["//Example/HelloWorld:HelloWorld", "//Example/AnotherApp:AnotherApp"])
+        #expect(targets == ["//Example/AnotherApp:AnotherApp", "//Example/HelloWorld:HelloWorld"])
         #expect(commandRunner.commands.count == 1)
         #expect(commandRunner.commands[0].command == "fakeBazel cquery 'kind(\"ios_application\", ...)' --output label")
     }
@@ -81,7 +81,7 @@ struct BazelTargetDiscovererTests {
             commandRunner: commandRunner,
         )
 
-        #expect(targets == ["//Example/HelloWorld:HelloWorld", "//Example/AnotherApp:AnotherApp"])
+        #expect(targets == ["//Example/AnotherApp:AnotherApp", "//Example/HelloWorld:HelloWorld"])
         #expect(commandRunner.commands.count == 1)
         #expect(
             commandRunner.commands[0].command
@@ -105,7 +105,7 @@ struct BazelTargetDiscovererTests {
             commandRunner: commandRunner,
         )
 
-        #expect(targets == ["//Example/HelloWorld:HelloWorld", "//Example/AnotherApp:AnotherApp"])
+        #expect(targets == ["//Example/AnotherApp:AnotherApp", "//Example/HelloWorld:HelloWorld"])
         #expect(commandRunner.commands.count == 1)
         #expect(
             commandRunner.commands[0].command

Tests/SourceKitBazelBSPTests/BazelTargetParserTests.swift

@@ -63,7 +63,8 @@ struct BazelTargetParserTests {
         )
 
         let result = try BazelQueryParser.parseTargetsWithProto(
-            from: targets,
+            from: targets.rules,
+            allSrcs: targets.srcs,
             rootUri: rootUri,
             toolchainPath: toolchainPath,
         )

Tests/SourceKitBazelBSPTests/BazelTargetQuerierTests.swift

@@ -66,7 +66,8 @@ struct BazelTargetQuerierTests {
         #expect(ranCommands.count == 1)
         #expect(ranCommands[0].command == expectedCommand)
         #expect(ranCommands[0].cwd == mockRootUri)
-        #expect(!result.isEmpty)
+        #expect(result.rules.count > 0)
+        #expect(result.srcs.count > 0)
     }
 
     @Test
@@ -110,7 +111,8 @@ struct BazelTargetQuerierTests {
         #expect(ranCommands.count == 1)
         #expect(ranCommands[0].command == expectedCommand)
         #expect(ranCommands[0].cwd == mockRootUri)
-        #expect(!result.isEmpty)
+        #expect(result.rules.count > 0)
+        #expect(result.srcs.count > 0)
     }
 
     @Test
@@ -217,9 +219,7 @@ struct BazelTargetQuerierTests {
             dependencyKinds: dependencyKinds
         )
 
-        let rules = result.filter { target in
-            target.type == .rule
-        }
+        let rules = result.rules
 
         let ranCommands = runner.commands