Properly clear diagnostics data for stopped servers (#63460)
# Objective Diagnostics published by a language server survive the buffer being detached from that server, so a file keeps showing errors from a server that is no longer reading it. To reproduce: 1. Install the [GitHub Actions extension](https://zed.dev/extensions/github-actions). 2. Open a composite action definition, `.github/actions/<name>/action.yml`. 3. Select the GitHub Actions language for it in the language selector. That server validates the file as a workflow, so it reports `on` and `jobs` as missing and `inputs`, `outputs` and `runs` as unexpected. 4. Select YAML again, since GitHub Actions was not what you wanted here. 5. The errors are still there. `editor: restart language servers` does not clear them either. They go away only once the buffer is dropped, in practice by switching to another project and back. Two things keep them alive: - `LocalLspStore::unregister_buffer_from_language_servers` drops the snapshot bookkeeping and sends `didClose`, but never clears what that server already published. `LspStore::set_language_for_buffer` calls it, so a language change detaches the buffer while leaving the old verdict on it. - Restarting does not reach the server either. `stop_local_language_servers_for_buffers` collects servers through `language_server_ids_for_buffer`, evaluated against the language the buffer has *now*. The old server no longer matches, so it is never stopped, and it is typically still running legitimately for other files in the project. Only `stop_local_language_server` clears diagnostics, and it is never reached for that server. ## Solution The first commit cleared the detached server's diagnostics inside `unregister_buffer_from_language_servers`, on `didClose`. Review pointed out that publishes are applied by path regardless of registration, so a running server can legitimately describe a file that is no longer open in it, and that this clearing also left `diagnostic_summaries` and the worktree's per-path diagnostics stale. The second commit reworks this. Detaching a buffer now only drops the buffer's entry in `buffers_opened_in_servers` and the path's pull-diagnostics result ids for the servers it left, two pieces of per-server state that previously leaked. After the buffer is registered with the servers of its new language, the server tree is rebuilt through `refresh_server_tree`, the same pruning that runs on settings changes: every server no registered buffer maps to any more is stopped through `stop_local_language_server`, whose path already clears buffer diagnostic sets, `diagnostic_summaries` and worktree diagnostics. A detached server that other buffers still map to keeps running and keeps everything it published. An earlier revision judged orphanhood by `buffers_opened_in_servers`, which is only filled in once a server runs, so it stopped servers that were still starting; `test_context_completion_provider_mentions` caught that. ## Testing Added `test_selecting_a_language_clears_the_old_servers_diagnostics`: a Rust buffer gets a diagnostic from a fake server, the buffer is switched to JavaScript, and afterwards both the buffer's diagnostics and the project-level `diagnostic_summary` must be empty. It fails on `main`, where the message survives the switch: ```text < ["unused constant"] > [] ``` The test passes with the rework and fails without it. The `project` test suite, `script/clippy` and style checks ran green on the reworked change in CI. Tested on Linux; the change is platform independent. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase The buffer is back on YAML, as the status bar shows, while the workflow schema's complaints from the GitHub Actions server are still on screen: `on` and `jobs` reported missing, and `inputs`, `outputs` and `runs` reported as unexpected values. <img width="2514" height="2008" alt="Screenshot From 2026-08-30 21-52-12" src="https://github.com/user-attachments/assets/040898a6-cc61-41b6-88c6-f346666e8e13" /> --- Release Notes: - Fixed diagnostics from the previous language server staying on a file after changing its language. --------- Co-authored-by: Kirill Bulatov <kirill@zed.dev>
This commit is contained in:
1 parent
bd46f70cfd
commit
cdee8dd547
4 files changed
+521
-60
No files matched your search
@@ -10037,6 +10037,17 @@ impl Editor {
|
||||
if !is_fresh_language {
|
||||
self.registered_buffers.remove(&buffer_id);
|
||||
}
|
||||
// No event exists for a buffer detaching from a language server: a switch
|
||||
// to a language with no server emits neither LanguageServerRemoved nor
|
||||
// LanguageServerBufferRegistered, so the language change itself is the only
|
||||
// signal that this buffer's server set changed. Run the same LSP data sweep
|
||||
// as those events do, or hints and colors from the old server stay visible.
|
||||
if self.project.is_some() {
|
||||
self.register_buffer(*buffer_id, cx);
|
||||
self.invalidate_semantic_tokens(Some(*buffer_id));
|
||||
self.update_lsp_data(Some(*buffer_id), window, cx);
|
||||
self.refresh_inlay_hints(InlayHintRefreshReason::ServerRemoved, cx);
|
||||
}
|
||||
jsx_tag_auto_close::refresh_enabled_in_any_buffer(self, multibuffer, cx);
|
||||
cx.emit(EditorEvent::Reparsed(*buffer_id));
|
||||
self.update_edit_prediction_settings(cx);
|
||||
|
||||
@@ -5034,6 +5034,81 @@ let c = 3;"#
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_hints_cleared_when_language_changes_and_no_server_attaches(
|
||||
cx: &mut gpui::TestAppContext,
|
||||
) {
|
||||
init_test(cx, &|settings| {
|
||||
settings.defaults.inlay_hints = Some(InlayHintSettingsContent {
|
||||
show_value_hints: Some(true),
|
||||
enabled: Some(true),
|
||||
edit_debounce_ms: Some(0),
|
||||
scroll_debounce_ms: Some(0),
|
||||
show_type_hints: Some(true),
|
||||
show_parameter_hints: Some(true),
|
||||
show_other_hints: Some(true),
|
||||
show_background: Some(false),
|
||||
toggle_on_modifiers_press: None,
|
||||
})
|
||||
});
|
||||
|
||||
let (_, editor, _fake_server) = prepare_test_objects(cx, |fake_server, file_with_hints| {
|
||||
fake_server.set_request_handler::<lsp::request::InlayHintRequest, _, _>(
|
||||
move |params, _| async move {
|
||||
assert_eq!(
|
||||
params.text_document.uri,
|
||||
lsp::Uri::from_file_path(file_with_hints).unwrap(),
|
||||
);
|
||||
Ok(Some(vec![lsp::InlayHint {
|
||||
position: lsp::Position::new(0, 1),
|
||||
label: lsp::InlayHintLabel::String("stale hint".to_string()),
|
||||
kind: None,
|
||||
text_edits: None,
|
||||
tooltip: None,
|
||||
padding_left: None,
|
||||
padding_right: None,
|
||||
data: None,
|
||||
}]))
|
||||
},
|
||||
);
|
||||
})
|
||||
.await;
|
||||
|
||||
cx.executor().run_until_parked();
|
||||
editor
|
||||
.update(cx, |editor, _, cx| {
|
||||
let expected_hints = vec!["stale hint".to_string()];
|
||||
assert_eq!(expected_hints, cached_hint_labels(editor, cx));
|
||||
assert_eq!(expected_hints, visible_hint_labels(editor, cx));
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
let plain_text = Arc::new(Language::new(
|
||||
LanguageConfig {
|
||||
name: "Plain Text".into(),
|
||||
..LanguageConfig::default()
|
||||
},
|
||||
None,
|
||||
));
|
||||
editor
|
||||
.update(cx, |editor, _, cx| {
|
||||
let project = editor.project().unwrap().clone();
|
||||
let buffer = editor.buffer().read(cx).as_singleton().unwrap();
|
||||
project.update(cx, |project, cx| {
|
||||
project.set_language_for_buffer(&buffer, plain_text, cx);
|
||||
});
|
||||
})
|
||||
.unwrap();
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
editor
|
||||
.update(cx, |editor, _, cx| {
|
||||
assert_eq!(Vec::<String>::new(), cached_hint_labels(editor, cx));
|
||||
assert_eq!(Vec::<String>::new(), visible_hint_labels(editor, cx));
|
||||
})
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
pub(crate) fn init_test(cx: &mut TestAppContext, f: &dyn Fn(&mut AllLanguageSettingsContent)) {
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
|
||||
Reference in new issue
Block a user