Preserve language service when closing generated interface#2598
Preserve language service when closing generated interface#2598edwardlee wants to merge 1 commit into
Conversation
edwardlee
requested review from
ahoppen,
hamishknight and
rintaro
as code owners
|
Good find, thanks! Could you add a test case that fails without this change? |
|
On second thought, what if the documentManager document is closed first while the related generated interfaces are still open? Do they lose the language service? |
Closing a generated interface document (sourcekit-lsp:// URI) was removing the language service for the originating source file because both share the same buildSettingsFile key. Guard the removal so it only happens when no other open document shares that key. Relates to swiftlang#2209
edwardlee
force-pushed
the
handle-interface-close
branch
from
30d1d29 to
0c3a2f9
Compare
|
Added test similar to testNoDiagnosticsInGeneratedInterface. Since the logic checks for any remaining open file with the key before shutting down LSP, both cases work. Tying to the uri makes sense, this was just simpler. |
|
@swift-ci Please test |
rintaro
left a comment
rintaro
left a comment
There was a problem hiding this comment.
Thank you for the test case!
I think the logic is fine. Just a code placement concern.
| } | ||
|
|
||
| workspace.removeLanguageServices(for: uri) | ||
| if !documentManager.openDocuments.contains(where: { $0.buildSettingsFile == uri.buildSettingsFile }) { |
There was a problem hiding this comment.
Using .buildSettingsFile as the key is an implementation detail of Workspace, so ideally this logic should remain encapsulated there.
Could you move this into Workspace.removeLanguageServices(for:)? You should be able to access the document manager via self.sourceKitLSPServer?.documentManager.
Also, maybe removeLanguageServices(for:) should be renamed to unsetLanguageServices(for:) to pair better with setLanguageServices(for:_:).
|
@swift-ci Please test Windows |
🤔 |
|
@swift-ci Please test |
|
Hi @edwardlee, again, great catch and thanks for the fix! Since this is a serious issue we need to fix asap, I've opened #2648 to get this merged sooner. |
2 participants
Closing a generated interface was removing the language service for the originating file because both share the same buildSettingsFile key. https://github.com/swiftlang/sourcekit-lsp/blob/main/Sources/SourceKitLSP/ReferenceDocumentURL.swift#L125
Guard the removal by when no other open document shares that key.
See #2209 and 7437d92