Do not overly eagerly invalidate the runnables (#51500)

Follow-up of https://github.com/zed-industries/zed/pull/51299

Release Notes:

- N/A

crates/editor/src/editor.rs

@@ -2142,7 +2142,7 @@ impl Editor {
                         editor.registered_buffers.clear();
                         editor.register_visible_buffers(cx);
                         editor.invalidate_semantic_tokens(None);
-                        editor.refresh_runnables(window, cx);
+                        editor.refresh_runnables(None, window, cx);
                         editor.update_lsp_data(None, window, cx);
                         editor.refresh_inlay_hints(InlayHintRefreshReason::ServerRemoved, cx);
                     }
@@ -2172,7 +2172,7 @@ impl Editor {
                         let buffer_id = *buffer_id;
                         if editor.buffer().read(cx).buffer(buffer_id).is_some() {
                             editor.register_buffer(buffer_id, cx);
-                            editor.refresh_runnables(window, cx);
+                            editor.refresh_runnables(Some(buffer_id), window, cx);
                             editor.update_lsp_data(Some(buffer_id), window, cx);
                             editor.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx);
                             refresh_linked_ranges(editor, window, cx);
@@ -2251,7 +2251,7 @@ impl Editor {
                     &task_inventory,
                     window,
                     |editor, _, window, cx| {
-                        editor.refresh_runnables(window, cx);
+                        editor.refresh_runnables(None, window, cx);
                     },
                 ));
             };
@@ -23789,7 +23789,7 @@ impl Editor {
                     .invalidate_buffer(&buffer.read(cx).remote_id());
                 self.update_lsp_data(Some(buffer_id), window, cx);
                 self.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx);
-                self.refresh_runnables(window, cx);
+                self.refresh_runnables(None, window, cx);
                 self.colorize_brackets(false, cx);
                 self.refresh_selected_text_highlights(&self.display_snapshot(cx), true, window, cx);
                 cx.emit(EditorEvent::ExcerptsAdded {
@@ -23850,12 +23850,11 @@ impl Editor {
                 }
                 self.colorize_brackets(false, cx);
                 self.update_lsp_data(None, window, cx);
-                self.refresh_runnables(window, cx);
+                self.refresh_runnables(None, window, cx);
                 cx.emit(EditorEvent::ExcerptsExpanded { ids: ids.clone() })
             }
             multi_buffer::Event::Reparsed(buffer_id) => {
-                self.clear_runnables(Some(*buffer_id));
-                self.refresh_runnables(window, cx);
+                self.refresh_runnables(Some(*buffer_id), window, cx);
                 self.refresh_selected_text_highlights(&self.display_snapshot(cx), true, window, cx);
                 self.colorize_brackets(true, cx);
                 jsx_tag_auto_close::refresh_enabled_in_any_buffer(self, multibuffer, cx);
@@ -23863,7 +23862,7 @@ impl Editor {
                 cx.emit(EditorEvent::Reparsed(*buffer_id));
             }
             multi_buffer::Event::DiffHunksToggled => {
-                self.refresh_runnables(window, cx);
+                self.refresh_runnables(None, window, cx);
             }
             multi_buffer::Event::LanguageChanged(buffer_id, is_fresh_language) => {
                 if !is_fresh_language {
@@ -23999,7 +23998,7 @@ impl Editor {
                 .unwrap_or(DiagnosticSeverity::Hint);
             self.set_max_diagnostics_severity(new_severity, cx);
         }
-        self.refresh_runnables(window, cx);
+        self.refresh_runnables(None, window, cx);
         self.update_edit_prediction_settings(cx);
         self.refresh_edit_prediction(true, false, window, cx);
         self.refresh_inline_values(cx);
@@ -25379,7 +25378,7 @@ impl Editor {
         self.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx);
         if !self.buffer().read(cx).is_singleton() {
             self.update_lsp_data(None, window, cx);
-            self.refresh_runnables(window, cx);
+            self.refresh_runnables(None, window, cx);
         }
     }
 }

crates/editor/src/runnables.rs

@@ -1,7 +1,7 @@
 use std::{collections::BTreeMap, mem, ops::Range, sync::Arc};
 
 use clock::Global;
-use collections::HashMap;
+use collections::{HashMap, HashSet};
 use gpui::{
     App, AppContext as _, AsyncWindowContext, ClickEvent, Context, Entity, Focusable as _,
     MouseButton, Task, Window,
@@ -30,6 +30,7 @@ use crate::{
 #[derive(Debug)]
 pub(super) struct RunnableData {
     runnables: HashMap<BufferId, (Global, BTreeMap<BufferRow, RunnableTasks>)>,
+    invalidate_buffer_data: HashSet<BufferId>,
     runnables_update_task: Task<()>,
 }
 
@@ -37,6 +38,7 @@ impl RunnableData {
     pub fn new() -> Self {
         Self {
             runnables: HashMap::default(),
+            invalidate_buffer_data: HashSet::default(),
             runnables_update_task: Task::ready(()),
         }
     }
@@ -108,7 +110,12 @@ pub struct ResolvedTasks {
 }
 
 impl Editor {
-    pub fn refresh_runnables(&mut self, window: &mut Window, cx: &mut Context<Self>) {
+    pub fn refresh_runnables(
+        &mut self,
+        invalidate_buffer_data: Option<BufferId>,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
         if !self.mode().is_full()
             || !EditorSettings::get_global(cx).gutter.runnables
             || !self.enable_runnables
@@ -117,13 +124,18 @@ impl Editor {
             return;
         }
         if let Some(buffer) = self.buffer().read(cx).as_singleton() {
-            if self
-                .runnables
-                .has_cached(buffer.read(cx).remote_id(), &buffer.read(cx).version())
+            let buffer_id = buffer.read(cx).remote_id();
+            if invalidate_buffer_data != Some(buffer_id)
+                && self
+                    .runnables
+                    .has_cached(buffer_id, &buffer.read(cx).version())
             {
                 return;
             }
         }
+        if let Some(buffer_id) = invalidate_buffer_data {
+            self.runnables.invalidate_buffer_data.insert(buffer_id);
+        }
 
         let project = self.project().map(Entity::downgrade);
         let lsp_task_sources = self.lsp_task_sources(true, true, cx);
@@ -249,6 +261,10 @@ impl Editor {
             .await;
             editor
                 .update(cx, |editor, cx| {
+                    for buffer_id in std::mem::take(&mut editor.runnables.invalidate_buffer_data) {
+                        editor.clear_runnables(Some(buffer_id));
+                    }
+
                     for ((buffer_id, row), mut new_tasks) in rows {
                         let Some(buffer) = editor.buffer().read(cx).buffer(buffer_id) else {
                             continue;
@@ -332,6 +348,7 @@ impl Editor {
         } else {
             self.runnables.runnables.clear();
         }
+        self.runnables.invalidate_buffer_data.clear();
         self.runnables.runnables_update_task = Task::ready(());
     }
 
@@ -697,12 +714,17 @@ impl Editor {
 mod tests {
     use std::{sync::Arc, time::Duration};
 
+    use futures::StreamExt as _;
     use gpui::{AppContext as _, Task, TestAppContext};
     use indoc::indoc;
-    use language::ContextProvider;
+    use language::{ContextProvider, FakeLspAdapter};
     use languages::rust_lang;
+    use lsp::LanguageServerName;
     use multi_buffer::{MultiBuffer, PathKey};
-    use project::{FakeFs, Project};
+    use project::{
+        FakeFs, Project,
+        lsp_store::lsp_ext_command::{CargoRunnableArgs, Runnable, RunnableArgs, RunnableKind},
+    };
     use serde_json::json;
     use task::{TaskTemplate, TaskTemplates};
     use text::Point;
@@ -710,8 +732,11 @@ mod tests {
 
     use crate::{
         Editor, UPDATE_DEBOUNCE, editor_tests::init_test, scroll::scroll_amount::ScrollAmount,
+        test::build_editor_with_project,
     };
 
+    const FAKE_LSP_NAME: &str = "the-fake-language-server";
+
     struct TestRustContextProvider;
 
     impl ContextProvider for TestRustContextProvider {
@@ -739,6 +764,28 @@ mod tests {
         }
     }
 
+    struct TestRustContextProviderWithLsp;
+
+    impl ContextProvider for TestRustContextProviderWithLsp {
+        fn associated_tasks(
+            &self,
+            _: Option<Arc<dyn language::File>>,
+            _: &gpui::App,
+        ) -> Task<Option<TaskTemplates>> {
+            Task::ready(Some(TaskTemplates(vec![TaskTemplate {
+                label: "Run test".into(),
+                command: "cargo".into(),
+                args: vec!["test".into()],
+                tags: vec!["rust-test".into()],
+                ..TaskTemplate::default()
+            }])))
+        }
+
+        fn lsp_task_source(&self) -> Option<LanguageServerName> {
+            Some(LanguageServerName::new_static(FAKE_LSP_NAME))
+        }
+    }
+
     fn rust_lang_with_task_context() -> Arc<language::Language> {
         Arc::new(
             Arc::try_unwrap(rust_lang())
@@ -747,6 +794,14 @@ mod tests {
         )
     }
 
+    fn rust_lang_with_lsp_task_context() -> Arc<language::Language> {
+        Arc::new(
+            Arc::try_unwrap(rust_lang())
+                .unwrap()
+                .with_context_provider(Some(Arc::new(TestRustContextProviderWithLsp))),
+        )
+    }
+
     fn collect_runnable_labels(
         editor: &Editor,
     ) -> Vec<(text::BufferId, language::BufferRow, Vec<String>)> {
@@ -853,7 +908,7 @@ mod tests {
         editor
             .update(cx, |editor, window, cx| {
                 editor.clear_runnables(None);
-                editor.refresh_runnables(window, cx);
+                editor.refresh_runnables(None, window, cx);
             })
             .unwrap();
         cx.executor().advance_clock(UPDATE_DEBOUNCE);
@@ -912,4 +967,127 @@ mod tests {
             "first.rs runnables should survive an edit to second.rs"
         );
     }
+
+    #[gpui::test]
+    async fn test_lsp_runnables_removed_after_edit(cx: &mut TestAppContext) {
+        init_test(cx, |_| {});
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(
+            path!("/project"),
+            json!({
+                "main.rs": indoc! {"
+                    #[test]
+                    fn test_one() {
+                        assert!(true);
+                    }
+
+                    fn helper() {}
+                "},
+            }),
+        )
+        .await;
+
+        let project = Project::test(fs, [path!("/project").as_ref()], cx).await;
+        let language_registry = project.read_with(cx, |project, _| project.languages().clone());
+        language_registry.add(rust_lang_with_lsp_task_context());
+
+        let mut fake_servers = language_registry.register_fake_lsp(
+            "Rust",
+            FakeLspAdapter {
+                name: FAKE_LSP_NAME,
+                ..FakeLspAdapter::default()
+            },
+        );
+
+        let buffer = project
+            .update(cx, |project, cx| {
+                project.open_local_buffer(path!("/project/main.rs"), cx)
+            })
+            .await
+            .unwrap();
+
+        let buffer_id = buffer.read_with(cx, |buffer, _| buffer.remote_id());
+
+        let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer.clone(), cx));
+        let editor = cx.add_window(|window, cx| {
+            build_editor_with_project(project.clone(), multi_buffer, window, cx)
+        });
+
+        let fake_server = fake_servers.next().await.expect("fake LSP server");
+
+        use project::lsp_store::lsp_ext_command::Runnables;
+        fake_server.set_request_handler::<Runnables, _, _>(move |params, _| async move {
+            let text = params.text_document.uri.path().to_string();
+            if text.contains("main.rs") {
+                let uri = lsp::Uri::from_file_path(path!("/project/main.rs")).expect("valid uri");
+                Ok(vec![Runnable {
+                    label: "LSP test_one".into(),
+                    location: Some(lsp::LocationLink {
+                        origin_selection_range: None,
+                        target_uri: uri,
+                        target_range: lsp::Range::new(
+                            lsp::Position::new(0, 0),
+                            lsp::Position::new(3, 1),
+                        ),
+                        target_selection_range: lsp::Range::new(
+                            lsp::Position::new(0, 0),
+                            lsp::Position::new(3, 1),
+                        ),
+                    }),
+                    kind: RunnableKind::Cargo,
+                    args: RunnableArgs::Cargo(CargoRunnableArgs {
+                        environment: Default::default(),
+                        cwd: path!("/project").into(),
+                        override_cargo: None,
+                        workspace_root: None,
+                        cargo_args: vec!["test".into(), "test_one".into()],
+                        executable_args: Vec::new(),
+                    }),
+                }])
+            } else {
+                Ok(Vec::new())
+            }
+        });
+
+        // Trigger a refresh to pick up both tree-sitter and LSP runnables.
+        editor
+            .update(cx, |editor, window, cx| {
+                editor.refresh_runnables(None, window, cx);
+            })
+            .expect("editor update");
+        cx.executor().advance_clock(UPDATE_DEBOUNCE);
+        cx.executor().run_until_parked();
+
+        let labels = editor
+            .update(cx, |editor, _, _| collect_runnable_labels(editor))
+            .expect("editor update");
+        assert_eq!(
+            labels,
+            vec![(buffer_id, 0, vec!["LSP test_one".to_string()]),],
+            "LSP runnables should appear for #[test] fn"
+        );
+
+        // Remove `#[test]` attribute so the function is no longer a test.
+        buffer.update(cx, |buffer, cx| {
+            let test_attr_end = buffer.text().find("\nfn test_one").expect("find fn");
+            buffer.edit([(0..test_attr_end, "")], None, cx);
+        });
+
+        // Also update the LSP handler to return no runnables.
+        fake_server
+            .set_request_handler::<Runnables, _, _>(move |_, _| async move { Ok(Vec::new()) });
+
+        cx.executor().advance_clock(UPDATE_DEBOUNCE);
+        cx.executor().run_until_parked();
+
+        let labels = editor
+            .update(cx, |editor, _, _| collect_runnable_labels(editor))
+            .expect("editor update");
+        assert_eq!(
+            labels,
+            Vec::<(text::BufferId, language::BufferRow, Vec<String>)>::new(),
+            "Runnables should be removed after #[test] is deleted and LSP returns empty"
+        );
+    }
 }