fix: don't panic for invalid package name (#133)

Removes a panic and adds a new diagnostic for invalid workspace member name. Towards denoland/deno#26334 Co-authored-by: David Sherret <[email protected]>
denoland · Nov 4, 2024 · 4fab369 · 4fab369
1 parent 5ab733a
commit 4fab369
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 18 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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