Skip to content

Package.init, log if manifest contains unexpected lua types #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
D-Berg wants to merge 1 commit into from
Open

Package.init, log if manifest contains unexpected lua types #23

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions src/Package.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pub const State = struct {

const package_index = @intFromEnum(index);
assert(package_index < self.packages.len);

const package = self.packages.get(package_index);
assert(package.id == id);
return package;
Expand Down Expand Up @@ -151,6 +152,7 @@ pub fn init(
) !Package {
assert(pkg_name.len > 0);
assert(repo.len > 0);

var arena_impl: std.heap.ArenaAllocator = .init(gpa);
defer arena_impl.deinit();

Expand All @@ -162,6 +164,7 @@ pub fn init(
pkg_name,
}, 0);
const manifest_stat = try packa_dir.statFile(io, manifest_path[1..], .{ .follow_symlinks = true });

const manifest = try packa_dir.readFileAllocOptions(
io,
manifest_path[1..],
Expand All @@ -186,18 +189,24 @@ pub fn init(
// TODO: log errors
const name = try state.string_state.internString(gpa, switch (lua.getField(pkg, "name")) {
.string => lua.toLString(-1),
else => return error.WrongLuaType,
else => |kind| {
log.err("Package expected name to be function, got {t}", .{kind});
return error.WrongLuaType;
Comment on lines +192 to +194

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the expected type in the name diagnostic.

The switch accepts only .string, but the log says that name must be a function. Report string instead.

Proposed fix 
-            log.err("Package expected name to be function, got {t}", .{kind});
+            log.err("Package expected name to be string, got {t}", .{kind});
📝 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
else => |kind| {
log.err("Package expected name to be function, got {t}", .{kind});
return error.WrongLuaType;
else => |kind| {
log.err("Package expected name to be string, got {t}", .{kind});
return error.WrongLuaType;
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Package.zig` around lines 192 - 194, Update the `else` branch of the
`name` type switch in `Package` to report that the package name was expected to
be a string, replacing the incorrect function type in the `log.err` diagnostic
while preserving the existing error return.

},
});
lua.pop(1);

if (!std.mem.eql(u8, pkg_name, name.slice(&state.string_state))) {
log.err("Package name differs from expected name '{s}', got {s}", .{ pkg_name, name.slice(&state.string_state) });
return error.WrontPackageName;
return error.WrongPackageName;
}

const version: std.SemanticVersion = try .parse(switch (lua.getField(pkg, "version")) {
.string => lua.toLString(-1),
else => return error.WrongLuaType,
else => |kind| {
log.err("Package expected type of version to be string, got {t}", .{kind});
return error.WrongLuaType;
},
});
lua.pop(1);

Expand Down