Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support null and negative integer request IDs #285

Merged
merged 2 commits into from
Jun 12, 2021

Conversation

ebkalderon
Copy link
Owner

Added

  • Add a new Id::Null enum variant.
  • Implement Default for Id.
  • Add unit tests to ensure null and negative integer request IDs parse successfully.

Changed

  • Change Id::Number(u64) to Id::Number(i64).
  • Change Response::error() constructor to accept Id instead of Option<Id>.
  • Change Response::id() method to return &Id instead of Option<&Id>.
  • Change Response::into_parts() method to return (Id, _) instead of (Option<Id>, _)
  • Use std::sync::atomic::AtomicU32 to track client request IDs instead of AtomicU64. This ensures the incrementing IDs will always cast losslessly into an i64 primitive data type.

Both of these request ID types are rare and bizarre and their use is generally discouraged by the JSON-RPC 2.0 spec, but are nonetheless permitted by the standard. Since we wish tower-lsp to be compatible with as many JSON-RPC client libraries as possible, we must support parsing null and negative integer request IDs, even if we never emit them ourselves.

@ebkalderon ebkalderon self-assigned this Jun 12, 2021
@ebkalderon ebkalderon force-pushed the support-null-negative-request-ids branch 10 times, most recently from a24ab95 to ed71395 Compare June 12, 2021 20:15
Despite the Language Server Protocol specification claiming otherwise
[here], the `id` field of a request object _can_ be `null` according to
the JSON-RPC 2.0 specification:

[here]: https://microsoft.github.io/language-server-protocol/specification#requestMessage

Although the use of `null` as a request ID value is rare in practice and
explicitly discouraged by the JSON-RPC 2.0 specification, we must be
compatible with as many JSON-RPC client implementations as possible.
Therefore, we must support parsing `null` request IDs in incoming
messages, even if we never emit them ourselves.
Although a rare occurrence, the use of negative integer values as
request IDs is nonetheless permitted by the JSON-RPC 2.0 specification.
To ensure compatibility with as many JSON-RPC client implementations as
possible, we must support parsing negative IDs, even though we never
emit them ourselves.
@ebkalderon ebkalderon force-pushed the support-null-negative-request-ids branch from ed71395 to 77a871e Compare June 12, 2021 22:10
@ebkalderon ebkalderon merged commit 03daa16 into master Jun 12, 2021
@ebkalderon ebkalderon deleted the support-null-negative-request-ids branch June 12, 2021 22:37
ebkalderon added a commit that referenced this pull request Feb 14, 2022
The `tower-lsp-macros` crate should have been released with a minor
version bump, not a patch version bump. This commit updates the versions
appropriately in the `Cargo.toml` manifests in preparation for yanking
and re-releasing the same code under the new designations `tower-lsp`
0.15.1 and `tower-lsp-macros` 0.5.0.

This is to address a semver incompatibility introduced in PR #285.
ebkalderon added a commit that referenced this pull request Feb 14, 2022
The `tower-lsp-macros` crate should have been released with a minor
version bump, not a patch version bump. This commit updates the versions
appropriately in the `Cargo.toml` manifests in preparation for yanking
and re-releasing the same code under the new designations `tower-lsp`
0.15.1 and `tower-lsp-macros` 0.5.0.

This is to address a semver incompatibility introduced in PR #285.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant