parse dependency specifier correctly (#1840)

2 files changed, 171 insertions, 128 deletions

diff --git a/src/install/dependency.zig b/src/install/dependency.zig
index 44ca91d51..ef78f3ea4 100644
--- a/src/install/dependency.zig
+++ b/src/install/dependency.zig
@@ -303,147 +303,149 @@ pub const Version = struct {
         }
 
         pub fn infer(dependency: string) Tag {
+            // empty string means >= 0.0.0
+            if (dependency.len == 0) return .npm;
             switch (dependency[0]) {
-                // npm package
-                '=', '>', '<', '0'...'9', '^', '*', '|' => return Tag.npm,
-
-                '.' => return Tag.folder,
-
+                // =1
+                // >1.2
+                // >=1.2.3
+                // <1
+                // <=1.2
+                // ^1.2.3
+                // *
+                // || 1.x
+                '=', '>', '<', '^', '*', '|' => return .npm,
+                // ./foo.tgz
+                // ./path/to/foo
+                // ../path/to/bar
+                '.' => {
+                    if (isTarball(dependency)) return .tarball;
+                    return .folder;
+                },
+                // ~1.2.3
+                // ~/foo.tgz
+                // ~/path/to/foo
                 '~' => {
-
                     // https://docs.npmjs.com/cli/v8/configuring-npm/package-json#local-paths
                     if (dependency.len > 1 and dependency[1] == '/') {
-                        return Tag.folder;
-                    }
-
-                    return Tag.npm;
-                },
-
-                'n' => {
-                    if (strings.hasPrefixComptime(dependency, "npm:")) {
-                        return Tag.npm;
-                    }
-                },
-
-                // MIGHT be semver, might not be.
-                'x', 'X' => {
-                    if (dependency.len == 1) {
-                        return Tag.npm;
-                    }
-
-                    if (dependency[1] == '.') {
-                        return Tag.npm;
-                    }
-
-                    return .dist_tag;
-                },
-
-                // git://, git@, git+ssh
-                'g' => {
-                    if (strings.hasPrefixComptime(dependency, "git://") or
-                        strings.hasPrefixComptime(dependency, "git@") or
-                        strings.hasPrefixComptime(dependency, "git+ssh") or
-                        strings.hasPrefixComptime(dependency, "git+file") or
-                        strings.hasPrefixComptime(dependency, "git+http") or
-                        strings.hasPrefixComptime(dependency, "git+https"))
-                    {
-                        return .git;
-                    }
-
-                    if (strings.hasPrefixComptime(dependency, "github:") or isGitHubRepoPath(dependency)) {
-                        return .github;
+                        if (isTarball(dependency)) return .tarball;
+                        return .folder;
                     }
-
-                    return .dist_tag;
+                    return .npm;
                 },
-
+                // /path/to/foo
+                // /path/to/foo.tgz
                 '/' => {
-                    if (isTarball(dependency)) {
-                        return .tarball;
-                    }
-
+                    if (isTarball(dependency)) return .tarball;
                     return .folder;
                 },
-
-                // https://, http://
-                'h' => {
-                    if (isTarball(dependency)) {
-                        return .tarball;
-                    }
-
-                    var remainder = dependency;
-                    if (strings.hasPrefixComptime(remainder, "https://")) {
-                        remainder = remainder["https://".len..];
-                    }
-
-                    if (strings.hasPrefixComptime(remainder, "http://")) {
-                        remainder = remainder["http://".len..];
-                    }
-
-                    if (strings.hasPrefixComptime(remainder, "github.com/") or isGitHubRepoPath(remainder)) {
-                        return .github;
-                    }
-
-                    return .dist_tag;
-                },
-
-                // Dependencies can start with v
-                // v1.0.0 is the same as 1.0.0
-                // However, a github repo or a tarball could start with v
-                'v' => {
-                    if (isTarball(dependency)) {
-                        return .tarball;
-                    }
-
-                    if (isGitHubRepoPath(dependency)) {
-                        return .github;
-                    }
-
+                // 1.2.3
+                // 123.tar.gz
+                '0'...'9' => {
+                    if (isTarball(dependency)) return .tarball;
                     return .npm;
                 },
-
-                // file:
+                // foo.tgz
+                // foo/repo
+                // file:path/to/foo
+                // file:path/to/foo.tar.gz
                 'f' => {
-                    if (isTarball(dependency))
-                        return .tarball;
-
                     if (strings.hasPrefixComptime(dependency, "file:")) {
+                        if (isTarball(dependency)) return .tarball;
                         return .folder;
                     }
-
-                    if (isGitHubRepoPath(dependency)) {
-                        return .github;
-                    }
-
-                    return .dist_tag;
                 },
-
-                // link:
-                'l' => {
-                    if (isTarball(dependency))
-                        return .tarball;
-
-                    if (strings.hasPrefixComptime(dependency, "link:")) {
-                        return .symlink;
+                // git_user/repo
+                // git_tarball.tgz
+                // github:user/repo
+                // git@example.com/repo.git
+                // git://user@example.com/repo.git
+                'g' => {
+                    if (strings.hasPrefixComptime(dependency, "git")) {
+                        const url = dependency["git".len..];
+                        if (url.len > 2) {
+                            switch (url[0]) {
+                                ':' => {
+                                    if (strings.hasPrefixComptime(url, "://")) return .git;
+                                },
+                                '+' => {
+                                    if (strings.hasPrefixComptime(url, "+ssh") or
+                                        strings.hasPrefixComptime(url, "+file") or
+                                        strings.hasPrefixComptime(url, "+http") or
+                                        strings.hasPrefixComptime(url, "+https"))
+                                    {
+                                        return .git;
+                                    }
+                                },
+                                'h' => {
+                                    if (strings.hasPrefixComptime(url, "hub:")) return .github;
+                                },
+                                else => {},
+                            }
+                        }
                     }
-
-                    if (isGitHubRepoPath(dependency)) {
-                        return .github;
+                },
+                // hello/world
+                // hello.tar.gz
+                // https://github.com/user/repo
+                'h' => {
+                    if (strings.hasPrefixComptime(dependency, "http")) {
+                        var url = dependency["http".len..];
+                        if (url.len > 2) {
+                            switch (url[0]) {
+                                ':' => {
+                                    if (strings.hasPrefixComptime(url, "://")) {
+                                        url = url["://".len..];
+                                    }
+                                },
+                                's' => {
+                                    if (strings.hasPrefixComptime(url, "s://")) {
+                                        url = url["s://".len..];
+                                    }
+                                },
+                                else => {},
+                            }
+                            if (strings.hasPrefixComptime(url, "github.com/")) return .github;
+                        }
                     }
-
-                    return .dist_tag;
                 },
-
+                // lisp.tgz
+                // lisp/repo
+                // link:path/to/foo
+                'l' => {
+                    if (strings.hasPrefixComptime(dependency, "link:")) return .symlink;
+                },
+                // newspeak.tgz
+                // newspeak/repo
+                // npm:package@1.2.3
+                'n' => {
+                    if (strings.hasPrefixComptime(dependency, "npm:")) return .npm;
+                },
+                // v1.2.3
+                // verilog.tar.gz
+                // verilog/repo
+                'v' => {
+                    if (isTarball(dependency)) return .tarball;
+                    if (isGitHubRepoPath(dependency)) return .github;
+                    return .npm;
+                },
+                // x
+                // xyz.tar.gz
+                // xyz/repo#main
+                'x', 'X' => {
+                    if (dependency.len == 1) return .npm;
+                    if (dependency[1] == '.') return .npm;
+                },
                 else => {},
             }
 
-            if (isTarball(dependency))
-                return .tarball;
-
-            if (isGitHubRepoPath(dependency)) {
-                return .github;
-            }
-
+            // foo.tgz
+            // bar.tar.gz
+            if (isTarball(dependency)) return .tarball;
+            // user/repo
+            // user/repo#main
+            if (isGitHubRepoPath(dependency)) return .github;
+            // beta
             return .dist_tag;
         }
     };
@@ -506,18 +508,17 @@ pub inline fn parse(
 pub fn parseWithOptionalTag(
     allocator: std.mem.Allocator,
     alias: String,
-    dependency_: string,
-    tag_or_null: ?Dependency.Version.Tag,
+    dependency: string,
+    tag: ?Dependency.Version.Tag,
     sliced: *const SlicedString,
     log: ?*logger.Log,
 ) ?Version {
-    var dependency = std.mem.trimLeft(u8, dependency_, " \t\n\r");
-    if (dependency.len == 0) return null;
+    const dep = std.mem.trimLeft(u8, dependency, " \t\n\r");
     return parseWithTag(
         allocator,
         alias,
-        dependency,
-        tag_or_null orelse Version.Tag.infer(dependency),
+        dep,
+        tag orelse Version.Tag.infer(dep),
         sliced,
         log,
     );
diff --git a/test/bun.js/install/bun-install.test.ts b/test/bun.js/install/bun-install.test.ts
index 980c5ca18..31f4b28d9 100644
--- a/test/bun.js/install/bun-install.test.ts
+++ b/test/bun.js/install/bun-install.test.ts
@@ -22,9 +22,7 @@ async function readdirSorted(path: PathLike): Promise<string[]> {
 }
 
 function resetHanlder() {
-  handler = function () {
-    return new Response("Tea Break~", { status: 418 });
-  };
+  handler = () => new Response("Tea Break~", { status: 418 });
 }
 
 const env = bunEnv;
@@ -138,6 +136,50 @@ it("should handle @scoped authentication", async () => {
   expect(requested).toBe(1);
 });
 
+it("should handle empty string in dependencies", async () => {
+  const urls: string[] = [];
+  handler = async (request) => {
+    expect(request.method).toBe("GET");
+    expect(request.headers.get("accept")).toBe(
+      "application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*",
+    );
+    expect(request.headers.get("npm-auth-type")).toBe(null);
+    expect(await request.text()).toBe("");
+    urls.push(request.url);
+    return new Response("not to be found", { status: 404 });
+  };
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.1",
+      dependencies: {
+        "bar": "",
+      },
+    }),
+  );
+  const { stdout, stderr, exited } = spawn({
+    cmd: [bunExe(), "install", "--config", import.meta.dir + "/basic.toml"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr).toBeDefined();
+  const err = await new Response(stderr).text();
+  expect(err).toContain('error: package "bar" not found localhost/bar 404');
+  expect(err).toContain("error: bar@ failed to resolve");
+  expect(stdout).toBeDefined();
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+ms\]\s*$/, "").split(/\r?\n/)).toEqual([""]);
+  expect(urls).toEqual([
+    "http://localhost:54321/bar",
+  ]);
+  expect(await exited).toBe(1);
+  expect(requested).toBe(1);
+});
+
 it("should handle workspaces", async () => {
   await writeFile(
     join(package_dir, "package.json"),