This allows us to clean up the creation of `TestBuildSystem` a little bit because the tests can create `TestBuildSystem` instead of retrieving it from the `BuildSystemManager`.
rdar://142906050
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.
https://github.com/swiftlang/sourcekit-lsp/pull/1714 changed the background preparation mode to `enabled` but a Swift 5.10 toolchain does not support `--experimental-prepare-for-indexing`. Thus, these tests fail. Skip tests that rely on background indexing when testing SourceKit-LSP with a Swift 5.10 host toolchain.
When `TestSourceKitLSPClient.nextNotification` timed out, it would cancel `AsyncSequence.Iterator.next()` and thus also cancel the underlying `AsyncStream` (which I only now found out). This means that calling `nextNotification` again would never return any notification. Hence, if we didn’t receive a `PublishDiagnosticsNotification` within the first timeout, we would never get one.
In some situations, we could return the timeout error from the timeout task, but receive a notification from the `self.notifications` `AsyncStream`. That notification would then be dropped and never get delivered to the test case, which can cause test cases to fail.
`URL.path` returns forward slashes in the path on Windows (https://github.com/swiftlang/swift-foundation/issues/973) where we expect backslashes. Work around that by defining our own `filePath` property that is backed by `withUnsafeFileSystemRepresentation`, which produces backslashes.
rdar://137963660
Currently, we return a `methodNotFound` error when we get a diagnostic refresh request from SourceKit-LSP. This doesn’t do any harm but draws the attention when reading test logs, despite being the expected result. Clean up that output by returning a proper response.
We forgot to decode the following keys in the custom decode function, which meant that you couldn’t set them using SourceKit-LSP’s `config.json` file.
- `backgroundPreparationMode`
- `sourcekitdRequestTimeout`
- `cancelTextDocumentRequestsOnEditAndClose`
We had the custom decoder function so that the keys weren’t required in the JSON but we could access eg. `SwiftPMOptions` without needing to deal with optionals in the codebase.
Make the accesses to these nested options structs a little more verbose but eliminate the source of the above bug, which seems like a good tradeoff.
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
This allows us to flip the default in the future more easily. It also allows users to disable background indexing when it’s enabled by default.
rdar://130280855
The idea here is to unify the different ways in which we can currently set options on SourceKit-LSP in a scalable way: Environment variables, command line arguments to `sourcekit-lsp` and initialization options.
The idea is that a user can define a `~/.sourcekit-lsp/.sourcekit-lsp` file (we store logs in `~/.sourcekit-lsp/logs` on non-Darwin platforms), which will be used as the default configuration for all SourceKit-LSP instances. They can also place a `.sourcekit-lsp` file in the root of a workspace to configure SourceKit-LSP for that project specifically, eg. setting arguments that need to be passed to `swift build` for that project and which thus also need to be set on SourceKit-LSP.
For compatibility reasons, I’m mapping the existing command line options into the new options structure for now. I hope to delete the command line arguments in the future and solely rely on `.sourcekit-lsp` configuration files.
Environment variable will be migrated to `.sourcekit-lsp` in a follow-up commit.
Since the `Atomic*` types can not be marked as `Sendable` (because they aren’t C structs), we can change the variables to constants and can remove `nonisolated(unsafe)`.
We used C atomics but these were allocated as Swift variables. Even thought they were atomic, concurrent accesses to them could violate Swift’s exclusivity laws, raising thread sanitizer errors.
Allocate the C atomics using malloc to fix this problem.
rdar://129170128
I’d like to qualify `--experimental-prepare-for-indexing` independently of background indexing using `swift build`. Because of this, I think there is value in using SourceKit-LSP using background indexing but without `--experimental-prepare-for-indexing`. It could also be useful to determine if bugs we may find are due to `--experimental-prepare-for-indexing` or also occur when running plain `swift build` commands.
This also means that you can use the index log to view which tasks are currently being executed.
Since we only have a single log stream we can write to, I decided to prefix every line in the index log with two colored emojis that an easy visual association of every log line to the task that generated them.
Otherwise, I think `ThreadSafeBox` might still have data races. This also requires us to make `TestSourceKitLSPClient.RequestHandler` sendable.
rdar://128572489
This improves serial test execution time of eg. `DocumentTestDiscoveryTests` from 36s to 22s because we don’t need to re-build the XCTest module from its interface when using an open source toolchain.
This also uncovered that we weren‘t passing the build setup flags to the prepare command.
rdar://126493151
Background indexing probably won’t be the last experimental feature in sourcekit-lsp that we want to gate behind a feature flag. Instead of adding new parameters ad-hoc, introduce a general notion of experimental features.
Package manifests don’t have an associated target to prepare and are represented by a `ConfiguredTarget` with an empty target ID. We were mistakingly running `swift build` with an empty target name to prepare them, which failed. There is nothing to prepare.
We were catching errors during diagnostic generation because VS Code will not request diagnostics again if SourceKit-LSP returns an error to any diagnostic request. We were, however, a little over-eager when doing this and also caught cancellation errors, which means that if the user made multiple edits in quick succession (which would cancel the diagnostics request from the first edit), we would return empty diagnostics, clearing the displayed diagnostics.
Diagnostics are usually not helpful if the file’s target hasn’t been prepared yet. We should await the target’s preparation before returning diagnostics.
rdar://128645617
Rebuilding the build graph can take a while (initial loading of the build graph takes ~7s for sourcekit-lsp) and it’s good to show some progress during this time.
