Implement pull-model documentDiagnostics #746
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,7 +103,7 @@ public final class SwiftLanguageServer: ToolchainLanguageServer { | |
|
|
||
| let sourcekitd: SourceKitD | ||
|
|
||
| let capabilityRegistry: CapabilityRegistry | ||
|
|
||
| let serverOptions: SourceKitServer.Options | ||
|
|
||
|
|
@@ -122,6 +122,14 @@ public final class SwiftLanguageServer: ToolchainLanguageServer { | |
| var keys: sourcekitd_keys { return sourcekitd.keys } | ||
| var requests: sourcekitd_requests { return sourcekitd.requests } | ||
| var values: sourcekitd_values { return sourcekitd.values } | ||
|
|
||
| var enablePublishDiagnostics: Bool { | ||
| // Since LSP 3.17.0, diagnostics can be reported through pull-based requests, | ||
| // in addition to the existing push-based publish notifications. | ||
| // If the client supports pull diagnostics, we report the capability | ||
| // and we should disable the publish notifications to avoid double-reporting. | ||
| return capabilityRegistry.pullDiagnosticsRegistration(for: .swift) == nil | ||
| } | ||
|
|
||
| private var state: LanguageServerState { | ||
| didSet { | ||
|
|
@@ -144,15 +152,14 @@ public final class SwiftLanguageServer: ToolchainLanguageServer { | |
| public init?( | ||
| client: LocalConnection, | ||
| toolchain: Toolchain, | ||
| options: SourceKitServer.Options, | ||
| workspace: Workspace, | ||
| reopenDocuments: @escaping (ToolchainLanguageServer) -> Void | ||
| ) throws { | ||
| guard let sourcekitd = toolchain.sourcekitd else { return nil } | ||
| self.client = client | ||
| self.sourcekitd = try SourceKitDImpl.getOrCreate(dylibPath: sourcekitd) | ||
| self.capabilityRegistry = workspace.capabilityRegistry | ||
| self.serverOptions = options | ||
| self.documentManager = DocumentManager() | ||
| self.state = .connected | ||
|
|
@@ -242,7 +249,7 @@ public final class SwiftLanguageServer: ToolchainLanguageServer { | |
|
|
||
| /// Inform the client about changes to the syntax highlighting tokens. | ||
| private func requestTokensRefresh() { | ||
| if capabilityRegistry.clientHasSemanticTokenRefreshSupport { | ||
| _ = client.send(WorkspaceSemanticTokensRefreshRequest(), queue: queue) { result in | ||
| if let error = result.failure { | ||
| log("refreshing tokens failed: \(error)", level: .warning) | ||
|
|
@@ -285,8 +292,7 @@ public final class SwiftLanguageServer: ToolchainLanguageServer { | |
| let stageUID: sourcekitd_uid_t? = response[sourcekitd.keys.diagnostic_stage] | ||
| let stage = stageUID.flatMap { DiagnosticStage($0, sourcekitd: sourcekitd) } ?? .sema | ||
|
|
||
| let supportsCodeDescription = capabilityRegistry.clientHasDiagnosticsCodeDescriptionSupport | ||
|
|
||
| // Note: we make the notification even if there are no diagnostics to clear the current state. | ||
| var newDiags: [CachedDiagnostic] = [] | ||
|
|
@@ -326,7 +332,10 @@ public final class SwiftLanguageServer: ToolchainLanguageServer { | |
| req[keys.sourcetext] = "" | ||
|
|
||
| if let dict = try? self.sourcekitd.sendSync(req) { | ||
| if (enablePublishDiagnostics) { | ||
| publishDiagnostics(response: dict, for: snapshot, compileCommand: compileCommand) | ||
| } | ||
|
|
||
| if dict[keys.diagnostic_stage] as sourcekitd_uid_t? == sourcekitd.values.diag_stage_sema { | ||
| // Only update semantic tokens if the 0,0 replacetext request returned semantic information. | ||
| updateSemanticTokens(response: dict, for: snapshot) | ||
|
|
@@ -370,7 +379,10 @@ extension SwiftLanguageServer { | |
| range: .bool(true), | ||
| full: .bool(true)), | ||
| inlayHintProvider: .value(InlayHintOptions( | ||
| resolveProvider: false)), | ||
| diagnosticProvider: DiagnosticOptions( | ||
| interFileDependencies: true, | ||
tristanlabelle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| workspaceDiagnostics: false) | ||
tristanlabelle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| )) | ||
| } | ||
|
|
||
|
|
@@ -964,6 +976,7 @@ extension SwiftLanguageServer { | |
| } | ||
|
|
||
| public func foldingRange(_ req: Request<FoldingRangeRequest>) { | ||
| let foldingRangeCapabilities = capabilityRegistry.clientCapabilities.textDocument?.foldingRange | ||
| queue.async { | ||
| guard let snapshot = self.documentManager.latestSnapshot(req.params.textDocument.uri) else { | ||
| log("failed to find snapshot for url \(req.params.textDocument.uri)") | ||
|
|
@@ -1142,17 +1155,16 @@ extension SwiftLanguageServer { | |
| } | ||
| } | ||
|
|
||
| // If the limit is less than one, do nothing. | ||
| if let limit = foldingRangeCapabilities?.rangeLimit, limit <= 0 { | ||
| req.reply([]) | ||
| return | ||
| } | ||
|
|
||
| let rangeFinder = FoldingRangeFinder( | ||
| snapshot: snapshot, | ||
| rangeLimit: foldingRangeCapabilities?.rangeLimit, | ||
| lineFoldingOnly: foldingRangeCapabilities?.lineFoldingOnly ?? false) | ||
| rangeFinder.walk(sourceFile) | ||
| let ranges = rangeFinder.finalize() | ||
|
|
||
|
|
@@ -1167,12 +1179,12 @@ extension SwiftLanguageServer { | |
| ] | ||
| let wantedActionKinds = req.params.context.only | ||
| let providers = providersAndKinds.filter { wantedActionKinds?.contains($0.1) != false } | ||
| let codeActionCapabilities = capabilityRegistry.clientCapabilities.textDocument?.codeAction | ||
| retrieveCodeActions(req, providers: providers.map { $0.provider }) { result in | ||
| switch result { | ||
| case .success(let codeActions): | ||
| let response = CodeActionRequestResponse(codeActions: codeActions, | ||
| clientCapabilities: codeActionCapabilities) | ||
| req.reply(response) | ||
| case .failure(let error): | ||
| req.reply(.failure(error)) | ||
|
|
@@ -1326,10 +1338,75 @@ extension SwiftLanguageServer { | |
| } | ||
| } | ||
| } | ||
|
|
||
| // Must be called on self.queue | ||
| public func _documentDiagnostic( | ||
| _ uri: DocumentURI, | ||
| _ completion: @escaping (Result<[Diagnostic], ResponseError>) -> Void | ||
| ) { | ||
| dispatchPrecondition(condition: .onQueue(queue)) | ||
|
|
||
| guard let snapshot = documentManager.latestSnapshot(uri) else { | ||
| let msg = "failed to find snapshot for url \(uri)" | ||
| log(msg) | ||
| return completion(.failure(.unknown(msg))) | ||
| } | ||
|
|
||
| let keys = self.keys | ||
|
|
||
| let skreq = SKDRequestDictionary(sourcekitd: self.sourcekitd) | ||
| skreq[keys.request] = requests.diagnostics | ||
| skreq[keys.sourcefile] = snapshot.document.uri.pseudoPath | ||
|
|
||
| // FIXME: SourceKit should probably cache this for us. | ||
| if let compileCommand = self.commandsByFile[uri] { | ||
| skreq[keys.compilerargs] = compileCommand.compilerArgs | ||
| } | ||
|
|
||
| let supportsCodeDescription = capabilityRegistry.clientHasDiagnosticsCodeDescriptionSupport | ||
|
|
||
| let handle = self.sourcekitd.send(skreq, self.queue) { response in | ||
| guard let dict = response.success else { | ||
| return completion(.failure(ResponseError(response.failure!))) | ||
| } | ||
|
|
||
| var diagnostics: [Diagnostic] = [] | ||
| dict[keys.diagnostics]?.forEach { _, diag in | ||
| if let diagnostic = Diagnostic(diag, in: snapshot, useEducationalNoteAsCode: supportsCodeDescription) { | ||
| diagnostics.append(diagnostic) | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| completion(.success(diagnostics)) | ||
| } | ||
|
|
||
| // FIXME: cancellation | ||
| _ = handle | ||
|
Comment on lines
+1384
to
+1385
There was a problem hiding this comment. I think you should be able to implement cancellation similar to here There was a problem hiding this comment. Could we leave this to a PR that fixes all cancellation at once? This was copied from many places |
||
| } | ||
|
|
||
| public func documentDiagnostic( | ||
| _ uri: DocumentURI, | ||
| _ completion: @escaping (Result<[Diagnostic], ResponseError>) -> Void | ||
| ) { | ||
| self.queue.async { | ||
| self._documentDiagnostic(uri, completion) | ||
| } | ||
| } | ||
|
|
||
| public func documentDiagnostic(_ req: Request<DocumentDiagnosticsRequest>) { | ||
| let uri = req.params.textDocument.uri | ||
| documentDiagnostic(req.params.textDocument.uri) { result in | ||
| switch result { | ||
| case .success(let diagnostics): | ||
| req.reply(.full(.init(items: diagnostics))) | ||
|
|
||
| case .failure(let error): | ||
| let message = "document diagnostic failed \(uri): \(error)" | ||
| log(message, level: .warning) | ||
| return req.reply(.failure(.unknown(message))) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public func executeCommand(_ req: Request<ExecuteCommandRequest>) { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it documented somewhere that LSP servers shouldn’t publish diagnostics if the client supports
pullDiagnostics? Or, if not, do you know if other LSP servers are also doing this as well or if VSCode is relying on it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the two can be mix-and-matched if they use different provider names, but it wouldn't be desirable here because we would be reporting the same diagnostics twice, and the push-model ones would still have the same staleness issue being addressed by pull-model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m just worried that some LSP client says that it supports pull diagnostics but for some reason still expects that diagnostics get published. That’s why I’d like to know what other LSP servers do…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest a solution to this here: https://github.com/apple/sourcekit-lsp/pull/746/files#r1206839125