fix catch scope var declarations (#2839)

* use `catch_binding` kind and report errors

* make hash optional

3 files changed, 45 insertions, 30 deletions

diff --git a/src/js_ast.zig b/src/js_ast.zig
index a92e8b2f9..1459d0a37 100644
--- a/src/js_ast.zig
+++ b/src/js_ast.zig
@@ -6564,6 +6564,7 @@ pub const Scope = struct {
         function_args,
         function_body,
         class_static_init,
+        catch_binding,
 
         pub fn jsonStringify(self: @This(), opts: anytype, o: anytype) !void {
             return try std.json.stringify(@tagName(self), opts, o);
diff --git a/src/js_parser.zig b/src/js_parser.zig
index 449029d07..6ba2b7ed1 100644
--- a/src/js_parser.zig
+++ b/src/js_parser.zig
@@ -6603,10 +6603,28 @@ fn NewParser_(
                 }
 
                 // Check for collisions that would prevent to hoisting "var" symbols up to the enclosing function scope
-                if (scope.parent != null) {
+                if (scope.parent) |parent_scope| {
                     nextMember: while (iter.next()) |res| {
                         const value = res.value_ptr.*;
                         var symbol: *Symbol = &symbols[value.ref.innerIndex()];
+
+                        const name = symbol.original_name;
+                        var hash: ?u64 = null;
+
+                        if (parent_scope.kind == .catch_binding and symbol.kind != .hoisted) {
+                            hash = Scope.getMemberHash(name);
+                            if (parent_scope.getMemberWithHash(name, hash.?)) |existing_member| {
+                                p.log.addSymbolAlreadyDeclaredError(
+                                    p.allocator,
+                                    p.source,
+                                    symbol.original_name,
+                                    value.loc,
+                                    existing_member.loc,
+                                ) catch unreachable;
+                                continue :nextMember;
+                            }
+                        }
+
                         if (!symbol.isHoisted()) {
                             continue :nextMember;
                         }
@@ -6652,9 +6670,7 @@ fn NewParser_(
                             is_sloppy_mode_block_level_fn_stmt = true;
                         }
 
-                        const name = symbol.original_name;
-
-                        const hash: u64 = Scope.getMemberHash(name);
+                        if (hash == null) hash = Scope.getMemberHash(name);
 
                         while (__scope) |_scope| {
                             const scope_kind = _scope.kind;
@@ -6673,7 +6689,7 @@ fn NewParser_(
                                 symbol.must_not_be_renamed = true;
                             }
 
-                            if (_scope.getMemberWithHash(name, hash)) |member_in_scope| {
+                            if (_scope.getMemberWithHash(name, hash.?)) |member_in_scope| {
                                 var existing_symbol: *Symbol = &symbols[member_in_scope.ref.innerIndex()];
                                 const existing_kind = existing_symbol.kind;
 
@@ -6690,7 +6706,7 @@ fn NewParser_(
                                 {
                                     // Silently merge this symbol into the existing symbol
                                     symbol.link = member_in_scope.ref;
-                                    var entry = _scope.getOrPutMemberWithHash(p.allocator, name, hash) catch unreachable;
+                                    var entry = _scope.getOrPutMemberWithHash(p.allocator, name, hash.?) catch unreachable;
                                     entry.value_ptr.* = value;
                                     entry.key_ptr.* = name;
                                     continue :nextMember;
@@ -6731,7 +6747,7 @@ fn NewParser_(
                             }
 
                             if (_scope.kindStopsHoisting()) {
-                                var entry = _scope.getOrPutMemberWithHash(allocator, name, hash) catch unreachable;
+                                var entry = _scope.getOrPutMemberWithHash(allocator, name, hash.?) catch unreachable;
                                 entry.value_ptr.* = value;
                                 entry.key_ptr.* = name;
                                 break;
@@ -9131,7 +9147,7 @@ fn NewParser_(
 
                     if (p.lexer.token == .t_catch) {
                         const catch_loc = p.lexer.loc();
-                        _ = try p.pushScopeForParsePass(.block, catch_loc);
+                        _ = try p.pushScopeForParsePass(.catch_binding, catch_loc);
                         try p.lexer.next();
                         var binding: ?js_ast.Binding = null;
 
@@ -11134,27 +11150,7 @@ fn NewParser_(
                 if (comptime !is_generated) {
                     switch (scope.canMergeSymbols(symbol.kind, kind, is_typescript_enabled)) {
                         .forbidden => {
-                            var notes = try p.allocator.alloc(logger.Data, 1);
-                            notes[0] =
-                                logger.rangeData(
-                                p.source,
-                                js_lexer.rangeOfIdentifier(p.source, existing.loc),
-                                std.fmt.allocPrint(
-                                    p.allocator,
-                                    "{s} was originally declared here",
-                                    .{symbol.original_name},
-                                ) catch unreachable,
-                            );
-
-                            p.log.addRangeErrorFmtWithNotes(
-                                p.source,
-                                js_lexer.rangeOfIdentifier(p.source, loc),
-                                p.allocator,
-                                notes,
-                                "\"{s}\" has already been declared",
-                                .{symbol.original_name},
-                            ) catch unreachable;
-
+                            try p.log.addSymbolAlreadyDeclaredError(p.allocator, p.source, symbol.original_name, loc, existing.loc);
                             return existing.ref;
                         },
                         .keep_existing => {
@@ -18227,7 +18223,7 @@ fn NewParser_(
                     p.popScope();
 
                     if (data.catch_) |*catch_| {
-                        p.pushScopeForVisitPass(.block, catch_.loc) catch unreachable;
+                        p.pushScopeForVisitPass(.catch_binding, catch_.loc) catch unreachable;
                         {
                             if (catch_.binding) |catch_binding| {
                                 p.visitBinding(catch_binding, null);
diff --git a/src/logger.zig b/src/logger.zig
index ac8acd7f7..1481eb9b7 100644
--- a/src/logger.zig
+++ b/src/logger.zig
@@ -1131,6 +1131,24 @@ pub const Log = struct {
         try self.addMsg(.{ .kind = .err, .data = rangeData(_source, Range{ .loc = loc }, text) });
     }
 
+    pub fn addSymbolAlreadyDeclaredError(self: *Log, allocator: std.mem.Allocator, source: *const Source, name: string, new_loc: Loc, old_loc: Loc) !void {
+        var notes = try allocator.alloc(Data, 1);
+        notes[0] = rangeData(
+            source,
+            source.rangeOfIdentifier(old_loc),
+            try std.fmt.allocPrint(allocator, "\"{s}\" was originally declared here", .{name}),
+        );
+
+        try self.addRangeErrorFmtWithNotes(
+            source,
+            source.rangeOfIdentifier(new_loc),
+            allocator,
+            notes,
+            "\"{s}\" has already been declared",
+            .{name},
+        );
+    }
+
     pub fn printForLogLevel(self: *Log, to: anytype) !void {
         if (Output.enable_ansi_colors) {
             return self.printForLogLevelWithEnableAnsiColors(to, true);