Reply faster to TargetSources

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelTargetQuerier.swift

@@ -133,6 +133,7 @@ final class BazelTargetQuerier {
             userProvidedTargets: userProvidedTargets,
             supportedTopLevelRuleTypes: supportedTopLevelRuleTypes,
             rootUri: config.rootUri,
+            executionRoot: config.executionRoot,
             toolchainPath: config.devToolchainPath,
         )
 

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelTargetQuerierParser.swift

@@ -76,6 +76,7 @@ protocol BazelTargetQuerierParser: AnyObject {
         userProvidedTargets: [String],
         supportedTopLevelRuleTypes: [TopLevelRuleType],
         rootUri: String,
+        executionRoot: String,
         toolchainPath: String,
     ) throws -> ProcessedCqueryResult
 
@@ -94,6 +95,7 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
         userProvidedTargets: [String],
         supportedTopLevelRuleTypes: [TopLevelRuleType],
         rootUri: String,
+        executionRoot: String,
         toolchainPath: String,
     ) throws -> ProcessedCqueryResult {
         let cquery = try BazelProtobufBindings.parseCqueryResult(data: data)
@@ -230,7 +232,7 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
             )
         }
 
-        var result: [String: (BuildTarget, [URI])] = [:]
+        var result: [String: (BuildTarget, SourcesItem)] = [:]
         var dependencyGraph: [String: [String]] = [:]
         for target in dependencyTargets {
             guard target.type == .rule else {
@@ -239,10 +241,17 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
             }
 
             let rule = target.rule
-            let id: URI = try rule.name.toTargetId(rootUri: rootUri)
+            let idUri: URI = try rule.name.toTargetId(rootUri: rootUri)
+            let id = BuildTargetIdentifier(uri: idUri)
             let baseDirectory: URI = try rule.name.toBaseDirectory(rootUri: rootUri)
 
-            let srcs = try processSrcsAttr(rule: rule, srcToUriMap: srcToUriMap)
+            let sourcesItem = try processSrcsAttr(
+                rule: rule,
+                targetId: id,
+                srcToUriMap: srcToUriMap,
+                rootUri: rootUri,
+                executionRoot: executionRoot
+            )
             let deps = try processDependenciesAttr(
                 rule: rule,
                 isBuildTestRule: false,
@@ -269,7 +278,7 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
             }
 
             let buildTarget = BuildTarget(
-                id: BuildTargetIdentifier(uri: id),
+                id: id,
                 displayName: rule.name,
                 baseDirectory: baseDirectory,
                 tags: [.library],
@@ -281,7 +290,7 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
                     toolchain: URI(string: "file://" + toolchainPath)
                 ).encodeToLSPAny()
             )
-            result[rule.name] = (buildTarget, srcs)
+            result[rule.name] = (buildTarget, sourcesItem)
         }
 
         // Determine which dependencies belong to which top-level targets.
@@ -307,7 +316,7 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
                 // If we don't know how to parse the full path to a target, we need to drop it.
                 // Otherwise we will not know how to properly communicate this target's capabilities to sourcekit-lsp.
                 logger.warning(
-                    "Skipping orphan target \(label, privacy: .public). This can happen if the target is a dependency of something we don't know how to parse."
+                    "Skipping orphan target \(label, privacy: .public). This can happen if the target is a dependency of a test host or of something we don't know how to parse."
                 )
                 result[label] = nil
             }
@@ -318,23 +327,23 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
         )
 
         var bspURIsToBazelLabelsMap: [URI: String] = [:]
-        var bspURIsToSrcsMap: [URI: [URI]] = [:]
+        var bspURIsToSrcsMap: [URI: SourcesItem] = [:]
         var srcToBspURIsMap: [URI: [URI]] = [:]
         var availableBazelLabels: Set<String> = []
         var topLevelLabelToRuleMap: [String: TopLevelRuleType] = [:]
         for dependencyTargetInfo in buildTargets {
             let target = dependencyTargetInfo.value.0
-            let srcs = dependencyTargetInfo.value.1
+            let sourcesItem = dependencyTargetInfo.value.1
             guard let displayName = target.displayName else {
                 // Should not happen, but the property is an optional
                 continue
             }
             let uri = target.id.uri
             bspURIsToBazelLabelsMap[uri] = displayName
-            bspURIsToSrcsMap[uri] = srcs
+            bspURIsToSrcsMap[uri] = sourcesItem
             availableBazelLabels.insert(displayName)
-            for src in srcs {
-                srcToBspURIsMap[src, default: []].append(uri)
+            for src in sourcesItem.sources {
+                srcToBspURIsMap[src.uri, default: []].append(uri)
             }
         }
         for (target, ruleType) in topLevelTargets {
@@ -369,8 +378,11 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
 
     private func processSrcsAttr(
         rule: BlazeQuery_Rule,
+        targetId: BuildTargetIdentifier,
         srcToUriMap: [String: URI],
-    ) throws -> [URI] {
+        rootUri: String,
+        executionRoot: String
+    ) throws -> SourcesItem {
         let srcsAttribute = rule.attribute.first { $0.name == "srcs" }
         let srcs: [URI]
         if let attr = srcsAttribute {
@@ -390,7 +402,80 @@ final class BazelTargetQuerierParserImpl: BazelTargetQuerierParser {
         } else {
             srcs = []
         }
-        return srcs
+        return SourcesItem(
+            target: targetId,
+            sources: srcs.map {
+                buildSourceItem(
+                    forSrc: $0,
+                    rootUri: rootUri,
+                    executionRoot: executionRoot
+                )
+            }
+        )
+    }
+
+    private func buildSourceItem(
+        forSrc src: URI,
+        rootUri: String,
+        executionRoot: String
+    ) -> SourceItem {
+        let srcString = src.stringValue
+        let copyDestinations = srcCopyDestinations(for: src, rootUri: rootUri, executionRoot: executionRoot)
+        let kind: SourceKitSourceItemKind
+        if srcString.hasSuffix("h") {
+            kind = .header
+        } else {
+            kind = .source
+        }
+        let language: Language?
+        if srcString.hasSuffix("swift") {
+            language = .swift
+        } else if srcString.hasSuffix("m") || kind == .header {
+            language = .objective_c
+        } else {
+            language = nil
+        }
+        return SourceItem(
+            uri: src,
+            kind: .file,
+            generated: false,  // FIXME: Need to handle this properly
+            dataKind: .sourceKit,
+            data: SourceKitSourceItemData(
+                language: language,
+                kind: kind,
+                outputPath: nil,
+                copyDestinations: copyDestinations
+            ).encodeToLSPAny()
+        )
+    }
+
+    /// The path sourcekit-lsp has is the "real" path of the file,
+    /// but Bazel works by copying them over to the execroot.
+    /// This method calculates this fake path so that sourcekit-lsp can
+    /// map the file back to the original workspace path for features like jump to definition.
+    private func srcCopyDestinations(
+        for src: URI,
+        rootUri: String,
+        executionRoot: String
+    ) -> [DocumentURI]? {
+        guard let srcPath = src.fileURL?.path else {
+            return nil
+        }
+
+        guard srcPath.hasPrefix(rootUri) else {
+            return nil
+        }
+
+        var relativePath = srcPath.dropFirst(rootUri.count)
+        // Not sure how much we can assume about rootUri, so adding this as an edge-case check
+        if relativePath.first == "/" {
+            relativePath = relativePath.dropFirst()
+        }
+
+        let newPath = executionRoot + "/" + String(relativePath)
+        return [
+            DocumentURI(filePath: newPath, isDirectory: false)
+        ]
     }
 
     private func processDependenciesAttr(

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/BazelTargetStore.swift

@@ -33,7 +33,7 @@ protocol BazelTargetStore: AnyObject {
     var isInitialized: Bool { get }
     func fetchTargets() throws -> [BuildTarget]
     func bazelTargetLabel(forBSPURI uri: URI) throws -> String
-    func bazelTargetSrcs(forBSPURI uri: URI) throws -> [URI]
+    func bazelTargetSrcs(forBSPURI uri: URI) throws -> SourcesItem
     func bspURIs(containingSrc src: URI) throws -> [URI]
     func platformBuildLabelInfo(forBSPURI uri: URI) throws -> BazelTargetPlatformInfo
     func targetsAqueryForArgsExtraction() throws -> ProcessedAqueryResult
@@ -114,12 +114,12 @@ final class BazelTargetStoreImpl: BazelTargetStore, @unchecked Sendable {
         return label
     }
 
-    /// Retrieves the list of registered source files for a given a BSP BuildTarget URI.
-    func bazelTargetSrcs(forBSPURI uri: URI) throws -> [URI] {
-        guard let srcs = cqueryResult?.bspURIsToSrcsMap[uri] else {
+    /// Retrieves the SourcesItem for a given a BSP BuildTarget URI.
+    func bazelTargetSrcs(forBSPURI uri: URI) throws -> SourcesItem {
+        guard let sourcesItem = cqueryResult?.bspURIsToSrcsMap[uri] else {
             throw BazelTargetStoreError.unknownBSPURI(uri)
         }
-        return srcs
+        return sourcesItem
     }
 
     /// Retrieves the list of BSP BuildTarget URIs that contain a given source file.

Sources/SourceKitBazelBSP/RequestHandlers/BuildTargets/ProcessedCqueryResult.swift

@@ -27,7 +27,7 @@ struct ProcessedCqueryResult {
     let buildTargets: [BuildTarget]
     let topLevelTargets: [(String, TopLevelRuleType)]
     let bspURIsToBazelLabelsMap: [URI: String]
-    let bspURIsToSrcsMap: [URI: [URI]]
+    let bspURIsToSrcsMap: [URI: SourcesItem]
     let srcToBspURIsMap: [URI: [URI]]
     let availableBazelLabels: Set<String>
     let topLevelLabelToRuleMap: [String: TopLevelRuleType]

Sources/SourceKitBazelBSP/RequestHandlers/TargetSourcesHandler.swift

@@ -27,11 +27,9 @@ private let logger = makeFileLevelBSPLogger()
 ///
 /// Returns the sources for the provided target based on previously gathered information.
 final class TargetSourcesHandler {
-    private let initializedConfig: InitializedServerConfig
     private let targetStore: BazelTargetStore
 
-    init(initializedConfig: InitializedServerConfig, targetStore: BazelTargetStore) {
-        self.initializedConfig = initializedConfig
+    init(targetStore: BazelTargetStore) {
         self.targetStore = targetStore
     }
 
@@ -43,89 +41,13 @@ final class TargetSourcesHandler {
         logger.info("Fetching sources for \(targets.count, privacy: .public) targets")
 
         let srcs: [SourcesItem] = try targetStore.stateLock.withLockUnchecked {
-            var srcs: [SourcesItem] = []
-            for target in targets {
-                let targetSrcs = try targetStore.bazelTargetSrcs(forBSPURI: target.uri)
-                let sources = convertToSourceItems(targetSrcs)
-                srcs.append(SourcesItem(target: target, sources: sources))
-            }
-            return srcs
+            try targets.map { try targetStore.bazelTargetSrcs(forBSPURI: $0.uri) }
         }
 
-        let count = srcs.reduce(0) { $0 + $1.sources.count }
-
         logger.info(
-            "Returning \(srcs.count, privacy: .public) source specs (\(count, privacy: .public) total source entries)"
+            "Returning \(srcs.count, privacy: .public) source specs"
         )
 
         return BuildTargetSourcesResponse(items: srcs)
     }
-
-    /// The path sourcekit-lsp has is the "real" path of the file,
-    /// but Bazel works by copying them over to the execroot.
-    /// This method calculates this fake path so that sourcekit-lsp can
-    /// map the file back to the original workspace path for features like jump to definition.
-    /// FIXME: SourceKit-LSP has a config for defining this statically, but I couldn't figure out
-    /// how to make it work. If we do, we can drop this logic and the FIXME at BuildTargetsHandler.swift.
-    func computeCopyDestinations(for src: URI) -> [DocumentURI]? {
-        guard let srcPath = src.fileURL?.path else {
-            return nil
-        }
-
-        let rootUri = initializedConfig.rootUri
-
-        guard srcPath.hasPrefix(rootUri) else {
-            return nil
-        }
-
-        let execRoot = initializedConfig.executionRoot
-
-        var relativePath = srcPath.dropFirst(rootUri.count)
-        // Not sure how much we can assume about rootUri, so adding this as an edge-case check
-        if relativePath.first == "/" {
-            relativePath = relativePath.dropFirst()
-        }
-
-        let newPath = execRoot + "/" + String(relativePath)
-        return [
-            DocumentURI(filePath: newPath, isDirectory: false)
-        ]
-    }
-
-    func convertToSourceItems(_ targetSrcs: [URI]) -> [SourceItem] {
-        var result: [SourceItem] = []
-        for src in targetSrcs {
-            let srcString = src.stringValue
-            let copyDestinations = computeCopyDestinations(for: src)
-            let kind: SourceKitSourceItemKind
-            if srcString.hasSuffix("h") {
-                kind = .header
-            } else {
-                kind = .source
-            }
-            let language: Language?
-            if srcString.hasSuffix("swift") {
-                language = .swift
-            } else if srcString.hasSuffix("m") || kind == .header {
-                language = .objective_c
-            } else {
-                language = nil
-            }
-            result.append(
-                SourceItem(
-                    uri: src,
-                    kind: .file,
-                    generated: false,  // FIXME: Need to handle this properly
-                    dataKind: .sourceKit,
-                    data: SourceKitSourceItemData(
-                        language: language,
-                        kind: kind,
-                        outputPath: nil,
-                        copyDestinations: copyDestinations
-                    ).encodeToLSPAny()
-                )
-            )
-        }
-        return result
-    }
 }

Sources/SourceKitBazelBSP/Server/SourceKitBazelBSPServer.swift

@@ -63,7 +63,7 @@ package final class SourceKitBazelBSPServer {
         registry.register(syncRequestHandler: waitUpdatesHandler.workspaceWaitForBuildSystemUpdates)
 
         // buildTarget/sources
-        let targetSourcesHandler = TargetSourcesHandler(initializedConfig: initializedConfig, targetStore: targetStore)
+        let targetSourcesHandler = TargetSourcesHandler(targetStore: targetStore)
         registry.register(syncRequestHandler: targetSourcesHandler.buildTargetSources)
 
         // textDocument/sourceKitOptions
@@ -109,8 +109,8 @@ package final class SourceKitBazelBSPServer {
         let connection = JSONRPCConnection(
             name: "sourcekit-lsp",
             protocol: MessageRegistry.bspProtocol,
-            inFD: inputHandle,
-            outFD: outputHandle
+            receiveFD: inputHandle,
+            sendFD: outputHandle
         )
         let handler = Self.makeBSPMessageHandler(baseConfig: baseConfig, connection: connection)
         self.init(

Tests/SourceKitBazelBSPTests/BazelTargetQuerierParserImplTests.swift

@@ -28,6 +28,7 @@ import Testing
 struct BazelTargetQuerierParserImplTests {
 
     private static let mockRootUri = "/path/to/project"
+    private static let mockExecutionRoot = "/tmp/execroot/_main"
     private static let mockToolchainPath = "/path/to/toolchain"
 
     @Test
@@ -55,6 +56,7 @@ struct BazelTargetQuerierParserImplTests {
             userProvidedTargets: userProvidedTargets,
             supportedTopLevelRuleTypes: supportedTopLevelRuleTypes,
             rootUri: Self.mockRootUri,
+            executionRoot: Self.mockExecutionRoot,
             toolchainPath: Self.mockToolchainPath
         )
 

Tests/SourceKitBazelBSPTests/Fakes/BazelTargetQuerierParserFake.swift

@@ -31,6 +31,7 @@ final class BazelTargetQuerierParserFake: BazelTargetQuerierParser {
         userProvidedTargets: [String],
         supportedTopLevelRuleTypes: [TopLevelRuleType],
         rootUri: String,
+        executionRoot: String,
         toolchainPath: String,
     ) throws -> ProcessedCqueryResult {
         guard let mockCqueryResult else {

Tests/SourceKitBazelBSPTests/Fakes/BazelTargetStoreFake.swift

@@ -55,7 +55,7 @@ final class BazelTargetStoreFake: BazelTargetStore {
         unimplemented()
     }
 
-    func bazelTargetSrcs(forBSPURI uri: DocumentURI) throws -> [DocumentURI] {
+    func bazelTargetSrcs(forBSPURI uri: DocumentURI) throws -> SourcesItem {
         unimplemented()
     }