Skip to content
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

Remove document manager in SwiftLanguageService #1466

Merged
merged 1 commit into from
Jul 4, 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
10 changes: 8 additions & 2 deletions Sources/SourceKitLSP/Clang/ClangLanguageService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import LanguageServerProtocolJSONRPC
import SKCore
import SKSupport
import SwiftExtensions
import SwiftSyntax

import struct TSCBasic.AbsolutePath

Expand Down Expand Up @@ -443,7 +444,7 @@ extension ClangLanguageService {

// MARK: - Text synchronization

public func openDocument(_ notification: DidOpenTextDocumentNotification) async {
public func openDocument(_ notification: DidOpenTextDocumentNotification, snapshot: DocumentSnapshot) async {
openDocuments[notification.textDocument.uri] = notification.textDocument.language
// Send clangd the build settings for the new file. We need to do this before
// sending the open notification, so that the initial diagnostics already
Expand All @@ -459,7 +460,12 @@ extension ClangLanguageService {

func reopenDocument(_ notification: ReopenTextDocumentNotification) {}

public func changeDocument(_ notification: DidChangeTextDocumentNotification) {
public func changeDocument(
_ notification: DidChangeTextDocumentNotification,
preEditSnapshot: DocumentSnapshot,
postEditSnapshot: DocumentSnapshot,
edits: [SourceEdit]
) {
clangd.send(notification)
}

Expand Down
36 changes: 0 additions & 36 deletions Sources/SourceKitLSP/DocumentManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -204,42 +204,6 @@ public final class DocumentManager: InMemoryDocumentManager, Sendable {
}
}

extension DocumentManager {

// MARK: - LSP notification handling

/// Convenience wrapper for `open(_:language:version:text:)` that logs on failure.
@discardableResult
func open(_ notification: DidOpenTextDocumentNotification) -> DocumentSnapshot? {
let doc = notification.textDocument
return orLog("failed to open document", level: .error) {
try open(doc.uri, language: doc.language, version: doc.version, text: doc.text)
}
}

/// Convenience wrapper for `close(_:)` that logs on failure.
func close(_ notification: DidCloseTextDocumentNotification) {
orLog("failed to close document", level: .error) {
try close(notification.textDocument.uri)
}
}

/// Convenience wrapper for `edit(_:newVersion:edits:updateDocumentTokens:)`
/// that logs on failure.
@discardableResult
func edit(
_ notification: DidChangeTextDocumentNotification
) -> (preEditSnapshot: DocumentSnapshot, postEditSnapshot: DocumentSnapshot, edits: [SourceEdit])? {
return orLog("failed to edit document", level: .error) {
return try edit(
notification.textDocument.uri,
newVersion: notification.textDocument.version,
edits: notification.contentChanges
)
}
}
}

fileprivate extension SourceEdit {
/// Constructs a `SourceEdit` from the given `TextDocumentContentChangeEvent`.
///
Expand Down
14 changes: 10 additions & 4 deletions Sources/SourceKitLSP/LanguageService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import Foundation
import LanguageServerProtocol
import SKCore
import SwiftSyntax

/// The state of a `ToolchainLanguageServer`
public enum LanguageServerState {
Expand Down Expand Up @@ -124,9 +125,9 @@ public protocol LanguageService: AnyObject, Sendable {
// MARK: - Text synchronization

/// Sent to open up a document on the Language Server.
/// This may be called before or after a corresponding
/// `documentUpdatedBuildSettings` call for the same document.
func openDocument(_ notification: DidOpenTextDocumentNotification) async
///
/// This may be called before or after a corresponding `documentUpdatedBuildSettings` call for the same document.
func openDocument(_ notification: DidOpenTextDocumentNotification, snapshot: DocumentSnapshot) async

/// Sent to close a document on the Language Server.
func closeDocument(_ notification: DidCloseTextDocumentNotification) async
Expand All @@ -138,7 +139,12 @@ public protocol LanguageService: AnyObject, Sendable {
/// Only intended for `SwiftLanguageService`.
func reopenDocument(_ notification: ReopenTextDocumentNotification) async

func changeDocument(_ notification: DidChangeTextDocumentNotification) async
func changeDocument(
_ notification: DidChangeTextDocumentNotification,
preEditSnapshot: DocumentSnapshot,
postEditSnapshot: DocumentSnapshot,
edits: [SourceEdit]
) async
func willSaveDocument(_ notification: WillSaveTextDocumentNotification) async
func didSaveDocument(_ notification: DidSaveTextDocumentNotification) async

Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/Rename.swift
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ extension SwiftLanguageService {
in uri: DocumentURI,
name: CompoundDeclName
) async throws -> String {
guard let snapshot = documentManager.latestSnapshotOrDisk(uri, language: .swift) else {
guard let snapshot = try? documentManager.latestSnapshotOrDisk(uri, language: .swift) else {
throw ResponseError.unknown("Failed to get contents of \(uri.forLogging) to translate Swift name to clang name")
}

Expand Down
38 changes: 33 additions & 5 deletions Sources/SourceKitLSP/SourceKitLSPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,18 @@ extension SourceKitLSPServer {
private func openDocument(_ notification: DidOpenTextDocumentNotification, workspace: Workspace) async {
// Immediately open the document even if the build system isn't ready. This is important since
// we check that the document is open when we receive messages from the build system.
documentManager.open(notification)
let snapshot = orLog("Opening document") {
try documentManager.open(
notification.textDocument.uri,
language: notification.textDocument.language,
version: notification.textDocument.version,
text: notification.textDocument.text
)
}
guard let snapshot else {
// Already logged failure
return
}

let textDocument = notification.textDocument
let uri = textDocument.uri
Expand All @@ -1286,7 +1297,7 @@ extension SourceKitLSPServer {
await workspace.buildSystemManager.registerForChangeNotifications(for: uri, language: language)

// If the document is ready, we can immediately send the notification.
await service.openDocument(notification)
await service.openDocument(notification, snapshot: snapshot)
}

func closeDocument(_ notification: DidCloseTextDocumentNotification) async {
Expand Down Expand Up @@ -1314,7 +1325,9 @@ extension SourceKitLSPServer {
func closeDocument(_ notification: DidCloseTextDocumentNotification, workspace: Workspace) async {
// Immediately close the document. We need to be sure to clear our pending work queue in case
// the build system still isn't ready.
documentManager.close(notification)
orLog("failed to close document", level: .error) {
try documentManager.close(notification.textDocument.uri)
}

let uri = notification.textDocument.uri

Expand All @@ -1335,8 +1348,23 @@ extension SourceKitLSPServer {
await workspace.semanticIndexManager?.schedulePreparationForEditorFunctionality(of: uri)

// If the document is ready, we can handle the change right now.
documentManager.edit(notification)
await workspace.documentService.value[uri]?.changeDocument(notification)
let editResult = orLog("Editing document") {
try documentManager.edit(
notification.textDocument.uri,
newVersion: notification.textDocument.version,
edits: notification.contentChanges
)
}
guard let (preEditSnapshot, postEditSnapshot, edits) = editResult else {
// Already logged failure
return
}
await workspace.documentService.value[uri]?.changeDocument(
notification,
preEditSnapshot: preEditSnapshot,
postEditSnapshot: postEditSnapshot,
edits: edits
)
}

func willSaveDocument(
Expand Down
41 changes: 18 additions & 23 deletions Sources/SourceKitLSP/Swift/SwiftLanguageService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public struct SwiftCompileCommand: Sendable, Equatable {
}

public actor SwiftLanguageService: LanguageService, Sendable {
/// The ``SourceKitLSPServer`` instance that created this `ClangLanguageService`.
/// The ``SourceKitLSPServer`` instance that created this `SwiftLanguageService`.
weak var sourceKitLSPServer: SourceKitLSPServer?

let sourcekitd: SourceKitD
Expand Down Expand Up @@ -121,9 +121,6 @@ public actor SwiftLanguageService: LanguageService, Sendable {
options.generatedFilesAbsolutePath.asURL.appendingPathComponent("GeneratedMacroExpansions")
}

// FIXME: ideally we wouldn't need separate management from a parent server in the same process.
var documentManager: DocumentManager

/// For each edited document, the last task that was triggered to send a `PublishDiagnosticsNotification`.
///
/// This is used to cancel previous publish diagnostics tasks if an edit is made to a document.
Expand All @@ -148,6 +145,15 @@ public actor SwiftLanguageService: LanguageService, Sendable {

private var diagnosticReportManager: DiagnosticReportManager!

var documentManager: DocumentManager {
get throws {
guard let sourceKitLSPServer = self.sourceKitLSPServer else {
throw ResponseError.unknown("Connection to the editor closed")
}
return sourceKitLSPServer.documentManager
}
}

/// Calling `scheduleCall` on `refreshDiagnosticsDebouncer` schedules a `DiagnosticsRefreshRequest` to be sent to
/// to the client.
///
Expand Down Expand Up @@ -183,7 +189,6 @@ public actor SwiftLanguageService: LanguageService, Sendable {
self.capabilityRegistry = workspace.capabilityRegistry
self.semanticIndexManager = workspace.semanticIndexManager
self.testHooks = testHooks
self.documentManager = DocumentManager()
self.state = .connected
self.diagnosticReportManager = nil // Needed to work around rdar://116221716
self.options = options
Expand All @@ -203,7 +208,7 @@ public actor SwiftLanguageService: LanguageService, Sendable {
sourcekitd: self.sourcekitd,
options: options,
syntaxTreeManager: syntaxTreeManager,
documentManager: documentManager,
documentManager: try documentManager,
clientHasDiagnosticsCodeDescriptionSupport: await self.clientHasDiagnosticsCodeDescriptionSupport
)

Expand Down Expand Up @@ -439,15 +444,10 @@ extension SwiftLanguageService {
])
}

public func openDocument(_ notification: DidOpenTextDocumentNotification) async {
public func openDocument(_ notification: DidOpenTextDocumentNotification, snapshot: DocumentSnapshot) async {
cancelInFlightPublishDiagnosticsTask(for: notification.textDocument.uri)
await diagnosticReportManager.removeItemsFromCache(with: notification.textDocument.uri)

guard let snapshot = self.documentManager.open(notification) else {
// Already logged failure.
return
}

let buildSettings = await self.buildSettings(for: snapshot.uri)

let req = openDocumentSourcekitdRequest(snapshot: snapshot, compileCommand: buildSettings)
Expand All @@ -460,8 +460,6 @@ extension SwiftLanguageService {
inFlightPublishDiagnosticsTasks[notification.textDocument.uri] = nil
await diagnosticReportManager.removeItemsFromCache(with: notification.textDocument.uri)

self.documentManager.close(notification)

let req = closeDocumentSourcekitdRequest(uri: notification.textDocument.uri)
_ = try? await self.sendSourcekitdRequest(req, fileContents: nil)
}
Expand Down Expand Up @@ -542,7 +540,12 @@ extension SwiftLanguageService {
}
}

public func changeDocument(_ notification: DidChangeTextDocumentNotification) async {
public func changeDocument(
_ notification: DidChangeTextDocumentNotification,
preEditSnapshot: DocumentSnapshot,
postEditSnapshot: DocumentSnapshot,
edits: [SourceEdit]
) async {
cancelInFlightPublishDiagnosticsTask(for: notification.textDocument.uri)

let keys = self.keys
Expand All @@ -552,10 +555,6 @@ extension SwiftLanguageService {
let replacement: String
}

guard let (preEditSnapshot, postEditSnapshot, edits) = self.documentManager.edit(notification) else {
return
}

for edit in edits {
let req = sourcekitd.dictionary([
keys.request: self.requests.editorReplaceText,
Expand Down Expand Up @@ -1006,10 +1005,6 @@ extension SwiftLanguageService: SKDNotificationHandler {

if notification.error == .connectionInterrupted {
await self.setState(.connectionInterrupted)

// We don't have any open documents anymore after sourcekitd crashed.
// Reset the document manager to reflect that.
self.documentManager = DocumentManager()
}
}
}
Expand Down