Skip to content

Commit a4ac6e0

Browse files
committed
Allow clients to pass experimental capabilities they want to enable as a dictionary
The motivation for this is that I noticed that the `workspace/peekDocuments` request only allows the specification of a document to peek but not the location to peek within it. The fix is to change the `location` parameter from `DocumentUri` to `Location`, but this is a protocol-breaking change, so the client needs to communicate that it can support locations in the `workspace/peekDocuments` request. The client currently communicates that it supports peeking documents by passing `workspace/peekDocuments: true` in the experimental client capabilities. We could just add another option to the experimental client capabilities like `workspace/peekDocuments.supportsLocations` but this seems a little hacky. Instead, what I propose is that we 1. Allow enabling experimental capabilities as `"<capability name>": { "supported": true }` 2. Switch the VS Code Swift extension to enable client capabilities using a dictionary if it discovers that it uses SourceKit-LSP ≥6.3 (older SourceKit-LSP don’t recogize a dictionary as enabling a capability) 3. Expanding the options to `"workspace/peekDocuments": { "supported" true, "supportsLocation": true }` to communicate that location-based peeking is supported This pattern will also support experimental capability changes like this in the future. Use cases that might lack because of this are: 1. Using an old Swift 6.3 toolchain that doesn’t support dictionary-based capabilities with the VS Code Swift extension. In that case macro expansion will work in the fallback mode that we have for other editors and active editor tracking (which cancels target preparation when a file is switched) is disabled. I think this is acceptable. 2. If there are other editors that support these experimental capabilities, they will continue to work just fine by passing the boolen value to enable the option. If they want to support one of the options (eg. location-based peeking), they will need to switch to dictinary-based enabling and thus check the SourceKit-LSP version prior to startup. This might be mildly annoying on their side but since the number of client that support these capabilities should be very small (I am not aware of any) and they need to explicitly opt-in to the new behavior, I think this is also acceptable.
1 parent d2e5bd7 commit a4ac6e0

File tree

7 files changed

+40
-21
lines changed

7 files changed

+40
-21
lines changed

Contributor Documentation/LSP Extensions.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@
22

33
SourceKit-LSP extends the LSP protocol in the following ways.
44

5+
To enable some of these extensions, the client needs to communicate that it can support them. To do so, it should pass a dictionary for the `capabilities.experimental` field in the `initialize` request. For each capability to enable, it should pass an entry as follows.
6+
7+
```json
8+
"<capabilityName>": {
9+
"supported": true
10+
}
11+
```
12+
513
## `PublishDiagnosticsClientCapabilities`
614

715
Added field (this is an extension from clangd that SourceKit-LSP re-exposes):

Sources/SourceKitLSP/CapabilityRegistry.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,18 @@ package final actor CapabilityRegistry {
103103
guard case .dictionary(let experimentalCapabilities) = clientCapabilities.experimental else {
104104
return false
105105
}
106-
return experimentalCapabilities[name] == .bool(true)
106+
// Before Swift 6.3 we expected experimental client capabilities to be passed as `"capabilityName": true`.
107+
// This proved to be insufficient for experimental capabilities that evolved over time. Since 6.3 we encourage
108+
// clients to pass experimental capabilities as `"capabilityName": { "supported": true }`, which allows the addition
109+
// of more configuration parameters to the capability.
110+
switch experimentalCapabilities[name] {
111+
case .bool(true):
112+
return true
113+
case .dictionary(let dict):
114+
return dict["supported"] == .bool(true)
115+
default:
116+
return false
117+
}
107118
}
108119

109120
// MARK: Initializer

Sources/SourceKitLSP/SourceKitLSPServer.swift

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -938,12 +938,14 @@ extension SourceKitLSPServer {
938938
guard let experimentalCapability = initializationOptions[capabilityName] else {
939939
continue
940940
}
941-
if case .dictionary(var experimentalCapabilities) = clientCapabilities.experimental {
942-
experimentalCapabilities[capabilityName] = experimentalCapability
943-
clientCapabilities.experimental = .dictionary(experimentalCapabilities)
944-
} else {
945-
clientCapabilities.experimental = .dictionary([capabilityName: experimentalCapability])
946-
}
941+
var experimentalCapabilities: [String: LSPAny] =
942+
if case .dictionary(let experimentalCapabilities) = clientCapabilities.experimental {
943+
experimentalCapabilities
944+
} else {
945+
[:]
946+
}
947+
experimentalCapabilities[capabilityName] = experimentalCapability
948+
clientCapabilities.experimental = .dictionary(experimentalCapabilities)
947949
}
948950

949951
// The client announces what CodeLenses it supports, and the LSP will only return

Sources/SourceKitLSP/Swift/OpenInterface.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ extension SwiftLanguageService {
3636
nil
3737
}
3838

39-
if case .dictionary(let experimentalCapabilities) = self.capabilityRegistry.clientCapabilities.experimental,
40-
case .bool(true) = experimentalCapabilities["workspace/getReferenceDocument"]
41-
{
39+
if self.capabilityRegistry.clientHasExperimentalCapability(GetReferenceDocumentRequest.method) {
4240
return GeneratedInterfaceDetails(uri: try urlData.uri, position: position)
4341
}
4442
let interfaceFilePath = self.generatedInterfacesPath.appendingPathComponent(urlData.displayName)

Tests/SourceKitLSPTests/ExpandMacroTests.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ final class ExpandMacroTests: XCTestCase {
7979
files: files,
8080
manifest: SwiftPMTestProject.macroPackageManifest,
8181
capabilities: ClientCapabilities(experimental: [
82-
"workspace/peekDocuments": .bool(peekDocuments),
83-
"workspace/getReferenceDocument": .bool(getReferenceDocument),
82+
PeekDocumentsRequest.method: .dictionary(["supported": .bool(peekDocuments)]),
83+
GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(getReferenceDocument)]),
8484
]),
8585
options: SourceKitLSPOptions.testDefault(),
8686
enableBackgroundIndexing: true
@@ -268,8 +268,8 @@ final class ExpandMacroTests: XCTestCase {
268268
files: files,
269269
manifest: SwiftPMTestProject.macroPackageManifest,
270270
capabilities: ClientCapabilities(experimental: [
271-
PeekDocumentsRequest.method: .bool(peekDocuments),
272-
GetReferenceDocumentRequest.method: .bool(getReferenceDocument),
271+
PeekDocumentsRequest.method: .dictionary(["supported": .bool(peekDocuments)]),
272+
GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(getReferenceDocument)]),
273273
]),
274274
options: SourceKitLSPOptions.testDefault(),
275275
enableBackgroundIndexing: true
@@ -464,8 +464,8 @@ final class ExpandMacroTests: XCTestCase {
464464
files: files,
465465
manifest: SwiftPMTestProject.macroPackageManifest,
466466
capabilities: ClientCapabilities(experimental: [
467-
"workspace/peekDocuments": .bool(true),
468-
"workspace/getReferenceDocument": .bool(true),
467+
PeekDocumentsRequest.method: .dictionary(["supported": .bool(true)]),
468+
GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(true)]),
469469
]),
470470
options: SourceKitLSPOptions.testDefault(),
471471
enableBackgroundIndexing: true

Tests/SourceKitLSPTests/SwiftInterfaceTests.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ final class SwiftInterfaceTests: XCTestCase {
4545
func testSystemModuleInterfaceReferenceDocument() async throws {
4646
let testClient = try await TestSourceKitLSPClient(
4747
capabilities: ClientCapabilities(experimental: [
48-
"workspace/getReferenceDocument": .bool(true)
48+
GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(true)])
4949
])
5050
)
5151
let uri = DocumentURI(for: .swift)
@@ -117,7 +117,7 @@ final class SwiftInterfaceTests: XCTestCase {
117117
}
118118
""",
119119
capabilities: ClientCapabilities(experimental: [
120-
"workspace/getReferenceDocument": .bool(true)
120+
GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(true)])
121121
]),
122122
indexSystemModules: true
123123
)
@@ -210,7 +210,7 @@ final class SwiftInterfaceTests: XCTestCase {
210210
)
211211
""",
212212
capabilities: ClientCapabilities(experimental: [
213-
"workspace/getReferenceDocument": .bool(true)
213+
GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(true)])
214214
]),
215215
enableBackgroundIndexing: true
216216
)
@@ -292,7 +292,7 @@ final class SwiftInterfaceTests: XCTestCase {
292292
func testNoDiagnosticsInGeneratedInterface() async throws {
293293
let testClient = try await TestSourceKitLSPClient(
294294
capabilities: ClientCapabilities(experimental: [
295-
"workspace/getReferenceDocument": .bool(true)
295+
GetReferenceDocumentRequest.method: .dictionary(["supported": .bool(true)])
296296
])
297297
)
298298
let uri = DocumentURI(for: .swift)

Tests/SourceKitLSPTests/WorkspaceTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1125,7 +1125,7 @@ final class WorkspaceTests: XCTestCase {
11251125
)
11261126
""",
11271127
capabilities: ClientCapabilities(experimental: [
1128-
DidChangeActiveDocumentNotification.method: .bool(true)
1128+
DidChangeActiveDocumentNotification.method: .dictionary(["supported": .bool(true)])
11291129
]),
11301130
hooks: Hooks(
11311131
indexHooks: IndexHooks(preparationTaskDidStart: { task in

0 commit comments

Comments
 (0)