Remove document manager in `SwiftLanguageService` #1466

louisunlimited · 2024-06-08T21:19:14Z

Fixes: Remove document manager in SwiftLanguageService #1423

louisunlimited · 2024-06-13T23:03:19Z

@ahoppen I found a lot of places where we reference documentManager directly, either it be within SwiftLanguageService or it's extensions elsewhere, so I ended up adding it as a computed property.

/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Sources/SourceKitLSP/DocumentManager.swift:98:27: warning: stored property 'documents' of 'Sendable'-conforming class 'DocumentManager' is mutable
  nonisolated(unsafe) var documents: [DocumentURI: Document] = [:]
                          ^
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Sources/SourceKitLSP/SourceKitLSPServer.swift:44:35: warning: stored property 'replied' of 'Sendable'-conforming generic class 'RequestAndReply' is mutable
  private nonisolated(unsafe) var replied: AtomicBool = AtomicBool(initialValue: false)
                                  ^
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Sources/SourceKitLSP/SourceKitLSPServer.swift:218:31: warning: cannot access property 'indexProgressManager' here in non-isolated initializer; this is an error in Swift 6
    self.indexProgressManager = IndexProgressManager(sourceKitLSPServer: self)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Sources/SourceKitLSP/SourceKitLSPServer.swift:218:74: note: after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init
    self.indexProgressManager = IndexProgressManager(sourceKitLSPServer: self)
                                                                         ^~~~
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift:508:9: warning: no 'async' operations occur within 'await' expression
        await sourceKitLSPServer.sendNotificationToClient(
        ^
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift:740:8: warning: converting non-sendable function value to '@Sendable (CodeActionRequest) async throws -> [CodeAction]' may introduce data races
      (retrieveSyntaxCodeActions, nil),
       ^
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift:741:8: warning: converting non-sendable function value to '@Sendable (CodeActionRequest) async throws -> [CodeAction]' may introduce data races
      (retrieveRefactorCodeActions, .refactor),
       ^
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift:742:8: warning: converting non-sendable function value to '@Sendable (CodeActionRequest) async throws -> [CodeAction]' may introduce data races
      (retrieveQuickFixCodeActions, .quickFix),
       ^
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Tests/SourceKitLSPTests/ExpectedIndexTaskTracker.swift:85:20: warning: cannot access property 'testHooks' here in non-isolated initializer; this is an error in Swift 6
    self.testHooks = IndexTestHooks(
    ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
/Users/louissssss/Desktop/CS_Projects/sourcekit-lsp/Tests/SourceKitLSPTests/ExpectedIndexTaskTracker.swift:86:40: note: after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init
      preparationTaskDidStart: { [weak self] in

Doing so introduces the above new warnings, I'm currently trying to figure out how to resolve these. Also some tests are failing, trying to address those as well.

ahoppen · 2024-06-17T16:41:57Z

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

+      guard let sourceKitLSPServer = self.sourceKitLSPServer else {
+        fatalError("Cannot get document manager because sourceKitLSPServer was deallocated")
+      }


We should not crash the sourcekit-lsp server process if sourceKitLSPServer got deallocated. It’s a weak variable and it becoming nil is a valid thing that can happen. Instead, we should throw an error if it becomes nil, which will return an error response for the requests. You should be able to make this getter throwing by annotating it as get throws.

If it throws we'd still have to wrap a try around documentManger everytime we access that right? arguably the same workload with wrapping a guard let when marking it as optional like we discussed earlier?

Yes, exactly. I think adding try is a lot more lightweight than guard let …. else { throw ResponseError.unknown("some message") }.

Many methods in SwiftLanguageService throw already anyway (eg. all the calls to documentManager.latestSnapshot) and for those methods, it’s just a matter of adding the try keyword if it doesn’t exist yet. For other methods, it should be fairly simple to log the failure by wrapping the access to documentManager in a orLog which will log the error and return nil if getting documentManager throws.

ahoppen · 2024-06-20T15:58:33Z

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

+  var documentManager: DocumentManager {
+    get throws {
+      guard let sourceKitLSPServer = self.sourceKitLSPServer else {
+        throw NSError(domain: "SourceKitLSPErrorDomain", code: 1001, userInfo: [NSLocalizedDescriptionKey: "Cannot get document manager because sourceKitLSPServer was deallocated"])


We generally don’t use NSError in SourceKit-LSP. I would either:

Define a custom error type that we can throw here or

follow the precedence set here and return ResponseError.unknown("Connection to the editor closed")

Yeah I feel like ResponseError.unknown would be suffice here, having a custom error seems a bit of an overkill for now

louisunlimited · 2024-06-22T21:39:56Z

Hey, sorry if this is taking a while, never had a good chance to sit down and dig into this. I'm still experiencing the same issues locally when I run swift test. All the notification tests are timing out for some reason that I don't quite understand yet. Also the warnings are still there. I probably need some help on this to pin point the issue if you have time

ahoppen

Looks great. Thank you @louisunlimited! Do you want to mark the PR as ready for review so we can get it merged?

ahoppen · 2024-06-24T14:21:59Z

Ah, looks like a merge conflict has crept up. Could you rebase your PR onto main? Also, could you squash your commits int a single one. It makes for a nicer Git history. https://github.com/apple/sourcekit-lsp/blob/main/CONTRIBUTING.md#authoring-commits

louisunlimited · 2024-06-26T13:24:40Z

Ah, looks like a merge conflict has crept up. Could you rebase your PR onto main? Also, could you squash your commits int a single one. It makes for a nicer Git history. https://github.com/apple/sourcekit-lsp/blob/main/CONTRIBUTING.md#authoring-commits

Ah shoot I just did merge instead of rebase. I can redo that

ahoppen

Sorry, I just found a few more comments. Not sure how I spotted them before. 🙈

ahoppen · 2024-06-27T05:22:53Z

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

@@ -432,7 +437,7 @@ extension SwiftLanguageService {
    cancelInFlightPublishDiagnosticsTask(for: notification.textDocument.uri)
    await diagnosticReportManager.removeItemsFromCache(with: notification.textDocument.uri)

-    guard let snapshot = self.documentManager.open(notification) else {
+    guard let snapshot = try? self.documentManager.open(notification) else {


Could you use orLog here as well?

Suggested change

guard let snapshot = try? self.documentManager.open(notification) else {

guard let snapshot = orLog("Getting document manager", { try self.documentManager.open(notification) }) else {

ahoppen · 2024-06-27T05:24:08Z

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

+    orLog("Connection to the editor closed") {
+      try? self.documentManager.close(notification)
+    }


I think the orLog doesn’t do anything here because documentManager.close doesn’t actually throw since you’re using try?.

Suggested change

orLog("Connection to the editor closed") {

try? self.documentManager.close(notification)

}

orLog("Getting document manager") {

try self.documentManager.close(notification)

}

ahoppen · 2024-06-27T05:24:21Z

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

@@ -543,7 +550,7 @@ extension SwiftLanguageService {
      let replacement: String
    }

-    guard let (preEditSnapshot, postEditSnapshot, edits) = self.documentManager.edit(notification) else {
+    guard let (preEditSnapshot, postEditSnapshot, edits) = try? self.documentManager.edit(notification) else {


Similar here, could you use orLog so we log an error if the document manager is nil?

Suggested change

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

guard let (preEditSnapshot, postEditSnapshot, edits) = orLog("Getting document manager", { try self.documentManager.edit(notification) } else {

louisunlimited · 2024-06-27T13:43:25Z

@ahoppen Just made these changes and squashed my commits, I'm not no my personal computer now so I can't test till later today, let me now if you need anything else! Thanks

ahoppen

Let’s give it a shot 🚀

ahoppen · 2024-06-27T19:48:57Z

@swift-ci Please test

ahoppen · 2024-06-28T08:16:34Z

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

@@ -543,7 +550,7 @@ extension SwiftLanguageService {
      let replacement: String
    }

-    guard let (preEditSnapshot, postEditSnapshot, edits) = self.documentManager.edit(notification) else {
+    guard let (preEditSnapshot, postEditSnapshot, edits) = orLog("Getting document manager", { try self.documentManager.edit(notification) }) else {


CI says this line exceeds the maximum line lengths 😢 . Could you run swift-format? https://github.com/swiftlang/sourcekit-lsp/blob/main/CONTRIBUTING.md#formatting

ahoppen · 2024-06-28T16:34:00Z

@swift-ci Please test

ahoppen · 2024-06-28T16:54:03Z

@swift-ci Please test Windows

louisunlimited · 2024-06-28T19:43:30Z

Yeah these failed test are something I probably need some help with

ahoppen · 2024-07-01T07:19:26Z

The issue was that SourceKitLSPServer opened the document in the document manager and then SwiftLanguageService tried opening the document in the same document manager again, which failed. Thus snapshot in openDocument was nil, we never opened the document in sourcekitd and thus couldn’t provide any semantic functionality for the document. I noticed this by looking at the logs generated during test execution and saw an error being logged that said failed to open document: alreadyOpen(…

The solution is that only SourceKitLSPServer should be responsible for updating the document manager state.

The following diff fixes the issue and also does a few clean-up, inlining methods that are now only called from one location.

Diff

diff --git a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift
index 0fcd6b61..9d2f5faf 100644
--- a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift
+++ b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift
@@ -17,6 +17,7 @@ import LanguageServerProtocolJSONRPC
 import SKCore
 import SKSupport
 import SwiftExtensions
+import SwiftSyntax
 
 import struct TSCBasic.AbsolutePath
 
@@ -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
@@ -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)
   }
 
diff --git a/Sources/SourceKitLSP/DocumentManager.swift b/Sources/SourceKitLSP/DocumentManager.swift
index 97cfdc62..3d0aaa61 100644
--- a/Sources/SourceKitLSP/DocumentManager.swift
+++ b/Sources/SourceKitLSP/DocumentManager.swift
@@ -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`.
   ///
diff --git a/Sources/SourceKitLSP/LanguageService.swift b/Sources/SourceKitLSP/LanguageService.swift
index 1b2429e3..638c0d0a 100644
--- a/Sources/SourceKitLSP/LanguageService.swift
+++ b/Sources/SourceKitLSP/LanguageService.swift
@@ -13,6 +13,7 @@
 import Foundation
 import LanguageServerProtocol
 import SKCore
+import SwiftSyntax
 
 /// The state of a `ToolchainLanguageServer`
 public enum LanguageServerState {
@@ -113,9 +114,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
@@ -127,7 +128,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
 
diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift
index fe41305f..ab176ebe 100644
--- a/Sources/SourceKitLSP/SourceKitLSPServer.swift
+++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift
@@ -1251,7 +1251,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
@@ -1265,7 +1276,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 {
@@ -1293,7 +1304,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
 
@@ -1314,8 +1327,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(
diff --git a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift
index bf137e15..119e9840 100644
--- a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift
+++ b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift
@@ -430,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 = orLog("Getting document manager", { try self.documentManager.open(notification) }) else {
-      // Already logged failure.
-      return
-    }
-
     let buildSettings = await self.buildSettings(for: snapshot.uri)
 
     let req = openDocumentSourcekitdRequest(snapshot: snapshot, compileCommand: buildSettings)
@@ -451,10 +446,6 @@ extension SwiftLanguageService {
     inFlightPublishDiagnosticsTasks[notification.textDocument.uri] = nil
     await diagnosticReportManager.removeItemsFromCache(with: notification.textDocument.uri)
 
-    orLog("Getting document manager") {
-      try self.documentManager.close(notification)
-    }
-
     let req = closeDocumentSourcekitdRequest(uri: notification.textDocument.uri)
     _ = try? await self.sourcekitd.send(req, fileContents: nil)
   }
@@ -537,7 +528,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
@@ -547,15 +543,6 @@ extension SwiftLanguageService {
       let replacement: String
     }
 
-    guard
-      let (preEditSnapshot, postEditSnapshot, edits) = orLog(
-        "Getting document manager",
-        { try self.documentManager.edit(notification) }
-      )
-    else {
-      return
-    }
-
     for edit in edits {
       let req = sourcekitd.dictionary([
         keys.request: self.requests.editorReplaceText,
@@ -1003,7 +990,6 @@ extension SwiftLanguageService: SKDNotificationHandler {
     }
 
     if notification.error == .connectionInterrupted {
-      self.state = .connectionInterrupted
       await self.setState(.connectionInterrupted)
     }
   }

louisunlimited · 2024-07-02T03:46:50Z

Oh thank you so much for this, I just did a quick fix but that introduced some other error; I'll look into this later tomorrow. Appreciate the help!

ahoppen · 2024-07-02T04:51:59Z

Let me know when it’s ready for review again.

louisunlimited · 2024-07-03T03:46:08Z

Hi @ahoppen, I just found out it's due to some of my dependencies being outdated, after I pulled all the external deps it builds fine. I was also able to run test and lint, let me know if CI somehow fails again and I can work on that tomorrow!

I was actually facing an error in SourceKitLSPTests.ExecuteCommandTests testFreestandingMacroExpansion saying that

 error: -[SourceKitLSPTests.ExecuteCommandTests testFreestandingMacroExpansion] : failed: caught error: "ResponseError(code: LanguageServerProtocol.ErrorCode(rawValue: -32001), message: "Unknown error: no edits reported for semantic refactoring action for url file:///private/var/folders/_9/4xjxjjbj0qg2tm33d82jh2qw0000gn/T/sourcekit-lsp-test-scratch/testFreestandingMacroExpansion-2D573E51/Sources/MyMacroClient/MyMacroClient.swift")"

sourcekit-lsp/Tests/SourceKitLSPTests/ExecuteCommandTests.swift:210: error: -[SourceKitLSPTests.ExecuteCommandTests testFreestandingMacroExpansion] : Failed due to unwaited expectation 'Handle Show Document Request'.

Interestingly this particular test also fails on a fresh clone of the repo's main branch so I wasn't sure if it's something related to my changes.

ahoppen · 2024-07-03T07:56:47Z

I was actually facing an error in SourceKitLSPTests.ExecuteCommandTests testFreestandingMacroExpansion

Thanks for letting me know. That test was missing a SkipUnless check and should have been skipped if you’re running tests using a Swift 5 toolchain. #1548

ahoppen · 2024-07-03T07:57:05Z

@swift-ci Please test

ahoppen · 2024-07-03T08:00:26Z

@swift-ci Please test Windows

louisunlimited · 2024-07-03T14:51:27Z

Uhm why did this CI fail when it says no identified problem. I'll rebase main and see what happens from there

ahoppen · 2024-07-03T21:02:19Z

CI failed with

+ /Users/ec2-user/jenkins/workspace/swift-sourcekit-lsp-PR-macOS/branch-main/build/buildbot_incremental/unified-swiftpm-build-macosx-x86_64/release/swift-format lint --parallel --strict --recursive /Users/ec2-user/jenkins/workspace/swift-sourcekit-lsp-PR-macOS/branch-main/sourcekit-lsp
branch-main/sourcekit-lsp/Sources/SourceKitLSP/LanguageService.swift:130:103: warning: [RemoveLine] remove line break
branch-main/sourcekit-lsp/Sources/SourceKitLSP/LanguageService.swift:131:1: warning: [TrailingWhitespace] remove trailing whitespace
branch-main/sourcekit-lsp/Sources/SourceKitLSP/LanguageService.swift:132:1: warning: [TrailingWhitespace] remove trailing whitespace

Could you run swift-format on your changes? https://github.com/swiftlang/sourcekit-lsp/blob/main/CONTRIBUTING.md#formatting

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 lint: ran swift format

louisunlimited · 2024-07-04T04:53:36Z

Yeah that's my bad, some how forgot to commit that file. Should be updated now!

ahoppen · 2024-07-04T09:15:50Z

@swift-ci Please test

ahoppen · 2024-07-04T09:27:39Z

@swift-ci Please test Windows

ahoppen reviewed Jun 17, 2024

View reviewed changes

ahoppen reviewed Jun 20, 2024

View reviewed changes

ahoppen approved these changes Jun 24, 2024

View reviewed changes

louisunlimited marked this pull request as ready for review June 26, 2024 13:24

louisunlimited force-pushed the main branch 2 times, most recently from b744285 to f0e8710 Compare June 26, 2024 17:23

ahoppen reviewed Jun 27, 2024

View reviewed changes

louisunlimited force-pushed the main branch 2 times, most recently from 8596b21 to 1e669d2 Compare June 27, 2024 13:42

ahoppen approved these changes Jun 27, 2024

View reviewed changes

ahoppen reviewed Jun 28, 2024

View reviewed changes

louisunlimited force-pushed the main branch 3 times, most recently from 052058e to 3230d44 Compare June 28, 2024 15:56

ahoppen approved these changes Jun 28, 2024

View reviewed changes

ahoppen enabled auto-merge June 28, 2024 16:34

ahoppen disabled auto-merge June 28, 2024 16:54

ahoppen enabled auto-merge (squash) June 28, 2024 16:54

auto-merge was automatically disabled July 2, 2024 03:37
Head branch was pushed to by a user without write access

louisunlimited force-pushed the main branch from b8ee006 to 3b8cb89 Compare July 2, 2024 03:37

louisunlimited force-pushed the main branch from 3b8cb89 to 89fec06 Compare July 2, 2024 03:38

louisunlimited force-pushed the main branch 2 times, most recently from 60293cf to 78dfaf1 Compare July 3, 2024 03:51

louisunlimited requested a review from ahoppen July 3, 2024 03:51

louisunlimited force-pushed the main branch from b02afe8 to b33fe4f Compare July 3, 2024 14:57

louisunlimited force-pushed the main branch from b33fe4f to 3be0342 Compare July 4, 2024 04:48

lokesh-tr mentioned this pull request Jul 4, 2024

Skip testAttachedMacroExpansion if host toolchain does not support background indexing #1553

Merged

ahoppen enabled auto-merge July 4, 2024 09:15

ahoppen merged commit b3fe0c8 into swiftlang:main Jul 4, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove document manager in `SwiftLanguageService` #1466

Remove document manager in `SwiftLanguageService` #1466

louisunlimited commented Jun 8, 2024

louisunlimited commented Jun 13, 2024 •

edited

Loading

ahoppen Jun 17, 2024

louisunlimited Jun 19, 2024

ahoppen Jun 20, 2024

ahoppen Jun 20, 2024

louisunlimited Jun 22, 2024 •

edited

Loading

louisunlimited commented Jun 22, 2024 •

edited

Loading

ahoppen left a comment

ahoppen commented Jun 24, 2024

louisunlimited commented Jun 26, 2024

ahoppen left a comment

ahoppen Jun 27, 2024

ahoppen Jun 27, 2024

ahoppen Jun 27, 2024

louisunlimited commented Jun 27, 2024

ahoppen left a comment

ahoppen commented Jun 27, 2024

ahoppen Jun 28, 2024

louisunlimited Jun 28, 2024 •

edited

Loading

ahoppen commented Jun 28, 2024

ahoppen commented Jun 28, 2024

louisunlimited commented Jun 28, 2024

ahoppen commented Jul 1, 2024 •

edited

Loading

louisunlimited commented Jul 2, 2024

ahoppen commented Jul 2, 2024

louisunlimited commented Jul 3, 2024

ahoppen commented Jul 3, 2024

ahoppen commented Jul 3, 2024

ahoppen commented Jul 3, 2024

louisunlimited commented Jul 3, 2024 •

edited

Loading

ahoppen commented Jul 3, 2024 •

edited

Loading

louisunlimited commented Jul 4, 2024

ahoppen commented Jul 4, 2024

ahoppen commented Jul 4, 2024

	guard let snapshot = try? self.documentManager.open(notification) else {
	guard let snapshot = orLog("Getting document manager", { try self.documentManager.open(notification) }) else {

	guard let (preEditSnapshot, postEditSnapshot, edits) = try? self.documentManager.edit(notification) else {
	guard let (preEditSnapshot, postEditSnapshot, edits) = orLog("Getting document manager", { try self.documentManager.edit(notification) } else {

Remove document manager in SwiftLanguageService #1466

Remove document manager in SwiftLanguageService #1466

Conversation

louisunlimited commented Jun 8, 2024

louisunlimited commented Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

louisunlimited Jun 22, 2024 • edited Loading

Choose a reason for hiding this comment

louisunlimited commented Jun 22, 2024 • edited Loading

ahoppen left a comment

Choose a reason for hiding this comment

ahoppen commented Jun 24, 2024

louisunlimited commented Jun 26, 2024

ahoppen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

louisunlimited commented Jun 27, 2024

ahoppen left a comment

Choose a reason for hiding this comment

ahoppen commented Jun 27, 2024

Choose a reason for hiding this comment

louisunlimited Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

ahoppen commented Jun 28, 2024

ahoppen commented Jun 28, 2024

louisunlimited commented Jun 28, 2024

ahoppen commented Jul 1, 2024 • edited Loading

louisunlimited commented Jul 2, 2024

ahoppen commented Jul 2, 2024

louisunlimited commented Jul 3, 2024

ahoppen commented Jul 3, 2024

ahoppen commented Jul 3, 2024

ahoppen commented Jul 3, 2024

louisunlimited commented Jul 3, 2024 • edited Loading

ahoppen commented Jul 3, 2024 • edited Loading

louisunlimited commented Jul 4, 2024

ahoppen commented Jul 4, 2024

ahoppen commented Jul 4, 2024

Remove document manager in `SwiftLanguageService` #1466

Remove document manager in `SwiftLanguageService` #1466

louisunlimited commented Jun 13, 2024 •

edited

Loading

louisunlimited Jun 22, 2024 •

edited

Loading

louisunlimited commented Jun 22, 2024 •

edited

Loading

louisunlimited Jun 28, 2024 •

edited

Loading

ahoppen commented Jul 1, 2024 •

edited

Loading

louisunlimited commented Jul 3, 2024 •

edited

Loading

ahoppen commented Jul 3, 2024 •

edited

Loading