From 4fab3697a226cea23f5bbd6531d8b3beb33cdf2a Mon Sep 17 00:00:00 2001
From: Satya Rohith <me@satyarohith.com>
Date: Tue, 5 Nov 2024 01:13:34 +0530
Subject: [PATCH] fix: don't panic for invalid package name (#133)

Removes a panic and adds a new diagnostic for invalid workspace
member name.

Towards https://github.com/denoland/deno/issues/26334

Co-authored-by: David Sherret <dsherret@users.noreply.github.com>
---
 Cargo.lock                |  8 ++---
 src/workspace/mod.rs      | 21 ++++++++++++
 src/workspace/resolver.rs | 68 +++++++++++++++++++++++++++++++--------
 3 files changed, 79 insertions(+), 18 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 976b3fc..b59a9ae 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -398,9 +398,9 @@ dependencies = [
 
 [[package]]
 name = "regex-automata"
-version = "0.4.7"
+version = "0.4.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "38caf58cc5ef2fed281f89292ef23f6365465ed9a41b7a7754eb4e26496c92df"
+checksum = "368758f23274712b504848e9d5a6f010445cc8b87a7cdb4d7cbee666c1288da3"
 dependencies = [
  "aho-corasick",
  "memchr",
@@ -409,9 +409,9 @@ dependencies = [
 
 [[package]]
 name = "regex-syntax"
-version = "0.8.4"
+version = "0.8.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7a66a03ae7c801facd77a29370b4faec201768915ac14a721ba36f20bc9c209b"
+checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c"
 
 [[package]]
 name = "rustix"
diff --git a/src/workspace/mod.rs b/src/workspace/mod.rs
index 1dbbae0..f96f545 100644
--- a/src/workspace/mod.rs
+++ b/src/workspace/mod.rs
@@ -141,6 +141,8 @@ pub enum WorkspaceDiagnosticKind {
     previous: bool,
     suggestion: NodeModulesDirMode,
   },
+  #[error("Invalid workspace member name \"{name}\". Ensure the name is in the format '@scope/name'.")]
+  InvalidMemberName { name: String },
 }
 
 #[derive(Debug, Error, Clone, PartialEq, Eq)]
@@ -675,6 +677,14 @@ impl Workspace {
       config: &ConfigFile,
       diagnostics: &mut Vec<WorkspaceDiagnostic>,
     ) {
+      if let Some(name) = &config.json.name {
+        if !is_valid_jsr_pkg_name(name) {
+          diagnostics.push(WorkspaceDiagnostic {
+            config_url: config.specifier.clone(),
+            kind: WorkspaceDiagnosticKind::InvalidMemberName { name: name.clone() },
+          });
+        }
+      }
       if config.json.deprecated_workspaces.is_some() {
         diagnostics.push(WorkspaceDiagnostic {
           config_url: config.specifier.clone(),
@@ -1928,6 +1938,17 @@ fn parent_specifier_str(specifier: &str) -> Option<&str> {
   }
 }
 
+fn is_valid_jsr_pkg_name(name: &str) -> bool {
+  let jsr = deno_semver::jsr::JsrPackageReqReference::from_str(&format!(
+    "jsr:{}@*",
+    name
+  ));
+  match jsr {
+    Ok(jsr) => jsr.sub_path().is_none(),
+    Err(_) => false,
+  }
+}
+
 #[cfg(test)]
 mod test {
   use std::cell::RefCell;
diff --git a/src/workspace/resolver.rs b/src/workspace/resolver.rs
index e82f298..7ce7ae9 100644
--- a/src/workspace/resolver.rs
+++ b/src/workspace/resolver.rs
@@ -549,20 +549,24 @@ impl WorkspaceResolver {
         if let Some(path) = specifier.strip_prefix(&member.name) {
           if path.is_empty() || path.starts_with('/') {
             let path = path.strip_prefix('/').unwrap_or(path);
-            return self.resolve_workspace_jsr_pkg(
-              member,
-              JsrPackageReqReference::from_str(&format!(
-                "jsr:{}{}/{}",
-                member.name,
-                member
-                  .version
-                  .as_ref()
-                  .map(|v| format!("@^{}", v))
-                  .unwrap_or_else(String::new),
-                path
-              ))
-              .unwrap(),
-            );
+            let pkg_req_ref = match JsrPackageReqReference::from_str(&format!(
+              "jsr:{}{}/{}",
+              member.name,
+              member
+                .version
+                .as_ref()
+                .map(|v| format!("@^{}", v))
+                .unwrap_or_else(String::new),
+              path
+            )) {
+              Ok(pkg_req_ref) => pkg_req_ref,
+              Err(_) => {
+                // Ignore the error as it will be surfaced as a diagnostic
+                // in workspace.diagnostics() routine.
+                continue;
+              }
+            };
+            return self.resolve_workspace_jsr_pkg(member, pkg_req_ref);
           }
         }
       }
@@ -1493,6 +1497,42 @@ mod test {
     }
   }
 
+  #[test]
+  fn invalid_package_name_with_slashes() {
+    let mut fs = TestFileSystem::default();
+    fs.insert_json(
+      root_dir().join("deno.json"),
+      json!({
+        "workspace": ["./libs/math"]
+      }),
+    );
+    fs.insert_json(
+      root_dir().join("libs/math/deno.json"),
+      json!({
+        "name": "@deno-test/libs/math", // Invalid package name containing slashes
+        "version": "1.0.0",
+        "exports": "./mod.ts"
+      }),
+    );
+    let workspace = workspace_at_start_dir(&fs, &root_dir());
+    let resolver = create_resolver(&workspace);
+    let result = resolver.resolve(
+      "@deno-test/libs/math",
+      &url_from_file_path(&root_dir().join("main.ts")).unwrap(),
+    );
+    // Resolve shouldn't panic and tt should result in unmapped
+    // bare specifier error as the package name is invalid.
+    assert!(result.err().unwrap().is_unmapped_bare_specifier());
+
+    let diagnostics = workspace.workspace.diagnostics();
+    assert_eq!(diagnostics.len(), 1);
+    assert!(diagnostics
+      .first()
+      .unwrap()
+      .to_string()
+      .starts_with(r#"Invalid workspace member name "@deno-test/libs/math"."#));
+  }
+
   fn create_resolver(workspace_dir: &WorkspaceDirectory) -> WorkspaceResolver {
     workspace_dir
       .workspace