On label conflict, prefer resolved tasks with never templates (#10586)

Kirill Bulatov created

This way, static (and other) templates that change, e.g. env vars, will
get the new template resolved task in the modal to run.

Release Notes:

- N/A

Change summary

crates/project/src/project_tests.rs  | 161 ++++++++++++++++++++++++++++-
crates/project/src/task_inventory.rs |  59 +++++++++-
2 files changed, 200 insertions(+), 20 deletions(-)

Detailed changes

crates/project/src/project_tests.rs 🔗

@@ -14,6 +14,7 @@ use serde_json::json;
 #[cfg(not(windows))]
 use std::os;
 use std::task::Poll;
+use task::{TaskContext, TaskSource, TaskTemplate, TaskTemplates};
 use unindent::Unindent as _;
 use util::{assert_set_eq, paths::PathMatcher, test::temp_tree};
 use worktree::WorktreeModelHandle as _;
@@ -125,8 +126,19 @@ async fn test_managing_project_specific_settings(cx: &mut gpui::TestAppContext)
 
     let project = Project::test(fs.clone(), ["/the-root".as_ref()], cx).await;
     let worktree = project.update(cx, |project, _| project.worktrees().next().unwrap());
+    let task_context = TaskContext::default();
 
     cx.executor().run_until_parked();
+    let workree_id = cx.update(|cx| {
+        project.update(cx, |project, cx| {
+            project.worktrees().next().unwrap().read(cx).id()
+        })
+    });
+    let global_task_source_kind = TaskSourceKind::Worktree {
+        id: workree_id,
+        abs_path: PathBuf::from("/the-root/.zed/tasks.json"),
+        id_base: "local_tasks_for_worktree",
+    };
     cx.update(|cx| {
         let tree = worktree.read(cx);
 
@@ -154,17 +166,29 @@ async fn test_managing_project_specific_settings(cx: &mut gpui::TestAppContext)
         assert_eq!(settings_a.tab_size.get(), 8);
         assert_eq!(settings_b.tab_size.get(), 2);
 
-        let workree_id = project.update(cx, |project, cx| {
-            project.worktrees().next().unwrap().read(cx).id()
-        });
         let all_tasks = project
             .update(cx, |project, cx| {
-                project
-                    .task_inventory()
-                    .update(cx, |inventory, cx| inventory.list_tasks(None, None, cx))
+                project.task_inventory().update(cx, |inventory, cx| {
+                    let (mut old, new) = inventory.used_and_current_resolved_tasks(
+                        None,
+                        Some(workree_id),
+                        &task_context,
+                        cx,
+                    );
+                    old.extend(new);
+                    old
+                })
             })
             .into_iter()
-            .map(|(source_kind, task)| (source_kind, task.label))
+            .map(|(source_kind, task)| {
+                let resolved = task.resolved.unwrap();
+                (
+                    source_kind,
+                    task.resolved_label,
+                    resolved.args,
+                    resolved.env,
+                )
+            })
             .collect::<Vec<_>>();
         assert_eq!(
             all_tasks,
@@ -172,24 +196,141 @@ async fn test_managing_project_specific_settings(cx: &mut gpui::TestAppContext)
                 (
                     TaskSourceKind::Worktree {
                         id: workree_id,
-                        abs_path: PathBuf::from("/the-root/.zed/tasks.json"),
+                        abs_path: PathBuf::from("/the-root/b/.zed/tasks.json"),
                         id_base: "local_tasks_for_worktree",
                     },
-                    "cargo check".to_string()
+                    "cargo check".to_string(),
+                    vec!["check".to_string()],
+                    HashMap::default(),
+                ),
+                (
+                    global_task_source_kind.clone(),
+                    "cargo check".to_string(),
+                    vec!["check".to_string(), "--all".to_string()],
+                    HashMap::default(),
                 ),
+            ]
+        );
+    });
+
+    project.update(cx, |project, cx| {
+        let inventory = project.task_inventory();
+        inventory.update(cx, |inventory, cx| {
+            let (mut old, new) = inventory.used_and_current_resolved_tasks(
+                None,
+                Some(workree_id),
+                &task_context,
+                cx,
+            );
+            old.extend(new);
+            let (_, resolved_task) = old
+                .into_iter()
+                .find(|(source_kind, _)| source_kind == &global_task_source_kind)
+                .expect("should have one global task");
+            inventory.task_scheduled(global_task_source_kind.clone(), resolved_task);
+        })
+    });
+
+    cx.update(|cx| {
+        let all_tasks = project
+            .update(cx, |project, cx| {
+                project.task_inventory().update(cx, |inventory, cx| {
+                    inventory.remove_local_static_source(Path::new("/the-root/.zed/tasks.json"));
+                    inventory.add_source(
+                        global_task_source_kind.clone(),
+                        |cx| {
+                            cx.new_model(|_| {
+                                let source = TestTaskSource {
+                                    tasks: TaskTemplates(vec![TaskTemplate {
+                                        label: "cargo check".to_string(),
+                                        command: "cargo".to_string(),
+                                        args: vec![
+                                            "check".to_string(),
+                                            "--all".to_string(),
+                                            "--all-targets".to_string(),
+                                        ],
+                                        env: HashMap::from_iter(Some((
+                                            "RUSTFLAGS".to_string(),
+                                            "-Zunstable-options".to_string(),
+                                        ))),
+                                        ..TaskTemplate::default()
+                                    }]),
+                                };
+                                Box::new(source) as Box<_>
+                            })
+                        },
+                        cx,
+                    );
+                    let (mut old, new) = inventory.used_and_current_resolved_tasks(
+                        None,
+                        Some(workree_id),
+                        &task_context,
+                        cx,
+                    );
+                    old.extend(new);
+                    old
+                })
+            })
+            .into_iter()
+            .map(|(source_kind, task)| {
+                let resolved = task.resolved.unwrap();
+                (
+                    source_kind,
+                    task.resolved_label,
+                    resolved.args,
+                    resolved.env,
+                )
+            })
+            .collect::<Vec<_>>();
+        assert_eq!(
+            all_tasks,
+            vec![
                 (
                     TaskSourceKind::Worktree {
                         id: workree_id,
                         abs_path: PathBuf::from("/the-root/b/.zed/tasks.json"),
                         id_base: "local_tasks_for_worktree",
                     },
-                    "cargo check".to_string()
+                    "cargo check".to_string(),
+                    vec!["check".to_string()],
+                    HashMap::default(),
+                ),
+                (
+                    TaskSourceKind::Worktree {
+                        id: workree_id,
+                        abs_path: PathBuf::from("/the-root/.zed/tasks.json"),
+                        id_base: "local_tasks_for_worktree",
+                    },
+                    "cargo check".to_string(),
+                    vec![
+                        "check".to_string(),
+                        "--all".to_string(),
+                        "--all-targets".to_string()
+                    ],
+                    HashMap::from_iter(Some((
+                        "RUSTFLAGS".to_string(),
+                        "-Zunstable-options".to_string()
+                    ))),
                 ),
             ]
         );
     });
 }
 
+struct TestTaskSource {
+    tasks: TaskTemplates,
+}
+
+impl TaskSource for TestTaskSource {
+    fn as_any(&mut self) -> &mut dyn std::any::Any {
+        self
+    }
+
+    fn tasks_to_schedule(&mut self, _: &mut ModelContext<Box<dyn TaskSource>>) -> TaskTemplates {
+        self.tasks.clone()
+    }
+}
+
 #[gpui::test]
 async fn test_managing_language_servers(cx: &mut gpui::TestAppContext) {
     init_test(cx);

crates/project/src/task_inventory.rs 🔗

@@ -7,7 +7,7 @@ use std::{
     sync::Arc,
 };
 
-use collections::{HashMap, VecDeque};
+use collections::{hash_map, HashMap, VecDeque};
 use gpui::{AppContext, Context, Model, ModelContext, Subscription};
 use itertools::{Either, Itertools};
 use language::Language;
@@ -229,7 +229,7 @@ impl Inventory {
                 },
             );
         let not_used_score = post_inc(&mut lru_score);
-        let current_resolved_tasks = self
+        let currently_resolved_tasks = self
             .sources
             .iter()
             .filter(|source| {
@@ -257,16 +257,55 @@ impl Inventory {
                 (kind.clone(), task, lru_score)
             })
             .collect::<Vec<_>>();
-        let previous_resolved_tasks = task_usage
+        let previously_spawned_tasks = task_usage
             .into_iter()
             .map(|(_, (kind, task, lru_score))| (kind.clone(), task.clone(), lru_score));
 
-        previous_resolved_tasks
-            .chain(current_resolved_tasks)
+        let mut tasks_by_label = HashMap::default();
+        tasks_by_label = previously_spawned_tasks.into_iter().fold(
+            tasks_by_label,
+            |mut tasks_by_label, (source, task, lru_score)| {
+                match tasks_by_label.entry((source, task.resolved_label.clone())) {
+                    hash_map::Entry::Occupied(mut o) => {
+                        let (_, previous_lru_score) = o.get();
+                        if previous_lru_score >= &lru_score {
+                            o.insert((task, lru_score));
+                        }
+                    }
+                    hash_map::Entry::Vacant(v) => {
+                        v.insert((task, lru_score));
+                    }
+                }
+                tasks_by_label
+            },
+        );
+        tasks_by_label = currently_resolved_tasks.into_iter().fold(
+            tasks_by_label,
+            |mut tasks_by_label, (source, task, lru_score)| {
+                match tasks_by_label.entry((source, task.resolved_label.clone())) {
+                    hash_map::Entry::Occupied(mut o) => {
+                        let (previous_task, _) = o.get();
+                        let new_template = task.original_task();
+                        if new_template.ignore_previously_resolved
+                            || new_template != previous_task.original_task()
+                        {
+                            o.insert((task, lru_score));
+                        }
+                    }
+                    hash_map::Entry::Vacant(v) => {
+                        v.insert((task, lru_score));
+                    }
+                }
+                tasks_by_label
+            },
+        );
+
+        tasks_by_label
+            .into_iter()
+            .map(|((kind, _), (task, lru_score))| (kind, task, lru_score))
             .sorted_unstable_by(task_lru_comparator)
-            .unique_by(|(kind, task, _)| (kind.clone(), task.resolved_label.clone()))
-            .partition_map(|(kind, task, lru_index)| {
-                if lru_index < not_used_score {
+            .partition_map(|(kind, task, lru_score)| {
+                if lru_score < not_used_score {
                     Either::Left((kind, task))
                 } else {
                     Either::Right((kind, task))
@@ -705,7 +744,7 @@ mod tests {
             (
                 TaskSourceKind::AbsPath {
                     id_base: "test source",
-                    abs_path: path_1.to_path_buf(),
+                    abs_path: path_2.to_path_buf(),
                 },
                 common_name.to_string(),
             ),
@@ -719,7 +758,7 @@ mod tests {
             (
                 TaskSourceKind::AbsPath {
                     id_base: "test source",
-                    abs_path: path_2.to_path_buf(),
+                    abs_path: path_1.to_path_buf(),
                 },
                 common_name.to_string(),
             ),