Skip to content

Remove sourcekitd test hooks #1435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Sources/Diagnose/RunSourcekitdRequestCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ public struct RunSourceKitdRequestCommand: AsyncParsableCommand {
throw ExitCode(1)
}
let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
dylibPath: try! AbsolutePath(validating: sourcekitdPath),
testHooks: SourceKitDTestHooks()
dylibPath: try! AbsolutePath(validating: sourcekitdPath)
)

if let lineColumn = position?.split(separator: ":", maxSplits: 2).map(Int.init),
Expand Down
25 changes: 5 additions & 20 deletions Sources/SourceKitD/DynamicallyLoadedSourceKitD.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,17 @@ extension sourcekitd_api_keys: @unchecked Sendable {}
extension sourcekitd_api_requests: @unchecked Sendable {}
extension sourcekitd_api_values: @unchecked Sendable {}

public struct SourceKitDTestHooks: Sendable {
public var sourcekitdRequestDidStart: (@Sendable (SKDRequestDictionary) -> Void)?

public init(sourcekitdRequestDidStart: (@Sendable (SKDRequestDictionary) -> Void)? = nil) {
self.sourcekitdRequestDidStart = sourcekitdRequestDidStart
}
}

/// Wrapper for sourcekitd, taking care of initialization, shutdown, and notification handler
/// multiplexing.
///
/// Users of this class should not call the api functions `initialize`, `shutdown`, or
/// `set_notification_handler`, which are global state managed internally by this class.
public actor DynamicallyLoadedSourceKitD: SourceKitD {
/// The path to the sourcekitd dylib.
private let path: AbsolutePath
public let path: AbsolutePath

/// The handle to the dylib.
private let dylib: DLHandle

public let testHooks: SourceKitDTestHooks
let dylib: DLHandle

/// The sourcekitd API functions.
public let api: sourcekitd_api_functions_t
Expand All @@ -65,23 +55,18 @@ public actor DynamicallyLoadedSourceKitD: SourceKitD {
/// List of notification handlers that will be called for each notification.
private var notificationHandlers: [WeakSKDNotificationHandler] = []

/// If there is already a `sourcekitd` instance from the given return it, otherwise create a new one.
///
/// `testHooks` are only considered when an instance is being created. If a sourcekitd instance at the given path
/// already exists, its test hooks will be used.
public static func getOrCreate(dylibPath: AbsolutePath, testHooks: SourceKitDTestHooks) async throws -> SourceKitD {
public static func getOrCreate(dylibPath: AbsolutePath) async throws -> SourceKitD {
try await SourceKitDRegistry.shared
.getOrAdd(dylibPath, create: { try DynamicallyLoadedSourceKitD(dylib: dylibPath, testHooks: testHooks) })
.getOrAdd(dylibPath, create: { try DynamicallyLoadedSourceKitD(dylib: dylibPath) })
}

init(dylib path: AbsolutePath, testHooks: SourceKitDTestHooks) throws {
init(dylib path: AbsolutePath) throws {
self.path = path
#if os(Windows)
self.dylib = try dlopen(path.pathString, mode: [])
#else
self.dylib = try dlopen(path.pathString, mode: [.lazy, .local, .first])
#endif
self.testHooks = testHooks
self.api = try sourcekitd_api_functions_t(self.dylib)
self.keys = sourcekitd_api_keys(api: self.api)
self.requests = sourcekitd_api_requests(api: self.api)
Expand Down
4 changes: 0 additions & 4 deletions Sources/SourceKitD/SourceKitD.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ extension sourcekitd_api_request_handle_t: @unchecked Sendable {}
/// *Implementors* are expected to handle initialization and shutdown, e.g. during `init` and
/// `deinit` or by wrapping an existing sourcekitd session that outlives this object.
public protocol SourceKitD: AnyObject, Sendable {
var testHooks: SourceKitDTestHooks { get }

/// The sourcekitd API functions.
var api: sourcekitd_api_functions_t { get }

Expand Down Expand Up @@ -101,8 +99,6 @@ extension SourceKitD {
public func send(_ request: SKDRequestDictionary, fileContents: String?) async throws -> SKDResponseDictionary {
log(request: request)

testHooks.sourcekitdRequestDidStart?(request)

let sourcekitdResponse: SKDResponse = try await withCancellableCheckedThrowingContinuation { continuation in
var handle: sourcekitd_api_request_handle_t? = nil
api.send_request(request.dict, &handle) { response in
Expand Down
5 changes: 0 additions & 5 deletions Sources/SourceKitLSP/SourceKitLSPServer+Options.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import LanguageServerProtocol
import SKCore
import SKSupport
import SemanticIndex
import SourceKitD

import struct TSCBasic.AbsolutePath
import struct TSCBasic.RelativePath
Expand Down Expand Up @@ -52,8 +51,6 @@ extension SourceKitLSPServer {
/// Experimental features that are enabled.
public var experimentalFeatures: Set<ExperimentalFeature>

public var sourcekitdTestHooks: SourceKitDTestHooks

public var indexTestHooks: IndexTestHooks

public init(
Expand All @@ -65,7 +62,6 @@ extension SourceKitLSPServer {
generatedInterfacesPath: AbsolutePath = defaultDirectoryForGeneratedInterfaces,
swiftPublishDiagnosticsDebounceDuration: TimeInterval = 2, /* 2s */
experimentalFeatures: Set<ExperimentalFeature> = [],
sourcekitdTestHooks: SourceKitDTestHooks = SourceKitDTestHooks(),
indexTestHooks: IndexTestHooks = IndexTestHooks()
) {
self.buildSetup = buildSetup
Expand All @@ -76,7 +72,6 @@ extension SourceKitLSPServer {
self.generatedInterfacesPath = generatedInterfacesPath
self.swiftPublishDiagnosticsDebounceDuration = swiftPublishDiagnosticsDebounceDuration
self.experimentalFeatures = experimentalFeatures
self.sourcekitdTestHooks = sourcekitdTestHooks
self.indexTestHooks = indexTestHooks
}
}
Expand Down
5 changes: 1 addition & 4 deletions Sources/SourceKitLSP/Swift/SwiftLanguageService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,7 @@ public actor SwiftLanguageService: LanguageService, Sendable {
guard let sourcekitd = toolchain.sourcekitd else { return nil }
self.sourceKitLSPServer = sourceKitLSPServer
self.swiftFormat = toolchain.swiftFormat
self.sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
dylibPath: sourcekitd,
testHooks: options.sourcekitdTestHooks
)
self.sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(dylibPath: sourcekitd)
self.capabilityRegistry = workspace.capabilityRegistry
self.semanticIndexManager = workspace.semanticIndexManager
self.serverOptions = options
Expand Down
3 changes: 1 addition & 2 deletions Tests/DiagnoseTests/DiagnoseTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,7 @@ private class InProcessSourceKitRequestExecutor: SourceKitRequestExecutor {
logger.info("Sending request: \(requestString)")

let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
dylibPath: try! AbsolutePath(validating: sourcekitd.path),
testHooks: SourceKitDTestHooks()
dylibPath: try! AbsolutePath(validating: sourcekitd.path)
)
let response = try await sourcekitd.run(requestYaml: requestString)

Expand Down
1 change: 0 additions & 1 deletion Tests/SourceKitDTests/SourceKitDRegistryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ private nonisolated(unsafe) var nextToken = AtomicUInt32(initialValue: 0)

final class FakeSourceKitD: SourceKitD {
let token: UInt32
var testHooks: SourceKitDTestHooks { SourceKitDTestHooks() }
var api: sourcekitd_api_functions_t { fatalError() }
var keys: sourcekitd_api_keys { fatalError() }
var requests: sourcekitd_api_requests { fatalError() }
Expand Down
5 changes: 1 addition & 4 deletions Tests/SourceKitDTests/SourceKitDTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ import class TSCBasic.Process
final class SourceKitDTests: XCTestCase {
func testMultipleNotificationHandlers() async throws {
let sourcekitdPath = await ToolchainRegistry.forTesting.default!.sourcekitd!
let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(
dylibPath: sourcekitdPath,
testHooks: SourceKitDTestHooks()
)
let sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate(dylibPath: sourcekitdPath)
let keys = sourcekitd.keys
let path = DocumentURI(for: .swift).pseudoPath

Expand Down
14 changes: 5 additions & 9 deletions Tests/SourceKitLSPTests/PullDiagnosticsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -310,15 +310,10 @@ final class PullDiagnosticsTests: XCTestCase {
}

func testDontReturnEmptyDiagnosticsIfDiagnosticRequestIsCancelled() async throws {
let diagnosticSourcekitdRequestDidStart = self.expectation(description: "diagnostic sourcekitd request did start")
let diagnosticRequestCancelled = self.expectation(description: "diagnostic request cancelled")
var serverOptions = SourceKitLSPServer.Options.testDefault
serverOptions.sourcekitdTestHooks.sourcekitdRequestDidStart = { request in
guard request.description.contains("source.request.diagnostics") else {
return
}
diagnosticSourcekitdRequestDidStart.fulfill()
self.wait(for: [diagnosticRequestCancelled], timeout: defaultTimeout)
serverOptions.indexTestHooks.preparationTaskDidStart = { _ in
await self.fulfillment(of: [diagnosticRequestCancelled], timeout: defaultTimeout)
// Poll until the `CancelRequestNotification` has been propagated to the request handling.
for _ in 0..<Int(defaultTimeout * 100) {
if Task.isCancelled {
Expand All @@ -331,7 +326,9 @@ final class PullDiagnosticsTests: XCTestCase {
files: [
"Lib.swift": "let x: String = 1"
],
serverOptions: serverOptions
serverOptions: serverOptions,
enableBackgroundIndexing: true,
pollIndex: false
)
let (uri, _) = try project.openDocument("Lib.swift")

Expand All @@ -342,7 +339,6 @@ final class PullDiagnosticsTests: XCTestCase {
XCTAssertEqual(result, .failure(ResponseError.cancelled))
diagnosticResponseReceived.fulfill()
}
try await fulfillmentOfOrThrow([diagnosticSourcekitdRequestDidStart])
project.testClient.send(CancelRequestNotification(id: requestID))
diagnosticRequestCancelled.fulfill()
try await fulfillmentOfOrThrow([diagnosticResponseReceived])
Expand Down