Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/ast/G.zig
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub const Class = struct {
return false;
}

if (property.kind == .normal) {
if (property.kind == .normal or property.kind == .auto_accessor) {
if (flags.contains(.is_static)) {
for ([2]?Expr{ property.value, property.initializer }) |val_| {
if (val_) |val| {
Expand Down Expand Up @@ -134,6 +134,7 @@ pub const Property = struct {
declare,
abstract,
class_static_block,
auto_accessor,

pub fn jsonStringify(self: @This(), writer: anytype) !void {
return try writer.write(@tagName(self));
Expand Down
170 changes: 166 additions & 4 deletions src/ast/P.zig
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,10 @@ pub fn NewParser_(
temp_refs_to_declare: List(TempRef) = .{},
temp_ref_count: i32 = 0,

// Temp vars for auto-accessor computed keys, collected during visitClass
// and emitted as var declarations by the caller.
auto_accessor_computed_key_refs: List(LocRef) = .{},

// When bundling, hoisted top-level local variables declared with "var" in
// nested scopes are moved up to be declared in the top-level scope instead.
// The old "var" statements are turned into regular assignments instead. This
Expand Down Expand Up @@ -4851,6 +4855,164 @@ pub fn NewParser_(
stmts.append(closure) catch unreachable;
}

pub fn lowerAutoAccessors(p: *P, properties: []G.Property) []G.Property {
var new_props = ListManaged(G.Property).init(p.allocator);
new_props.ensureTotalCapacity(properties.len * 3) catch unreachable;
var auto_accessor_count: u32 = 0;

for (properties) |prop| {
if (prop.kind != .auto_accessor) {
new_props.append(prop) catch unreachable;
continue;
}

const loc = if (prop.key) |k| k.loc else logger.Loc.Empty;
const is_static = prop.flags.contains(.is_static);
const is_computed = prop.flags.contains(.is_computed);

// Derive backing field name from the key
const backing_name: []const u8 = blk: {
if (prop.key != null and prop.key.?.data == .e_private_identifier) {
// accessor #x -> backing field #_x
const orig_name = p.loadNameFromRef(prop.key.?.data.e_private_identifier.ref);
break :blk std.fmt.allocPrint(p.allocator, "#_{s}", .{orig_name[1..]}) catch unreachable;
} else if (!is_computed) {
if (prop.key != null and prop.key.?.data == .e_string) {
// accessor x -> backing field #x
const str = prop.key.?.data.e_string;
break :blk std.fmt.allocPrint(p.allocator, "#{s}", .{str.data}) catch unreachable;
}
Comment on lines +4873 to +4884

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid backing-field collisions with existing private names.

For non-computed accessors, #${name} (and #_${name} for private accessors) can collide with user-declared private fields, producing invalid JS due to duplicate private identifiers. Please guard these paths and fall back to a unique name when a collision is detected.

🔧 Example fix (fallback to unique name on collision)
                 const backing_name: []const u8 = blk: {
                     if (prop.key != null and prop.key.?.data == .e_private_identifier) {
                         // accessor `#x` -> backing field `#_x`
                         const orig_name = p.loadNameFromRef(prop.key.?.data.e_private_identifier.ref);
-                        break :blk std.fmt.allocPrint(p.allocator, "#_{s}", .{orig_name[1..]}) catch unreachable;
+                        const candidate = std.fmt.allocPrint(p.allocator, "#_{s}", .{orig_name[1..]}) catch unreachable;
+                        if (!privateNameCollides(p, properties, candidate)) break :blk candidate;
                     } else if (!is_computed) {
                         if (prop.key != null and prop.key.?.data == .e_string) {
                             // accessor x -> backing field `#x`
                             const str = prop.key.?.data.e_string;
-                            break :blk std.fmt.allocPrint(p.allocator, "#{s}", .{str.data}) catch unreachable;
+                            const candidate = std.fmt.allocPrint(p.allocator, "#{s}", .{str.data}) catch unreachable;
+                            if (!privateNameCollides(p, properties, candidate)) break :blk candidate;
                         }
                     }
                     // Computed key -> counter-based name, skipping any
                     // that collide with existing private names in the class
// add a small local helper near the top of lowerAutoAccessors
const privateNameCollides = struct {
    fn check(p: *P, props: []G.Property, candidate: []const u8) bool {
        for (props) |other| {
            if (other.key != null and other.key.?.data == .e_private_identifier) {
                const other_name = p.loadNameFromRef(other.key.?.data.e_private_identifier.ref);
                if (strings.eql(candidate, other_name)) return true;
            }
        }
        return false;
    }
}.check;
🤖 Prompt for AI Agents
In `@src/ast/P.zig` around lines 4873 - 4884, The generated backing field names
for non-computed accessors (code around lowerAutoAccessors producing
backing_name from prop.key and patterns like "#_{s}" and "#{s}") can collide
with user-declared private identifiers; add a collision check that scans the
current props for existing .e_private_identifier names (use p.loadNameFromRef to
read each private name) and, when a candidate backing_name matches an existing
private identifier, fall back to creating a unique name (e.g., append a suffix
or generate a unique id) instead of returning the colliding "#..." name;
implement this as a small helper (e.g., privateNameCollides) and invoke it in
the branches that produce backing_name for private_identifier and .e_string
keys.

}
// Computed key -> counter-based name, skipping any
// that collide with existing private names in the class
// (private names are not subject to automatic renaming).
while (true) : (auto_accessor_count += 1) {
const offset: u8 = @intCast(@min(auto_accessor_count, 25));
const candidate = std.fmt.allocPrint(p.allocator, "#{c}", .{
@as(u8, 'a' + offset),
}) catch unreachable;
var collides = false;
for (properties) |other| {
if (other.key != null and other.key.?.data == .e_private_identifier) {
const other_name = p.loadNameFromRef(other.key.?.data.e_private_identifier.ref);
if (strings.eql(candidate, other_name)) {
collides = true;
break;
}
}
}
if (!collides) {
auto_accessor_count += 1;
break :blk candidate;
}
}
unreachable;
};

// Create backing field symbol
const backing_ref = p.newSymbol(
if (is_static) .private_static_field else .private_field,
backing_name,
) catch unreachable;
p.recordDeclaredSymbol(backing_ref) catch unreachable;

// For private auto-accessors, update the original symbol kind
// from .private_field to .private_get_set_pair
if (prop.key != null and prop.key.?.data == .e_private_identifier) {
const orig_ref = prop.key.?.data.e_private_identifier.ref;
p.symbols.items[orig_ref.innerIndex()].kind =
if (is_static) .private_static_get_set_pair else .private_get_set_pair;
}

// For computed keys, cache in a temp variable so the expression
// is only evaluated once (getter uses _a = expr, setter uses _a)
var getter_key = prop.key;
var setter_key = prop.key;
if (is_computed) {
const temp_ref = p.newSymbol(.other, "_a") catch unreachable;
p.recordDeclaredSymbol(temp_ref) catch unreachable;
p.auto_accessor_computed_key_refs.append(
p.allocator,
LocRef{ .loc = loc, .ref = temp_ref },
) catch unreachable;

// Getter key: _a = expr (evaluate and cache)
getter_key = p.newExpr(E.Binary{
.op = .bin_assign,
.left = p.newExpr(E.Identifier{ .ref = temp_ref }, loc),
.right = prop.key.?,
}, loc);
// Setter key: _a (reuse cached value)
setter_key = p.newExpr(E.Identifier{ .ref = temp_ref }, loc);
Comment on lines +4858 to +4946

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Prevent temp/backing-name collisions for computed auto-accessors.
The computed backing name only ranges over #a..#z (Line 4887), which can collide with user private names or after 26 accessors, yielding duplicate private identifiers (syntax error). The temp name is always _a (Line 4915), which can clash with user let/const _a when renaming is off, also causing a syntax error. Please make both names collision‑resistant and unique per computed accessor.

🔧 Suggested fix (unique, collision‑resistant names)
-            var auto_accessor_count: u32 = 0;
+            var auto_accessor_count: u32 = 0;
+            var computed_key_index: ?u32 = null;
@@
-                    const offset: u8 = `@intCast`(`@min`(auto_accessor_count, 25));
-                    const name = std.fmt.allocPrint(p.allocator, "#{c}", .{
-                        `@as`(u8, 'a' + offset),
-                    }) catch unreachable;
-                    auto_accessor_count += 1;
-                    break :blk name;
+                    computed_key_index = auto_accessor_count;
+                    auto_accessor_count += 1;
+                    const name = std.fmt.allocPrint(
+                        p.allocator,
+                        "#_auto_accessor_{d}",
+                        .{computed_key_index.?},
+                    ) catch unreachable;
+                    break :blk name;
@@
-                    const temp_ref = p.newSymbol(.other, "_a") catch unreachable;
+                    const temp_name = std.fmt.allocPrint(
+                        p.allocator,
+                        "_auto_accessor_{d}",
+                        .{computed_key_index.?},
+                    ) catch unreachable;
+                    const temp_ref = p.newSymbol(.other, temp_name) catch unreachable;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn lowerAutoAccessors(p: *P, properties: []G.Property) []G.Property {
var new_props = ListManaged(G.Property).init(p.allocator);
new_props.ensureTotalCapacity(properties.len * 3) catch unreachable;
var auto_accessor_count: u32 = 0;
for (properties) |prop| {
if (prop.kind != .auto_accessor) {
new_props.append(prop) catch unreachable;
continue;
}
const loc = if (prop.key) |k| k.loc else logger.Loc.Empty;
const is_static = prop.flags.contains(.is_static);
const is_computed = prop.flags.contains(.is_computed);
// Derive backing field name from the key
const backing_name: []const u8 = blk: {
if (prop.key != null and prop.key.?.data == .e_private_identifier) {
// accessor #x -> backing field #_x
const orig_name = p.loadNameFromRef(prop.key.?.data.e_private_identifier.ref);
break :blk std.fmt.allocPrint(p.allocator, "#_{s}", .{orig_name[1..]}) catch unreachable;
} else if (!is_computed) {
if (prop.key != null and prop.key.?.data == .e_string) {
// accessor x -> backing field #x
const str = prop.key.?.data.e_string;
break :blk std.fmt.allocPrint(p.allocator, "#{s}", .{str.data}) catch unreachable;
}
}
// Computed key -> counter-based name
const offset: u8 = @intCast(@min(auto_accessor_count, 25));
const name = std.fmt.allocPrint(p.allocator, "#{c}", .{
@as(u8, 'a' + offset),
}) catch unreachable;
auto_accessor_count += 1;
break :blk name;
};
// Create backing field symbol
const backing_ref = p.newSymbol(
if (is_static) .private_static_field else .private_field,
backing_name,
) catch unreachable;
p.recordDeclaredSymbol(backing_ref) catch unreachable;
// For private auto-accessors, update the original symbol kind
// from .private_field to .private_get_set_pair
if (prop.key != null and prop.key.?.data == .e_private_identifier) {
const orig_ref = prop.key.?.data.e_private_identifier.ref;
p.symbols.items[orig_ref.innerIndex()].kind =
if (is_static) .private_static_get_set_pair else .private_get_set_pair;
}
// For computed keys, cache in a temp variable so the expression
// is only evaluated once (getter uses _a = expr, setter uses _a)
var getter_key = prop.key;
var setter_key = prop.key;
if (is_computed) {
const temp_ref = p.newSymbol(.other, "_a") catch unreachable;
p.recordDeclaredSymbol(temp_ref) catch unreachable;
p.auto_accessor_computed_key_refs.append(
p.allocator,
LocRef{ .loc = loc, .ref = temp_ref },
) catch unreachable;
// Getter key: _a = expr (evaluate and cache)
getter_key = p.newExpr(E.Binary{
.op = .bin_assign,
.left = p.newExpr(E.Identifier{ .ref = temp_ref }, loc),
.right = prop.key.?,
}, loc);
// Setter key: _a (reuse cached value)
setter_key = p.newExpr(E.Identifier{ .ref = temp_ref }, loc);
pub fn lowerAutoAccessors(p: *P, properties: []G.Property) []G.Property {
var new_props = ListManaged(G.Property).init(p.allocator);
new_props.ensureTotalCapacity(properties.len * 3) catch unreachable;
var auto_accessor_count: u32 = 0;
var computed_key_index: ?u32 = null;
for (properties) |prop| {
if (prop.kind != .auto_accessor) {
new_props.append(prop) catch unreachable;
continue;
}
const loc = if (prop.key) |k| k.loc else logger.Loc.Empty;
const is_static = prop.flags.contains(.is_static);
const is_computed = prop.flags.contains(.is_computed);
// Derive backing field name from the key
const backing_name: []const u8 = blk: {
if (prop.key != null and prop.key.?.data == .e_private_identifier) {
// accessor `#x` -> backing field `#_x`
const orig_name = p.loadNameFromRef(prop.key.?.data.e_private_identifier.ref);
break :blk std.fmt.allocPrint(p.allocator, "#_{s}", .{orig_name[1..]}) catch unreachable;
} else if (!is_computed) {
if (prop.key != null and prop.key.?.data == .e_string) {
// accessor x -> backing field `#x`
const str = prop.key.?.data.e_string;
break :blk std.fmt.allocPrint(p.allocator, "#{s}", .{str.data}) catch unreachable;
}
}
// Computed key -> counter-based name
computed_key_index = auto_accessor_count;
auto_accessor_count += 1;
const name = std.fmt.allocPrint(
p.allocator,
"#_auto_accessor_{d}",
.{computed_key_index.?},
) catch unreachable;
break :blk name;
};
// Create backing field symbol
const backing_ref = p.newSymbol(
if (is_static) .private_static_field else .private_field,
backing_name,
) catch unreachable;
p.recordDeclaredSymbol(backing_ref) catch unreachable;
// For private auto-accessors, update the original symbol kind
// from .private_field to .private_get_set_pair
if (prop.key != null and prop.key.?.data == .e_private_identifier) {
const orig_ref = prop.key.?.data.e_private_identifier.ref;
p.symbols.items[orig_ref.innerIndex()].kind =
if (is_static) .private_static_get_set_pair else .private_get_set_pair;
}
// For computed keys, cache in a temp variable so the expression
// is only evaluated once (getter uses _a = expr, setter uses _a)
var getter_key = prop.key;
var setter_key = prop.key;
if (is_computed) {
const temp_name = std.fmt.allocPrint(
p.allocator,
"_auto_accessor_{d}",
.{computed_key_index.?},
) catch unreachable;
const temp_ref = p.newSymbol(.other, temp_name) catch unreachable;
p.recordDeclaredSymbol(temp_ref) catch unreachable;
p.auto_accessor_computed_key_refs.append(
p.allocator,
LocRef{ .loc = loc, .ref = temp_ref },
) catch unreachable;
// Getter key: _a = expr (evaluate and cache)
getter_key = p.newExpr(E.Binary{
.op = .bin_assign,
.left = p.newExpr(E.Identifier{ .ref = temp_ref }, loc),
.right = prop.key.?,
}, loc);
// Setter key: _a (reuse cached value)
setter_key = p.newExpr(E.Identifier{ .ref = temp_ref }, loc);
🤖 Prompt for AI Agents
In `@src/ast/P.zig` around lines 4858 - 4929, The computed-accessor backing-name
and temp symbol are not unique: change lowerAutoAccessors so the computed
backing_name generation (currently using auto_accessor_count with 'a'..'z')
produces a collision-resistant unique string (e.g., include the
auto_accessor_count numeric suffix or a monotonically increasing id instead of
limiting to 26) and increment the counter deterministically; likewise create the
temp symbol name passed to p.newSymbol (currently "_a") to include the same
unique suffix (e.g., "_auto_accessor_<n>") so each computed accessor gets
distinct backing and temp identifiers; update uses of auto_accessor_count,
backing_name creation, and the temp_ref/newSymbol call to use this unique naming
scheme.

}

// 1. Backing field (kind = .normal, private symbol, with initializer)
new_props.append(.{
.kind = .normal,
.key = p.newExpr(E.PrivateIdentifier{ .ref = backing_ref }, loc),
.initializer = prop.initializer,
.flags = Flags.Property.init(.{ .is_static = is_static }),
}) catch unreachable;

// Helper: this.#backing expression
const this_dot_backing = p.newExpr(E.Index{
.target = p.newExpr(E.This{}, loc),
.index = p.newExpr(E.PrivateIdentifier{ .ref = backing_ref }, loc),
}, loc);

// 2. Getter: get x() { return this.#backing; }
new_props.append(.{
.kind = .get,
.key = getter_key,
.value = p.newExpr(E.Function{ .func = .{
.body = G.FnBody.initReturnExpr(p.allocator, this_dot_backing) catch unreachable,
.open_parens_loc = loc,
} }, loc),
.flags = Flags.Property.init(.{
.is_static = is_static,
.is_method = true,
.is_computed = is_computed,
}),
}) catch unreachable;

// 3. Setter: set x(_) { this.#backing = _; }
const setter_param_ref = p.newSymbol(.other, "_") catch unreachable;
const setter_arg = p.allocator.alloc(G.Arg, 1) catch unreachable;
setter_arg[0] = .{
.binding = Binding.alloc(p.allocator, B.Identifier{ .ref = setter_param_ref }, loc),
};

const assign_expr = p.newExpr(E.Binary{
.op = .bin_assign,
.left = p.newExpr(E.Index{
.target = p.newExpr(E.This{}, loc),
.index = p.newExpr(E.PrivateIdentifier{ .ref = backing_ref }, loc),
}, loc),
.right = p.newExpr(E.Identifier{ .ref = setter_param_ref }, loc),
}, loc);

const setter_body_stmts = p.allocator.alloc(Stmt, 1) catch unreachable;
setter_body_stmts[0] = p.s(S.SExpr{ .value = assign_expr }, loc);

new_props.append(.{
.kind = .set,
.key = setter_key,
.value = p.newExpr(E.Function{ .func = .{
.body = .{ .loc = loc, .stmts = setter_body_stmts },
.args = setter_arg,
.open_parens_loc = loc,
} }, loc),
.flags = Flags.Property.init(.{
.is_static = is_static,
.is_method = true,
.is_computed = is_computed,
}),
}) catch unreachable;
}

return new_props.items;
}

pub fn lowerClass(
noalias p: *P,
stmtorexpr: js_ast.StmtOrExpr,
Expand Down Expand Up @@ -4910,9 +5072,9 @@ pub fn NewParser_(
const descriptor_key = prop.key.?;
const loc = descriptor_key.loc;

// TODO: when we have the `accessor` modifier, add `and !prop.flags.contains(.has_accessor_modifier)` to
// the if statement.
const descriptor_kind: Expr = if (!prop.flags.contains(.is_method))
const descriptor_kind: Expr = if (prop.kind == .auto_accessor)
p.newExpr(E.Null{}, loc)
else if (!prop.flags.contains(.is_method))
p.newExpr(E.Undefined{}, loc)
else
p.newExpr(E.Null{}, loc);
Expand All @@ -4929,7 +5091,7 @@ pub fn NewParser_(

if (p.options.features.emit_decorator_metadata) {
switch (prop.kind) {
.normal, .abstract => {
.normal, .abstract, .auto_accessor => {
{
// design:type
var args = p.allocator.alloc(Expr, 2) catch unreachable;
Expand Down
15 changes: 12 additions & 3 deletions src/ast/parseProperty.zig
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ pub fn ParseProperty(
const scope_index = p.scopes_in_order.items.len;
if (try p.parseProperty(kind, opts, null)) |_prop| {
var prop = _prop;
if (prop.kind == .normal and prop.value == null and opts.ts_decorators.len > 0) {
if ((prop.kind == .normal or prop.kind == .auto_accessor) and prop.value == null and opts.ts_decorators.len > 0) {
prop.kind = .declare;
return prop;
}
Expand All @@ -289,7 +289,7 @@ pub fn ParseProperty(
opts.is_ts_abstract = true;
const scope_index = p.scopes_in_order.items.len;
if (try p.parseProperty(kind, opts, null)) |*prop| {
if (prop.kind == .normal and prop.value == null and opts.ts_decorators.len > 0) {
if ((prop.kind == .normal or prop.kind == .auto_accessor) and prop.value == null and opts.ts_decorators.len > 0) {
var prop_ = prop.*;
prop_.kind = .abstract;
return prop_;
Expand All @@ -299,6 +299,15 @@ pub fn ParseProperty(
return null;
}
},
.p_accessor => {
if (opts.is_class and !opts.is_async and !opts.is_generator and
(js_lexer.PropertyModifierKeyword.List.get(raw) orelse .p_static) == .p_accessor)
{
kind = .auto_accessor;
errors = null;
continue :restart;
}
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.p_private, .p_protected, .p_public, .p_readonly, .p_override => {
// Skip over TypeScript keywords
if (opts.is_class and is_typescript_enabled and (js_lexer.PropertyModifierKeyword.List.get(raw) orelse .p_static) == keyword) {
Expand Down Expand Up @@ -430,7 +439,7 @@ pub fn ParseProperty(

// Parse a class field with an optional initial value
if (opts.is_class and
kind == .normal and !opts.is_async and
(kind == .normal or kind == .auto_accessor) and !opts.is_async and
!opts.is_generator and
p.lexer.token != .t_open_paren and
!has_type_parameters and
Expand Down
25 changes: 25 additions & 0 deletions src/ast/visit.zig
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,20 @@ pub fn Visit(
}
}
}

// Lower auto-accessors into backing field + getter + setter
{
var has_auto_accessors = false;
for (class.properties) |prop| {
if (prop.kind == .auto_accessor) {
has_auto_accessors = true;
break;
}
}
if (has_auto_accessors) {
class.properties = p.lowerAutoAccessors(class.properties);
}
}
}

if (p.symbols.items[shadow_ref.innerIndex()].use_count_estimate == 0) {
Expand Down Expand Up @@ -830,6 +844,17 @@ pub fn Visit(
break :list_getter &visited;
};
try p.visitAndAppendStmt(list, stmt);

// Emit var declarations for auto-accessor computed key temps
// that were generated from class expressions during this statement.
// (Class statement temps are handled in visitAndAppendStmt directly.)
// var is hoisted so placement within the statement list is fine.
for (p.auto_accessor_computed_key_refs.items) |temp| {
const decls = p.allocator.alloc(G.Decl, 1) catch unreachable;
decls[0] = .{ .binding = p.b(B.Identifier{ .ref = temp.ref.? }, temp.loc) };
try list.append(p.s(S.Local{ .decls = G.Decl.List.fromOwnedSlice(decls) }, temp.loc));
}
Comment on lines +848 to +856

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find S.Local struct definition
rg -n "struct Local|pub const Local" -g '*.zig' src --max-count=20

Repository: oven-sh/bun

Length of output: 686


🏁 Script executed:

#!/bin/bash
# Search for S.Local usage patterns in visit.zig to see if .kind is typically explicit
rg -n "S\.Local\{" -g '*.zig' src/ast/visit.zig -A 3 | head -60

Repository: oven-sh/bun

Length of output: 683


🏁 Script executed:

#!/bin/bash
# Look for the definition of S namespace and Local struct more broadly
fd -g '*.zig' src | xargs grep -l "pub.*struct.*Local" | head -5

Repository: oven-sh/bun

Length of output: 37


🏁 Script executed:

#!/bin/bash
# Get the S.Local struct definition from src/ast/S.zig
sed -n '171,200p' src/ast/S.zig

Repository: oven-sh/bun

Length of output: 964


🏁 Script executed:

#!/bin/bash
# Get more context around S.Local definition
sed -n '165,210p' src/ast/S.zig

Repository: oven-sh/bun

Length of output: 1295


Make temp decl kind explicit (.k_var).

These computed-key temps shouldn't depend on S.Local defaults. The default is .k_var, which is the intended value here, but explicit declaration aligns with how .kind is set in nearby code (lines 944–946 and 961–963).

♻️ Proposed tweak
-                        try list.append(p.s(S.Local{ .decls = G.Decl.List.fromOwnedSlice(decls) }, temp.loc));
+                        try list.append(p.s(S.Local{
+                            .kind = .k_var,
+                            .decls = G.Decl.List.fromOwnedSlice(decls),
+                        }, temp.loc));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Emit var declarations for auto-accessor computed key temps
// that were generated from class expressions during this statement.
// (Class statement temps are handled in visitAndAppendStmt directly.)
// var is hoisted so placement within the statement list is fine.
for (p.auto_accessor_computed_key_refs.items) |temp| {
const decls = p.allocator.alloc(G.Decl, 1) catch unreachable;
decls[0] = .{ .binding = p.b(B.Identifier{ .ref = temp.ref.? }, temp.loc) };
try list.append(p.s(S.Local{ .decls = G.Decl.List.fromOwnedSlice(decls) }, temp.loc));
}
// Emit var declarations for auto-accessor computed key temps
// that were generated from class expressions during this statement.
// (Class statement temps are handled in visitAndAppendStmt directly.)
// var is hoisted so placement within the statement list is fine.
for (p.auto_accessor_computed_key_refs.items) |temp| {
const decls = p.allocator.alloc(G.Decl, 1) catch unreachable;
decls[0] = .{ .binding = p.b(B.Identifier{ .ref = temp.ref.? }, temp.loc) };
try list.append(p.s(S.Local{
.kind = .k_var,
.decls = G.Decl.List.fromOwnedSlice(decls),
}, temp.loc));
}
🤖 Prompt for AI Agents
In `@src/ast/visit.zig` around lines 848 - 856, The loop emitting temps for
auto_accessor_computed_key_refs relies on S.Local's default kind but should set
the Var kind explicitly; update the creation of the G.Decl for each temp in the
loop that builds p.s(S.Local{ .decls = G.Decl.List.fromOwnedSlice(decls) },
temp.loc) so the decl includes the explicit kind `.k_var` (i.e., set the
Decl.kind field when constructing the G.Decl for each temp), matching nearby
patterns that explicitly set `.kind` for local declarations and ensuring
consistency for the auto_accessor_computed_key_refs handling.

p.auto_accessor_computed_key_refs.clearRetainingCapacity();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Transform block-level function declarations into variable declarations
Expand Down
8 changes: 8 additions & 0 deletions src/ast/visitStmt.zig
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,14 @@ pub fn VisitStmt(

_ = p.visitClass(stmt.loc, &data.class, Ref.None);

// Emit var declarations for auto-accessor computed key temps
for (p.auto_accessor_computed_key_refs.items) |temp| {
const decls = p.allocator.alloc(Decl, 1) catch unreachable;
decls[0] = .{ .binding = Binding.alloc(p.allocator, B.Identifier{ .ref = temp.ref.? }, temp.loc) };
stmts.append(p.s(S.Local{ .decls = Decl.List.fromOwnedSlice(decls) }, temp.loc)) catch unreachable;
}
Comment on lines +590 to +595

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

# Find S.Local struct definition
rg -n "pub const Local\|struct Local" -A 10 -g '*.zig' src | head -50

Repository: oven-sh/bun

Length of output: 37


🏁 Script executed:

# Search for k_var enumeration
rg -n "k_var" -B 2 -A 2 -g '*.zig' src | head -50

Repository: oven-sh/bun

Length of output: 3675


🏁 Script executed:

# Look at the broader context of S.Local usage in the file
sed -n '585,600p' src/ast/visitStmt.zig

Repository: oven-sh/bun

Length of output: 949


🏁 Script executed:

# Search for where S is defined or S.Local is declared
rg -n "^pub const S\|^pub struct S" -A 5 -g '*.zig' src | head -100

Repository: oven-sh/bun

Length of output: 37


🏁 Script executed:

# Try searching in js_ast file
fd -n "js_ast" -g '*.zig' src | xargs rg -l "Local"

Repository: oven-sh/bun

Length of output: 17217


🏁 Script executed:

# Look for where Local kind enum is defined
rg -n "kind.*:" -g '*.zig' src/ast | grep -i local

Repository: oven-sh/bun

Length of output: 367


🏁 Script executed:

# Find the Local struct in S.zig
rg -n "pub const Local\|^    Local" -A 15 src/ast/S.zig | head -80

Repository: oven-sh/bun

Length of output: 37


🏁 Script executed:

# Check S.zig and search for Local more broadly
grep -n "Local\|kind" src/ast/S.zig | head -50

Repository: oven-sh/bun

Length of output: 385


🏁 Script executed:

# Let's read S.zig to find Local definition
wc -l src/ast/S.zig

Repository: oven-sh/bun

Length of output: 72


Consider explicitly setting .kind = .k_var to match codebase patterns.

While S.Local.kind defaults to .k_var (as defined in S.zig), other parts of the codebase explicitly set this field when creating var declarations (e.g., convertStmtsForChunkForDevServer.zig, visit.zig). Making it explicit here would clarify intent and improve consistency.

♻️ Suggested improvement
-                    stmts.append(p.s(S.Local{ .decls = Decl.List.fromOwnedSlice(decls) }, temp.loc)) catch unreachable;
+                    stmts.append(p.s(S.Local{
+                        .kind = .k_var,
+                        .decls = Decl.List.fromOwnedSlice(decls),
+                    }, temp.loc)) catch unreachable;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Emit var declarations for auto-accessor computed key temps
for (p.auto_accessor_computed_key_refs.items) |temp| {
const decls = p.allocator.alloc(Decl, 1) catch unreachable;
decls[0] = .{ .binding = Binding.alloc(p.allocator, B.Identifier{ .ref = temp.ref.? }, temp.loc) };
stmts.append(p.s(S.Local{ .decls = Decl.List.fromOwnedSlice(decls) }, temp.loc)) catch unreachable;
}
// Emit var declarations for auto-accessor computed key temps
for (p.auto_accessor_computed_key_refs.items) |temp| {
const decls = p.allocator.alloc(Decl, 1) catch unreachable;
decls[0] = .{ .binding = Binding.alloc(p.allocator, B.Identifier{ .ref = temp.ref.? }, temp.loc) };
stmts.append(p.s(S.Local{
.kind = .k_var,
.decls = Decl.List.fromOwnedSlice(decls),
}, temp.loc)) catch unreachable;
}
🤖 Prompt for AI Agents
In `@src/ast/visitStmt.zig` around lines 590 - 595, The loop emits local
declarations using p.s(S.Local{ .decls = ... }) but relies on the default
S.Local.kind; update the S.Local construction to explicitly set .kind = .k_var
to match project conventions (i.e., call p.s with S.Local{ .kind = .k_var,
.decls = Decl.List.fromOwnedSlice(decls) }) when appending stmts for
p.auto_accessor_computed_key_refs so intent and consistency with other sites
like convertStmtsForChunkForDevServer.zig and visit.zig are clear.

p.auto_accessor_computed_key_refs.clearRetainingCapacity();

// Remove the export flag inside a namespace
const was_export_inside_namespace = data.is_export and p.enclosing_namespace_arg_ref != null;
if (was_export_inside_namespace) {
Expand Down
2 changes: 2 additions & 0 deletions src/js_lexer_tables.zig
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ pub const StrictModeReservedWordsRemap = ComptimeStringMap(string, .{

pub const PropertyModifierKeyword = enum {
p_abstract,
p_accessor,
p_async,
p_declare,
p_get,
Expand All @@ -227,6 +228,7 @@ pub const PropertyModifierKeyword = enum {

pub const List = ComptimeStringMap(PropertyModifierKeyword, .{
.{ "abstract", .p_abstract },
.{ "accessor", .p_accessor },
.{ "async", .p_async },
.{ "declare", .p_declare },
.{ "get", .p_get },
Expand Down
9 changes: 8 additions & 1 deletion src/js_printer.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3292,6 +3292,13 @@ fn NewPrinter(
p.print("set");
p.printSpace();
},
.auto_accessor => {
if (comptime is_json and Environment.allow_assert)
unreachable;
p.printSpaceBeforeIdentifier();
p.print("accessor");
p.printSpace();
},
else => {},
}

Expand Down Expand Up @@ -3478,7 +3485,7 @@ fn NewPrinter(
},
}

if (item.kind != .normal) {
if (item.kind != .normal and item.kind != .auto_accessor) {
if (comptime is_json) {
bun.unreachablePanic("item.kind must be normal in json", .{});
}
Expand Down
Loading
Loading