debugger: Fix issues with debugging scripts from package.json (#32995)

Cole Miller created

- [x] Pass in cwd
- [x] Use the appropriate package manager
- [x] Don't mix up package.json and composer.json

Release Notes:

- debugger: Fixed wrong arguments being passed to the DAP when debugging
scripts from package.json.

Change summary

crates/languages/src/json.rs                 | 115 ++++++++++++---------
crates/languages/src/lib.rs                  |   3 
crates/languages/src/package_json.rs         | 106 ++++++++++++++++++++
crates/languages/src/typescript.rs           | 108 -------------------
crates/project/src/debugger/locators/node.rs |   3 
5 files changed, 182 insertions(+), 153 deletions(-)

Detailed changes

crates/languages/src/json.rs 🔗

@@ -31,6 +31,8 @@ use std::{
 use task::{AdapterSchemas, TaskTemplate, TaskTemplates, VariableName};
 use util::{ResultExt, archive::extract_zip, fs::remove_matching, maybe, merge_json_value_into};
 
+use crate::PackageJsonData;
+
 const SERVER_PATH: &str =
     "node_modules/vscode-langservers-extracted/bin/vscode-json-language-server";
 
@@ -47,12 +49,14 @@ impl ContextProvider for JsonTaskProvider {
         file: Option<Arc<dyn language::File>>,
         cx: &App,
     ) -> gpui::Task<Option<TaskTemplates>> {
-        let Some(file) = project::File::from_dyn(file.as_ref())
-            .filter(|file| file.path.file_name() == Some("package.json".as_ref()))
-            .cloned()
-        else {
+        let Some(file) = project::File::from_dyn(file.as_ref()).cloned() else {
             return Task::ready(None);
         };
+        let is_package_json = file.path.ends_with("package.json");
+        let is_composer_json = file.path.ends_with("composer.json");
+        if !is_package_json && !is_composer_json {
+            return Task::ready(None);
+        }
 
         cx.spawn(async move |cx| {
             let contents = file
@@ -62,53 +66,68 @@ impl ContextProvider for JsonTaskProvider {
                 .await
                 .ok()?;
 
-            let as_json = serde_json_lenient::Value::from_str(&contents.text).ok()?;
-            let gutter_tasks = [
-                TaskTemplate {
-                    label: "package script $ZED_CUSTOM_script".to_owned(),
-                    command: "npm".to_owned(),
-                    args: vec![
-                        "--prefix".into(),
-                        "$ZED_DIRNAME".into(),
-                        "run".into(),
-                        VariableName::Custom("script".into()).template_value(),
-                    ],
-                    tags: vec!["package-script".into()],
-                    ..TaskTemplate::default()
-                },
-                TaskTemplate {
-                    label: "composer script $ZED_CUSTOM_script".to_owned(),
-                    command: "composer".to_owned(),
-                    args: vec![
-                        "-d".into(),
-                        "$ZED_DIRNAME".into(),
-                        VariableName::Custom("script".into()).template_value(),
-                    ],
-                    tags: vec!["composer-script".into()],
-                    ..TaskTemplate::default()
-                },
-            ];
-            let tasks = as_json
-                .get("scripts")?
-                .as_object()?
-                .keys()
-                .map(|key| TaskTemplate {
-                    label: format!("run {key}"),
-                    command: "npm".to_owned(),
-                    args: vec![
-                        "--prefix".into(),
-                        "$ZED_DIRNAME".into(),
-                        "run".into(),
-                        key.into(),
-                    ],
-                    ..TaskTemplate::default()
-                })
-                .chain(gutter_tasks)
-                .collect();
-            Some(TaskTemplates(tasks))
+            let task_templates = if is_package_json {
+                let package_json = serde_json_lenient::from_str::<
+                    HashMap<String, serde_json_lenient::Value>,
+                >(&contents.text)
+                .ok()?;
+                let package_json = PackageJsonData::new(file.path.clone(), package_json);
+                let command = package_json.package_manager.unwrap_or("npm").to_owned();
+                package_json
+                    .scripts
+                    .into_iter()
+                    .map(|(_, key)| TaskTemplate {
+                        label: format!("run {key}"),
+                        command: command.clone(),
+                        args: vec!["run".into(), key],
+                        cwd: Some(VariableName::Dirname.template_value()),
+                        ..TaskTemplate::default()
+                    })
+                    .chain([TaskTemplate {
+                        label: "package script $ZED_CUSTOM_script".to_owned(),
+                        command: command.clone(),
+                        args: vec![
+                            "run".into(),
+                            VariableName::Custom("script".into()).template_value(),
+                        ],
+                        cwd: Some(VariableName::Dirname.template_value()),
+                        tags: vec!["package-script".into()],
+                        ..TaskTemplate::default()
+                    }])
+                    .collect()
+            } else if is_composer_json {
+                serde_json_lenient::Value::from_str(&contents.text)
+                    .ok()?
+                    .get("scripts")?
+                    .as_object()?
+                    .keys()
+                    .map(|key| TaskTemplate {
+                        label: format!("run {key}"),
+                        command: "composer".to_owned(),
+                        args: vec!["-d".into(), "$ZED_DIRNAME".into(), key.into()],
+                        ..TaskTemplate::default()
+                    })
+                    .chain([TaskTemplate {
+                        label: "composer script $ZED_CUSTOM_script".to_owned(),
+                        command: "composer".to_owned(),
+                        args: vec![
+                            "-d".into(),
+                            "$ZED_DIRNAME".into(),
+                            VariableName::Custom("script".into()).template_value(),
+                        ],
+                        tags: vec!["composer-script".into()],
+                        ..TaskTemplate::default()
+                    }])
+                    .collect()
+            } else {
+                vec![]
+            };
+
+            Some(TaskTemplates(task_templates))
         })
     }
 }
+
 fn server_binary_arguments(server_path: &Path) -> Vec<OsString> {
     vec![server_path.into(), "--stdio".into()]
 }

crates/languages/src/lib.rs 🔗

@@ -18,6 +18,7 @@ mod c;
 mod css;
 mod go;
 mod json;
+mod package_json;
 mod python;
 mod rust;
 mod tailwind;
@@ -25,6 +26,8 @@ mod typescript;
 mod vtsls;
 mod yaml;
 
+pub(crate) use package_json::{PackageJson, PackageJsonData};
+
 #[derive(RustEmbed)]
 #[folder = "src/"]
 #[exclude = "*.rs"]

crates/languages/src/package_json.rs 🔗

@@ -0,0 +1,106 @@
+use chrono::{DateTime, Local};
+use collections::{BTreeSet, HashMap};
+use serde_json_lenient::Value;
+use std::{path::Path, sync::Arc};
+
+#[derive(Clone, Debug)]
+pub struct PackageJson {
+    pub mtime: DateTime<Local>,
+    pub data: PackageJsonData,
+}
+
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
+pub struct PackageJsonData {
+    pub jest_package_path: Option<Arc<Path>>,
+    pub mocha_package_path: Option<Arc<Path>>,
+    pub vitest_package_path: Option<Arc<Path>>,
+    pub jasmine_package_path: Option<Arc<Path>>,
+    pub scripts: BTreeSet<(Arc<Path>, String)>,
+    pub package_manager: Option<&'static str>,
+}
+
+impl PackageJsonData {
+    pub fn new(path: Arc<Path>, package_json: HashMap<String, Value>) -> Self {
+        let mut scripts = BTreeSet::new();
+        if let Some(Value::Object(package_json_scripts)) = package_json.get("scripts") {
+            scripts.extend(
+                package_json_scripts
+                    .keys()
+                    .cloned()
+                    .map(|name| (path.clone(), name)),
+            );
+        }
+
+        let mut jest_package_path = None;
+        let mut mocha_package_path = None;
+        let mut vitest_package_path = None;
+        let mut jasmine_package_path = None;
+        if let Some(Value::Object(dependencies)) = package_json.get("devDependencies") {
+            if dependencies.contains_key("jest") {
+                jest_package_path.get_or_insert_with(|| path.clone());
+            }
+            if dependencies.contains_key("mocha") {
+                mocha_package_path.get_or_insert_with(|| path.clone());
+            }
+            if dependencies.contains_key("vitest") {
+                vitest_package_path.get_or_insert_with(|| path.clone());
+            }
+            if dependencies.contains_key("jasmine") {
+                jasmine_package_path.get_or_insert_with(|| path.clone());
+            }
+        }
+        if let Some(Value::Object(dev_dependencies)) = package_json.get("dependencies") {
+            if dev_dependencies.contains_key("jest") {
+                jest_package_path.get_or_insert_with(|| path.clone());
+            }
+            if dev_dependencies.contains_key("mocha") {
+                mocha_package_path.get_or_insert_with(|| path.clone());
+            }
+            if dev_dependencies.contains_key("vitest") {
+                vitest_package_path.get_or_insert_with(|| path.clone());
+            }
+            if dev_dependencies.contains_key("jasmine") {
+                jasmine_package_path.get_or_insert_with(|| path.clone());
+            }
+        }
+
+        let package_manager = package_json
+            .get("packageManager")
+            .and_then(|value| value.as_str())
+            .and_then(|value| {
+                if value.starts_with("pnpm") {
+                    Some("pnpm")
+                } else if value.starts_with("yarn") {
+                    Some("yarn")
+                } else if value.starts_with("npm") {
+                    Some("npm")
+                } else {
+                    None
+                }
+            });
+
+        Self {
+            jest_package_path,
+            mocha_package_path,
+            vitest_package_path,
+            jasmine_package_path,
+            scripts,
+            package_manager,
+        }
+    }
+
+    pub fn merge(&mut self, other: Self) {
+        self.jest_package_path = self.jest_package_path.take().or(other.jest_package_path);
+        self.mocha_package_path = self.mocha_package_path.take().or(other.mocha_package_path);
+        self.vitest_package_path = self
+            .vitest_package_path
+            .take()
+            .or(other.vitest_package_path);
+        self.jasmine_package_path = self
+            .jasmine_package_path
+            .take()
+            .or(other.jasmine_package_path);
+        self.scripts.extend(other.scripts);
+        self.package_manager = self.package_manager.or(other.package_manager);
+    }
+}

crates/languages/src/typescript.rs 🔗

@@ -18,7 +18,6 @@ use smol::{fs, io::BufReader, lock::RwLock, stream::StreamExt};
 use std::{
     any::Any,
     borrow::Cow,
-    collections::BTreeSet,
     ffi::OsString,
     path::{Path, PathBuf},
     sync::Arc,
@@ -28,6 +27,8 @@ use util::archive::extract_zip;
 use util::merge_json_value_into;
 use util::{ResultExt, fs::remove_matching, maybe};
 
+use crate::{PackageJson, PackageJsonData};
+
 #[derive(Debug)]
 pub(crate) struct TypeScriptContextProvider {
     last_package_json: PackageJsonContents,
@@ -57,108 +58,7 @@ const TYPESCRIPT_JASMINE_PACKAGE_PATH_VARIABLE: VariableName =
 #[derive(Clone, Debug, Default)]
 struct PackageJsonContents(Arc<RwLock<HashMap<PathBuf, PackageJson>>>);
 
-#[derive(Clone, Debug)]
-struct PackageJson {
-    mtime: DateTime<Local>,
-    data: PackageJsonData,
-}
-
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
-struct PackageJsonData {
-    jest_package_path: Option<Arc<Path>>,
-    mocha_package_path: Option<Arc<Path>>,
-    vitest_package_path: Option<Arc<Path>>,
-    jasmine_package_path: Option<Arc<Path>>,
-    scripts: BTreeSet<(Arc<Path>, String)>,
-    package_manager: Option<&'static str>,
-}
-
 impl PackageJsonData {
-    fn new(path: Arc<Path>, package_json: HashMap<String, Value>) -> Self {
-        let mut scripts = BTreeSet::new();
-        if let Some(serde_json::Value::Object(package_json_scripts)) = package_json.get("scripts") {
-            scripts.extend(
-                package_json_scripts
-                    .keys()
-                    .cloned()
-                    .map(|name| (path.clone(), name)),
-            );
-        }
-
-        let mut jest_package_path = None;
-        let mut mocha_package_path = None;
-        let mut vitest_package_path = None;
-        let mut jasmine_package_path = None;
-        if let Some(serde_json::Value::Object(dependencies)) = package_json.get("devDependencies") {
-            if dependencies.contains_key("jest") {
-                jest_package_path.get_or_insert_with(|| path.clone());
-            }
-            if dependencies.contains_key("mocha") {
-                mocha_package_path.get_or_insert_with(|| path.clone());
-            }
-            if dependencies.contains_key("vitest") {
-                vitest_package_path.get_or_insert_with(|| path.clone());
-            }
-            if dependencies.contains_key("jasmine") {
-                jasmine_package_path.get_or_insert_with(|| path.clone());
-            }
-        }
-        if let Some(serde_json::Value::Object(dev_dependencies)) = package_json.get("dependencies")
-        {
-            if dev_dependencies.contains_key("jest") {
-                jest_package_path.get_or_insert_with(|| path.clone());
-            }
-            if dev_dependencies.contains_key("mocha") {
-                mocha_package_path.get_or_insert_with(|| path.clone());
-            }
-            if dev_dependencies.contains_key("vitest") {
-                vitest_package_path.get_or_insert_with(|| path.clone());
-            }
-            if dev_dependencies.contains_key("jasmine") {
-                jasmine_package_path.get_or_insert_with(|| path.clone());
-            }
-        }
-
-        let package_manager = package_json
-            .get("packageManager")
-            .and_then(|value| value.as_str())
-            .and_then(|value| {
-                if value.starts_with("pnpm") {
-                    Some("pnpm")
-                } else if value.starts_with("yarn") {
-                    Some("yarn")
-                } else if value.starts_with("npm") {
-                    Some("npm")
-                } else {
-                    None
-                }
-            });
-
-        Self {
-            jest_package_path,
-            mocha_package_path,
-            vitest_package_path,
-            jasmine_package_path,
-            scripts,
-            package_manager,
-        }
-    }
-
-    fn merge(&mut self, other: Self) {
-        self.jest_package_path = self.jest_package_path.take().or(other.jest_package_path);
-        self.mocha_package_path = self.mocha_package_path.take().or(other.mocha_package_path);
-        self.vitest_package_path = self
-            .vitest_package_path
-            .take()
-            .or(other.vitest_package_path);
-        self.jasmine_package_path = self
-            .jasmine_package_path
-            .take()
-            .or(other.jasmine_package_path);
-        self.scripts.extend(other.scripts);
-        self.package_manager = self.package_manager.or(other.package_manager);
-    }
-
     fn fill_task_templates(&self, task_templates: &mut TaskTemplates) {
         if self.jest_package_path.is_some() {
             task_templates.0.push(TaskTemplate {
@@ -400,8 +300,8 @@ impl TypeScriptContextProvider {
                         fs.load(&package_json_path).await.with_context(|| {
                             format!("loading package.json from {package_json_path:?}")
                         })?;
-                    let package_json: HashMap<String, serde_json::Value> =
-                        serde_json::from_str(&package_json_string).with_context(|| {
+                    let package_json: HashMap<String, serde_json_lenient::Value> =
+                        serde_json_lenient::from_str(&package_json_string).with_context(|| {
                             format!("parsing package.json from {package_json_path:?}")
                         })?;
                     let new_data =

crates/project/src/debugger/locators/node.rs 🔗

@@ -29,8 +29,9 @@ impl DapLocator for NodeLocator {
             return None;
         }
         if build_config.command != TYPESCRIPT_RUNNER_VARIABLE.template_value()
-            && build_config.command != "composer"
             && build_config.command != "npm"
+            && build_config.command != "pnpm"
+            && build_config.command != "yarn"
         {
             return None;
         }