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

Do not error for actions with no data to resolve #19369

Merged
merged 1 commit into from
Mar 15, 2025

Conversation

SomeoneToIgnore
Copy link
Contributor

Same as the other resolve code handlers in the same file.

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#version_3_17_0 does not forbid resolving with no data, so instead of erroring, just consider such items resolved already.

Same as the other resolve code handlers in the same file.
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#version_3_17_0
does not forbid resolving with no data, so instead of erroring, just consider such items resolved already.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 15, 2025
@Veykril Veykril enabled auto-merge March 15, 2025 19:32
@Veykril Veykril added this pull request to the merge queue Mar 15, 2025
Merged via the queue into rust-lang:master with commit e0a293c Mar 15, 2025
9 checks passed
@SomeoneToIgnore SomeoneToIgnore deleted the lenient-action-resolve branch March 15, 2025 20:01
SomeoneToIgnore added a commit to zed-industries/zed that referenced this pull request Mar 15, 2025
…#26848)

Re-applies what's been reverted in
#26832 with an action-related
fix in
64b5d37

Before, actions were resolved only if `data` is present and either of
the possible fields is empty:

https://github.com/zed-industries/zed/blob/e842b4eade368127f01944f1b442016edd6b292d/crates/project/src/lsp_store.rs#L1632-L1633

But Zed resolves completions and inlays once, unconditionally, and the
reverted PR applied the same strategy to actions.
That did not work despite the spec not forbidding `data`-less actions to
be resolved.

Soon, it starts to work due to
rust-lang/rust-analyzer#19369 but it seems safer
to restore the original filtering code.

Code lens have no issues with `data`-less resolves:

https://github.com/rust-lang/rust-analyzer/blob/220d913cbcd05f9b7635179a98c685f31821a572/crates/rust-analyzer/src/handlers/request.rs#L1618-L1620

so the same approach as completions and inlays is kept: resolve once.


Release Notes:

- N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants