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
# Conflicts:
# Tests/SourceKitLSPTests/BackgroundIndexingTests.swift
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.
# Conflicts:
# Sources/SourceKitLSP/SourceKitLSPServer+Options.swift
# Sources/SourceKitLSP/Swift/SwiftLanguageService.swift
# Sources/sourcekit-lsp/SourceKitLSP.swift
# Tests/SourceKitLSPTests/BackgroundIndexingTests.swift
# Tests/SourceKitLSPTests/ExecuteCommandTests.swift
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.
This allows us to run `sourcekit-lsp index --project /path/to/project` to index a project. Intended to debugging purposes, eg.
- Profile the time it takes to index a project
- See if the project can be indexed successfully
- Look at signposts generated during indexing in Instruments to see whether indexing or preparation is the bottleneck and how well we can parallelize tasks.
This allows a user of SourceKit-LSP to inspect the result of background indexing. This allows a user of SourceKit-LSP to inspect the result of background indexing. I think this gives useful insights into what SourceKit-LSP is indexing and why/how it fails, if it fails, also for users of SourceKit-LSP.
rdar://127474136
Fixes#1265
We weren’t logging requests sent to a `TestSourceKitLSPClient` because we were assuming that `JSONRPCConnection` logs those requests in `SourceKitLSPServer`. But no logging happens in `LocalConnection`, which `TestSourceKitLSPClient` uses.
Since `LangaugeServerProtcol` can’t depend on `LSPLogging`, move the type to `LSPTestsSupport`.
When the client sends us `workspace/didChangeWatchedFiles` notification of an updated `.swift` file, we should refresh the other open files in that module since they might be referencing functions from that updated file.
If a `.swiftmodule` file has been updated, we refresh all the files within the package since they might import that module. Technically, we would only need to refresh files that are in module that are downstream of the updated module but we don’t currently have that information easily available from SwiftPM. Also, usually, if the client has a file from a low-level module open, he’ll be working on that module which means that such an optimization won’t help. The real solution here is to wait for us to finish preparation (which we would exactly know when it finishes since sourcekit-lsp would schedule it) but for that we need to implement background preparation.
Fixes#620Fixes#1116
rdar://99329579
rdar://123971779
Instead of returning `nil` to indicate that the position conversion failed, log a fault and perform a best-effort recovery.
I think this allows us to perform better recovery and also makes code calling these position conversions a lot simpler because it doesn’t need to make decisions about what to do if position conversions fail.
Rename all the classes that write files to disk to create a test project that we can open in sourcekit-lsp to end with `TestProject`. This is better than the old `Workspace` suffix because it avoids ambiguities with the `Workspace` type inside sourcekit-lsp.
- IndexedSingleSwiftFileWorkspace -> IndexedSingleSwiftFileTestProject
- MultiFileTestWorkspace -> MultiFileTestProject
- SwiftPMTestWorkspace -> SwiftPMTestProject
The client ID was needed when a `MessageHandler` could handle messages from multiple connections. We don’t support this anymore (because it wasn’t needed) and so the client ID doesn’t need to get passed through as well.
Currently, all tests send publish diagnostics notifications, which is noise in the logs for most tests. Change the tests to use the pull diagnostics model by default and make the push diagnostic model opt-in.
rdar://123241539
This should help users debug issues when functionality is not working because compiler arguments are missing.
Note that when opening a standalone Swift file, we consider that file as having non-fallback build settings, even though the build settings are synthesized from the fallback build system. Thus, this will only pop up when the user is in a context that has some build system (like a compilation database or a Swift package) but the current file isn’t handled by that.
rdar://119741960
This requires a bit of a restructuring of `ToolchainRegistry` because calling `scanForToolchain` on `self` is a suspension point. Instead, we need to implement the entire toolchain discovery in the initializer itself, which really isn’t that bad anyway.
This allows us to make the `SourceKitLSP.run` function synchronous again.
Fixes#1023
rdar://120976054
Global state is never a good thing and we needed to modify it in tests. The design becomes a lot cleaner if we explicitly pass the toolchain registry around.
The new approach has a few advantages over the olde TIBS-based approach:
1. The source file being tested is defined within the test case itself and not in a separate file, which makes it easier to understand the test case since the auxiliaury file doesn’t need to be opened. Finding it inside `Sources/SKTestSupport/INPUTS` is already hard for people that are not familiar with the codebase.
2. The build setup is significantly simpler since it doesn’t rely on `ninja`. It is thus easier to understand what is run during the test.
3. We can use the emoji location markers to refer to test locations, like we do for files that are opened using `TestSourceKitLSPClient.openDocument`.
This commit only migrates call hierarchy testing to the new design. If we like it, I’ll migrate the other test workspaces as well.
Sending the `InitializeRequest` was always unnecessarily verbose. If we automatically initialize the server when creating the test client, the code becomes sufficiently concise that we can set up the client + server in the test method itself and no longer need the `setUp` methods, making the tests easier to read.
Instead of listening for document updates sent from `sourcekitd` and sending a `PublishDiagnosticsNotification` based on the `sourcekitd` notification, wait for a little while and then execute an internal `DocumentDiagnosticsRequest` to load diagnostics and send them to the client.
This has two advantages:
- It unifies the two diagnostic implementations
- It removes the need to keep track of semantic diagnostics in `SwiftLanguageServer`
- It gets us one step closed to opening and editing documents in `syntactic_only` mode. The only thing that is left now are semantic tokens.
