Skip to content

Commit

Permalink
fix: SwiftLanguageService instead of ClangLanguageService
Browse files Browse the repository at this point in the history
fix!: remove `documentManager` property

fix!: remove `documentManager` in constructor

fix: added documentManager as a computed Property

fix: remove old documentmanager resetting step

fix!: get throws instead of crashing server

fix: throw ResponseError instead of NSError

fix: adding try & orLog where necesary

fix: refined orLog usage

style: ran swift-format

fix: document being opened twice

fix(DocumentManager): remove unwanted comments
  • Loading branch information
louisunlimited committed Jul 3, 2024
1 parent 1efb267 commit 78dfaf1
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 71 deletions.
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
15 changes: 11 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 @@ -113,9 +114,10 @@ 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 @@ -127,7 +129,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 @@ -1256,7 +1256,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 @@ -1270,7 +1281,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 @@ -1298,7 +1309,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 @@ -1319,8 +1332,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 @@ -122,9 +122,6 @@ public actor SwiftLanguageService: LanguageService, Sendable {
generatedFilesPath.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 @@ -149,6 +146,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 @@ -184,7 +190,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

Expand All @@ -204,7 +209,7 @@ public actor SwiftLanguageService: LanguageService, Sendable {
self.diagnosticReportManager = DiagnosticReportManager(
sourcekitd: self.sourcekitd,
syntaxTreeManager: syntaxTreeManager,
documentManager: documentManager,
documentManager: try documentManager,
clientHasDiagnosticsCodeDescriptionSupport: await self.clientHasDiagnosticsCodeDescriptionSupport
)

Expand Down Expand Up @@ -425,15 +430,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 @@ -446,8 +446,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.sourcekitd.send(req, fileContents: nil)
}
Expand Down Expand Up @@ -528,7 +526,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 @@ -538,10 +541,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 @@ -990,10 +989,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

0 comments on commit 78dfaf1

Please sign in to comment.