Previously, we `fatalError`ing when `JSONDecoder` failed to decode a message from the client. Instead of crashing, try recovering from such invalid messages as best as possible. If we know that the state might have gotten out of sync with the client, show a notification message to the user, asking them to file an issue.
rdar://112991102
This started off by guarding `state` against concurrent access and then escalated slightly.
The main changes are:
- `state` must only be accessed on `queue` now, preventing race conditions of message handling while the connection is being closed.
- A few functions were renamed, merged, and documented to simplify the code
- A few dead members were removed
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.
Not entirely sure what introduced the race condition. Found by thread sanitizer. Probably wouldn’t have been an issue if we had migrated this code to strict concurrency checking.
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.
`Sleep` is a `void` returning function, which makes the blackhole
redundant. Remove the unnecessary assignment which also silences
the warning associated with it.
Instead of having ad-hoc timeout durations in all the test cases, specify a default timeout duration that can be used by tests.
This allows us increase the timeout duration for all tests if we discover that e.g. sourcekitd is slower in CI setups.
rdar://91615376
Workaround for SR-16097 to avoid hitting SR-16095: VSCode will send shutdown requests without a 'params' key. On macOS getting the superDecoder for a non-existent container returns an empty decoder, on Linux it throws `DecodingError.keyNotFound`, causing a crash.
Perform a targeted fix: If we don't have 'params' and the request is a ShutdownRequest, manually create a `ShutdownRequest` without any parameters.
rdar://91288093
A number of tests were failing with the -Onone lifetime changes.
Regardless of what happens with that change, I'd like to keep our tests
passing with the stricter rules since we also test in release builds.
The underlying JSON parsing error is from Foundation, so we don't want
to try to compare that in our tests. As a quick fix, just compare a
prefix of the string that we control.
This set of changes is the last set of changes required to build the
entire test suite on Windows. Although the tests do not yet fully pass,
this is a prerequisite for getting the test suite passing.
This actually addresses the real issue that was ignored earlier about
pipes on Windows. The FileHandle cannot provide a non-owning file
descriptor (the returned file descriptor would need to be explicitly
`_close`'d by the receiver). Foundation now vends a `_handle` accessor
to the OS primitive handle. Use this to create the dispatch loop for
messaging. We now create the JSONRPCConnection from handles on Windows
which actually should help enable running some of the tests on Windows
as well.
* Replaces two logs since those don't result in response messages anymore
* Adds a func for sending messages synchronously, to get the two messages out before fatalError
* Send `didSave` notifications to clangd if it supports it
- Minor update to the LSP protocol for `didSave` and `colorProvider` to be more in line with the spec
- Only send color requests over to clangd if it supports it
Rationale for this change: clangd recently implemented didSave support,
it is used to rebuild preambles when the user edits a header file
referenced by an open file.
Change-Id: Ie6e2198bdeccb9d1b4083806c254475baadc2d2b
Close the fd via the FileHandle rather than directly on the file
descriptor so that it does not get closed again when the pipe is
released. Fixes running test in parallel on Linux.
Specifically, we care that all outstanding **writes** are finished
before we call the close handler, because otherwise we may (a) send
corrupted output during shutdown, or (b) drop notifications and replies
sent during the shutdown process. The former is a potential issue for
clients that are not robust about parse failures, and the latter is an
issue for reproducibility and robustness during testing/debugging - in
particular, we have some integration tests that send data without
waiting for individual replies and they need to finish outstanding
replies before exiting.
rdar://60159448
Request types should always have the suffix Request and notifications
should end with Notification.
Also moved all request and notification types into separate folders to
reduce the number of files in the LanguageServerProtocol folder.
Since the connection and message handler have a reciprocal need to know
about each other, move the closeHandler so that it has the opportunity
to call a method on the message handler if desired.
According to the LSP specification, arbitrary URIs can be used as
document identifiers. Instead of internally assuming that all URIs are
URLs, use a DocumentURI enum to represent URIs. These can either be file
URLs or other URIs whose value as treated as an opaque string.
We will be able to split the LSP modules off later. These LSP modules
will provide the ability to write custom LSP servers and clients in
Swift. The sourcekit-lsp repository will build on top of this new
package to provide an LSP implementation that creates a language server
for Swift and C-based-languages.
We don't need to check the message buffer in the middle of the test; it
suffices that we got the expectation fulfilled at the right times. The
check at the end of the test can be done by synchronizing via close()
before reading.
These tests don't really need to check the message buffer anyway, it was
mostly a sanity check. There is still a race in testMessageBuffer, which
actually has a reason to look at the buffer and will need to be handled
separately.
