[install] fix crash when installing package that uses loose semver pre-release (#4302)

- also fix parsing of `1.2.3pre+build`

fixes #4266

4 files changed, 42 insertions, 34 deletions

diff --git a/src/install/install.zig b/src/install/install.zig
index b0cdf35c8..baa35b869 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -2287,7 +2287,7 @@ pub const PackageManager = struct {
             if (entry.kind != .directory and entry.kind != .sym_link) continue;
             const name = entry.name;
             const sliced = SlicedString.init(name, name);
-            const parsed = Semver.Version.parse(sliced, allocator);
+            const parsed = Semver.Version.parse(sliced);
             if (!parsed.valid or parsed.wildcard != .none) continue;
             // not handling OOM
             // TODO: wildcard
diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig
index 8eb411739..363418976 100644
--- a/src/install/lockfile.zig
+++ b/src/install/lockfile.zig
@@ -2860,7 +2860,7 @@ pub const Package = extern struct {
         };
         if (workspace_json.has_found_version) {
             const version = SlicedString.init(workspace_json.found_version, workspace_json.found_version);
-            const result = Semver.Version.parse(version, allocator);
+            const result = Semver.Version.parse(version);
             if (result.valid and result.wildcard == .none) {
                 entry.version = result.version.fill();
             }
diff --git a/src/install/npm.zig b/src/install/npm.zig
index 0a25fe636..9f3f2952c 100644
--- a/src/install/npm.zig
+++ b/src/install/npm.zig
@@ -846,8 +846,16 @@ pub const PackageManifest = struct {
                 const versions = versions_q.expr.data.e_object.properties.slice();
                 for (versions) |prop| {
                     const version_name = prop.key.?.asString(allocator) orelse continue;
+                    const sliced_version = SlicedString.init(version_name, version_name);
+                    const parsed_version = Semver.Version.parse(sliced_version);
 
-                    if (strings.indexOfChar(version_name, '-') != null) {
+                    if (Environment.allow_assert) std.debug.assert(parsed_version.valid);
+                    if (!parsed_version.valid) {
+                        log.addErrorFmt(&source, prop.value.?.loc, allocator, "Failed to parse dependency {s}", .{version_name}) catch unreachable;
+                        continue;
+                    }
+
+                    if (parsed_version.version.tag.hasPre()) {
                         pre_versions_len += 1;
                         extern_string_count += 1;
                     } else {
@@ -1022,25 +1030,23 @@ pub const PackageManifest = struct {
                 var dependency_names = all_dependency_names_and_values;
                 var prev_extern_bin_group = extern_strings_bin_entries;
 
-                var version_string__: String = String{};
                 for (versions) |prop| {
                     const version_name = prop.key.?.asString(allocator) orelse continue;
+                    var sliced_version = SlicedString.init(version_name, version_name);
+                    var parsed_version = Semver.Version.parse(sliced_version);
 
-                    var sliced_string = SlicedString.init(version_name, version_name);
-
-                    // We only need to copy the version tags if it's a pre/post
-                    if (std.mem.indexOfAny(u8, version_name, "-+") != null) {
-                        version_string__ = string_builder.append(String, version_name);
-                        sliced_string = version_string__.sliced(string_buf);
-                    }
-
-                    const parsed_version = Semver.Version.parse(sliced_string, allocator);
                     if (Environment.allow_assert) std.debug.assert(parsed_version.valid);
-
-                    if (!parsed_version.valid) {
-                        log.addErrorFmt(&source, prop.value.?.loc, allocator, "Failed to parse dependency {s}", .{version_name}) catch unreachable;
-                        continue;
+                    // We only need to copy the version tags if it contains pre and/or build
+                    if (parsed_version.version.tag.hasBuild() or parsed_version.version.tag.hasPre()) {
+                        const version_string = string_builder.append(String, version_name);
+                        sliced_version = version_string.sliced(string_buf);
+                        parsed_version = Semver.Version.parse(sliced_version);
+                        if (Environment.allow_assert) {
+                            std.debug.assert(parsed_version.valid);
+                            std.debug.assert(parsed_version.version.tag.hasBuild() or parsed_version.version.tag.hasPre());
+                        }
                     }
+                    if (!parsed_version.valid) continue;
 
                     var package_version = PackageVersion{};
 
@@ -1451,7 +1457,7 @@ pub const PackageManifest = struct {
 
                         const sliced_string = dist_tag_value_literal.value.sliced(string_buf);
 
-                        dist_tag_versions[dist_tag_i] = Semver.Version.parse(sliced_string, allocator).version.fill();
+                        dist_tag_versions[dist_tag_i] = Semver.Version.parse(sliced_string).version.fill();
                         dist_tag_i += 1;
                     }
                 }
diff --git a/src/install/semver.zig b/src/install/semver.zig
index 1ec76b370..04818cfd3 100644
--- a/src/install/semver.zig
+++ b/src/install/semver.zig
@@ -689,13 +689,13 @@ pub const Version = extern struct {
             const self = formatter.version;
             try std.fmt.format(writer, "{?d}.{?d}.{?d}", .{ self.major, self.minor, self.patch });
 
-            if (!self.tag.pre.isEmpty()) {
+            if (self.tag.hasPre()) {
                 const pre = self.tag.pre.slice(formatter.input);
                 try writer.writeAll("-");
                 try writer.writeAll(pre);
             }
 
-            if (!self.tag.build.isEmpty()) {
+            if (self.tag.hasBuild()) {
                 const build = self.tag.build.slice(formatter.input);
                 try writer.writeAll("+");
                 try writer.writeAll(build);
@@ -814,11 +814,11 @@ pub const Version = extern struct {
         var multi_tag_warn = false;
         // TODO: support multiple tags
 
-        pub fn parse(allocator: Allocator, sliced_string: SlicedString) TagResult {
-            return parseWithPreCount(allocator, sliced_string, 0);
+        pub fn parse(sliced_string: SlicedString) TagResult {
+            return parseWithPreCount(sliced_string, 0);
         }
 
-        pub fn parseWithPreCount(_: Allocator, sliced_string: SlicedString, initial_pre_count: u32) TagResult {
+        pub fn parseWithPreCount(sliced_string: SlicedString, initial_pre_count: u32) TagResult {
             var input = sliced_string.slice;
             var build_count: u32 = 0;
             var pre_count: u32 = initial_pre_count;
@@ -876,7 +876,7 @@ pub const Version = extern struct {
                     },
                     '+' => {
                         // qualifier  ::= ( '-' pre )? ( '+' build )?
-                        if (state == .pre) {
+                        if (state == .pre or state == .none and initial_pre_count > 0) {
                             result.tag.pre = sliced_string.sub(input[start..i]).external();
                             if (comptime Environment.isDebug) {
                                 std.debug.assert(!strings.containsChar(result.tag.pre.slice(sliced_string.buf), '-'));
@@ -901,7 +901,6 @@ pub const Version = extern struct {
             if (state == .none and initial_pre_count > 0) {
                 state = .pre;
                 start = 0;
-                result.tag.pre = sliced_string.sub(input[start..i]).external();
             }
 
             switch (state) {
@@ -932,7 +931,7 @@ pub const Version = extern struct {
         stopped_at: u32 = 0,
     };
 
-    pub fn parse(sliced_string: SlicedString, allocator: Allocator) ParseResult {
+    pub fn parse(sliced_string: SlicedString) ParseResult {
         var input = sliced_string.slice;
         var result = ParseResult{};
 
@@ -1024,7 +1023,7 @@ pub const Version = extern struct {
                     }) {
                         i += 1;
                     }
-                    const tag_result = Tag.parse(allocator, sliced_string.sub(input[part_start_i..]));
+                    const tag_result = Tag.parse(sliced_string.sub(input[part_start_i..]));
                     result.version.tag = tag_result.tag;
                     i += tag_result.len;
                     break;
@@ -1071,9 +1070,12 @@ pub const Version = extern struct {
 
                     // Some weirdo npm packages in the wild have a version like "1.0.0rc.1"
                     // npm just expects that to work...even though it has no "-" qualifier.
-                    if (result.wildcard == .none and part_i >= 2 and ((c >= 'A' and c <= 'Z') or (c >= 'a' and c <= 'z'))) {
+                    if (result.wildcard == .none and part_i >= 2 and switch (c) {
+                        'a'...'z', 'A'...'Z', '_' => true,
+                        else => false,
+                    }) {
                         part_start_i = i;
-                        const tag_result = Tag.parseWithPreCount(allocator, sliced_string.sub(input[part_start_i..]), 1);
+                        const tag_result = Tag.parseWithPreCount(sliced_string.sub(input[part_start_i..]), 1);
                         result.version.tag = tag_result.tag;
                         i += tag_result.len;
                         is_done = true;
@@ -1781,7 +1783,7 @@ pub const Query = struct {
             }
 
             if (!skip_round) {
-                const parse_result = Version.parse(sliced.sub(input[i..]), allocator);
+                const parse_result = Version.parse(sliced.sub(input[i..]));
                 const version = parse_result.version.fill();
                 if (version.tag.hasBuild()) list.flags.setValue(Group.Flags.build, true);
                 if (version.tag.hasPre()) list.flags.setValue(Group.Flags.pre, true);
@@ -1817,7 +1819,7 @@ pub const Query = struct {
                 i += @as(usize, @intFromBool(!hyphenate));
 
                 if (hyphenate) {
-                    const second_parsed = Version.parse(sliced.sub(input[i..]), allocator);
+                    const second_parsed = Version.parse(sliced.sub(input[i..]));
                     var second_version = second_parsed.version.fill();
                     if (second_version.tag.hasBuild()) list.flags.setValue(Group.Flags.build, true);
                     if (second_version.tag.hasPre()) list.flags.setValue(Group.Flags.pre, true);
@@ -1905,7 +1907,7 @@ pub const Query = struct {
 const expect = if (Environment.isTest) struct {
     pub var counter: usize = 0;
     pub fn isRangeMatch(input: string, version_str: string) bool {
-        var parsed = Version.parse(SlicedString.init(version_str, version_str), default_allocator);
+        var parsed = Version.parse(SlicedString.init(version_str, version_str));
         std.debug.assert(parsed.valid);
         // std.debug.assert(strings.eql(parsed.version.raw.slice(version_str), version_str));
 
@@ -1956,7 +1958,7 @@ const expect = if (Environment.isTest) struct {
     pub fn version(input: string, v: [3]?u32, src: std.builtin.SourceLocation) void {
         Output.initTest();
         defer counter += 1;
-        const result = Version.parse(SlicedString.init(input, input), default_allocator);
+        const result = Version.parse(SlicedString.init(input, input));
         std.debug.assert(result.valid);
 
         if (v[0] != result.version.major or v[1] != result.version.minor or v[2] != result.version.patch) {
@@ -1980,7 +1982,7 @@ const expect = if (Environment.isTest) struct {
         Output.initTest();
         defer counter += 1;
 
-        var result = Version.parse(SlicedString.init(input, input), default_allocator);
+        var result = Version.parse(SlicedString.init(input, input));
         if (!v.eql(result.version.fill())) {
             Output.panic("<r><red>Fail<r> Expected version <b>\"{s}\"<r> to match <b>\"{?d}.{?d}.{?d}\" but received <red>\"{?d}.{?d}.{?d}\"<r>\nAt: <blue><b>{s}:{d}:{d}<r><d> in {s}<r>", .{
                 input,