When `DYLD_(FRAMEWORK|LIBRARY)_PATH` is set, `dlopen` will first check
if the basename of the provided path is within any of its search paths.
Thus it's possible that only a single library is loaded for each
toolchain, rather than a separate like we expect. The paths should be
equal in this case, since the client plugin is loaded based on the path
of `sourcekitd.framework` (and we should only have one for the same
reason). Allow this case and just avoid re-initializing.
This should be a last stop-gap measure in case sourcekitd or clangd get stuck, don’t respond to any requests anymore and don’t honor cancellation either. In that case we can restore SourceKit-LSP behavior by killing them and using the crash recovery logic to restore functionality.
rdar://149492159
Just something I noticed while looking through code. I am not aware of any issues caused by this.
Also relax the `precondition` in `DLHandle.deinit` to a fault. If we do not close a `DLHandle`, that’s a bug but it’s not so bad that we should crash sourcekit-lsp for it.
We don’t have any guarantees which thread these blocks will be called on by sourcekitd, so we shouldn’t make any assumptions about it in Swift. We should thus mark them as Sendable.
There is only one real class that implements the `SourceKitD` protocol, so there really isn’t any need for the protocol + class split at all. Unify them to make code simpler to reason about.
Otherwise, the cancellation could get handled before the request actually gets sent to sourcekitd, which is not what we want to test here. There appears to be a second underlying issue that causes unexpected results when we cancel the fast completion request after sending the slow completion request. I’ll look at that in a follow-up PR.
Otherwise, we would log sending the sourcekitd request even if the task was already cancelled and we thus took an early exit in `withCancellableCheckedThrowingContinuation`.
This adds a sourcekitd plugin that drives the code completion requests. It also includes a `CompletionScoring` module that’s used to rank code completion results based on their contextual match, allowing us to show more relevant code completion results at the top.
Fixes all build warnings in SourceKit-LSP so that it builds without any issues using recent Swift development snapshots (`swift-DEVELOPMENT-SNAPSHOT-2024-07-22-a` and `swift-6.0-DEVELOPMENT-SNAPSHOT-2024-07-24-a`)
Change a l public declarations to the `package` access level, accept for:
- The `LanguageServerProtocol` module
- The `BuildServerProtocol` module
- `InProcessClient.InProcessSourceKitLSPClient`
- `LanguageServerProtocolJSONRPC` (I would like to create a more ergonomic API for this like `InProcessSourceKitLSPClient` in the future, but for now, we’ll leave it public)
Unfortunately, our pattern of marking functions as `@_spi(Testing) public` no longer works with the `package` access level because declarations at the `package` access level cannot be marked as SPI. I have decided to just mark these functions as `package`. Alternatives would be:
- Add an underscore to these functions, like we did for functions exposed for testing before the introduction of `SPI`
- Use `@testable` import in the test targets and mark the methods as `internal`
Resolves#1315
rdar://128295618
VS Code does not cancel semantic tokens requests. If a source file gets into a state where an AST build takes very long, this can cause us to wait for the semantic tokens from sourcekitd for a few minutes, effectively blocking all other semantic functionality in that file.
To circumvent this problem (or any other problem where an editor might not be cancelling requests they are no longer interested in) add a maximum request duration for SourceKitD requests, defaulting to 2 minutes.
rdar://130948453
The purpose of the different modules wasn’t clearly defined, which lead to inconsistent responsibilities between the different modules. Define each module’s purpose and move a few files between modules to satisfy these definitions.
There are a few more larger changes that will need to be made for a fully consistent module structure. These are FIXMEs in the new Modules.md document and I’ll address them in follow-up PRs.
Turns out that sourcekitd test hooks were a bad idea because of the following comment that I wrote:
```
`testHooks` are only considered when an instance is being created. If a sourcekitd instance at the given path already exists, its test hooks will be used.
```
During test execution in Xcode, we generate a bunch of `SourceKitServer` instances in the same process that all call `DynamicallyLoadedSourceKitD.getOrCreate`. Now, if `testDontReturnEmptyDiagnosticsIfDiagnosticRequestIsCancelled` is not the first test being executed in the process (which usually it is not), the test hooks in it won’t get used.
Switch back to using the preparation hooks, essentially reverting https://github.com/apple/sourcekit-lsp/pull/1412 and keeping the following snippet to fix the underlying issue
```swift
// Poll until the `CancelRequestNotification` has been propagated to the request handling.
for _ in 0..<Int(defaultTimeout * 100) {
if Task.isCancelled {
break
}
usleep(10_000)
}
```
I saw a few non-deterministic test failures. I think the issue was that handling of the `CancelRequestNotification` is done asynchronously, which left a short window in which the sourcekitd diagnostics request could run and return results instead of being cancelled. Wait for the diagnostic request to actually be cancelled before running it for real.
While doing this, also introduce proper sourcekitd test hooks instead of relying on the preparation test hooks, which just got run as a side effect.
Otherwise, I think `ThreadSafeBox` might still have data races. This also requires us to make `TestSourceKitLSPClient.RequestHandler` sendable.
rdar://128572489
I saw a failure in CI where a sourcekitd diagnostic request got cancelled without an LSP cancellation request. I am suspecting that there is some implicit cancellation going on in sourcekitd despite `key.cancel_builds: 0`, which doesn’t make sense to me. Adding some explicit logging to sourcekit-lsp to check if we cancel the request for some reason.
Make logging protocol requirements on `SourceKitD` so that implementers of the protocol can decide how to log requests. This allows us to add different kinds of sourcekitd connections that behave differently in the future, for example one that dynamically links against sourcekitd instead of loading it it using `dlopen`.
Naming types in sourcekitd_functions.h `sourcekit_api_` instead of `sourcekitd_` indicates that these are types to be used with dynamically loaded sourcekitd libraries. It avoids confusion if sourcekitd is also linked, which adds the `sourcekitd_` symbols.
Adding nullability annotations to it is also just nice.
And some improved formatting never hurts.
When receiving a `CancellationNotification`, we cancel the task that handles the request with that ID.
This will cause `cancel_notification` to be sent to sourcekitd or a `CancellationNotification` to be sent to `clangd`, which ultimately cancels the request.
rdar://117492860
OSLog is the suggesting logging solution on Apple platforms and we should be using it there, taking advantage of the different log levels and privacy masking.
Switch sourcekit-lsp to use OSLog on Apple platforms and implement a logger that is API-compatible with OSLog for all uses in sourcekit-lsp and which can be used on non-Darwin platforms.
The goal of this commit is to introduce the new logging API. There are still improvements about what we log and we can display more privacy-insensitive information after masking. Those changes will be in follow-up commits.
Add `.swift-format` to the repo and format the repo with `swift-format`.
This commit does not add any automation to enforce formatting of sourcekit-lsp in CI. The goal of this commit is to get the majority of source changes out of the way so that the diff of actually enforcing formatting will have fewer changes or conflicts.
