Upgrade to Zig 0.15.2 - #38
Conversation
…nd fix self references in free functions
📝 WalkthroughWalkthroughThis PR performs a Zig toolchain version bump from 0.15.1 to 0.15.2 across configuration and documentation files, refactors code generators to propagate allocators through function parameters instead of using instance state, adds explicit allocation-tracking flags to the Server struct, simplifies output path handling in the main generator, and standardizes error formatting across test files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/copilot-instructions.md:
- Around line 365-367: The troubleshooting text is ambiguous about allowed Zig
versions; update the lines containing the strings "zig: command not found",
"Wrong Zig version", and the phrase "not 0.13.x or 0.15.x" to explicitly require
Zig 0.15.2 and to clarify that other 0.15.y releases (e.g., 0.15.0/0.15.1) are
not acceptable; reword to read something like "Install Zig 0.15.2 exactly" and
"Ensure you're using Zig 0.15.2 (do not use 0.13.x or earlier 0.15.x releases
prior to 0.15.2)" so readers understand 0.15.2 is the only supported 0.15
version.
In `@src/generators/unified/api_generator.zig`:
- Around line 238-241: The generated Zig code calls str.writer() without passing
an allocator which fails in Zig 0.15.2; update the generator call site that
writes the stringify line (the appendSlice invocation that emits "try
std.json.stringify(requestBody, .{}, str.writer());") to instead emit "try
std.json.stringify(requestBody, .{}, str.writer(allocator));". Apply the same
change wherever this string is emitted (notably in the apigenerator
implementations that produce that line for unified, v2.0, and v3.0) so the
generated code calls str.writer(allocator) and compiles.
🧹 Nitpick comments (3)
src/tests/swagger_v2_tests.zig (1)
9-9: UnnecessaryseekBy(0)call still present.The PR description states "Remove unnecessary seekBy(0) calls," but
loadSwaggerDocumentstill hastry file.seekBy(0);on Line 9. This is a no-op (seeking by 0 bytes from the current position) and should be removed for consistency with the stated objective.Proposed fix
fn loadSwaggerDocument(allocator: std.mem.Allocator, file_path: []const u8) !models.SwaggerDocument { const file = try std.fs.cwd().openFile(file_path, .{}); defer file.close(); - try file.seekBy(0); const file_contents = try file.readToEndAlloc(allocator, std.math.maxInt(usize)); defer allocator.free(file_contents); return try models.SwaggerDocument.parseFromJson(allocator, file_contents); }src/tests/unified_converter_tests.zig (1)
10-10: UnnecessaryseekBy(0)call remains.The PR description mentions removing unnecessary
seekBy(0)calls. This one on Line 10 (and Line 19) still exists. A file opened for reading is already at position 0.♻️ Remove no-op seek
const file = try std.fs.cwd().openFile(file_path, .{}); defer file.close(); - try file.seekBy(0); const file_contents = try file.readToEndAlloc(allocator, std.math.maxInt(usize));src/tests/comprehensive_converter_tests.zig (1)
10-10: Same unnecessaryseekBy(0)as in other test files.Same as in
unified_converter_tests.zig— Lines 10 and 19 have no-opseekBy(0)calls that could be removed.
| - **"zig: command not found"**: Install Zig 0.15.2 exactly using methods above | ||
| - **Wrong Zig version**: Uninstall other versions, install 0.15.2 specifically | ||
| - **Build failures**: Ensure you're using Zig 0.15.2, not 0.13.x or 0.15.x |
There was a problem hiding this comment.
Ambiguous version reference in troubleshooting.
Line 367 says "not 0.13.x or 0.15.x" but 0.15.2 is 0.15.x. This could confuse readers.
📝 Suggested fix
-- **Build failures**: Ensure you're using Zig 0.15.2, not 0.13.x or 0.15.x
+- **Build failures**: Ensure you're using Zig 0.15.2, not 0.13.x or 0.14.x📝 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.
| - **"zig: command not found"**: Install Zig 0.15.2 exactly using methods above | |
| - **Wrong Zig version**: Uninstall other versions, install 0.15.2 specifically | |
| - **Build failures**: Ensure you're using Zig 0.15.2, not 0.13.x or 0.15.x | |
| - **"zig: command not found"**: Install Zig 0.15.2 exactly using methods above | |
| - **Wrong Zig version**: Uninstall other versions, install 0.15.2 specifically | |
| - **Build failures**: Ensure you're using Zig 0.15.2, not 0.13.x or 0.14.x |
🤖 Prompt for AI Agents
In @.github/copilot-instructions.md around lines 365 - 367, The troubleshooting
text is ambiguous about allowed Zig versions; update the lines containing the
strings "zig: command not found", "Wrong Zig version", and the phrase "not
0.13.x or 0.15.x" to explicitly require Zig 0.15.2 and to clarify that other
0.15.y releases (e.g., 0.15.0/0.15.1) are not acceptable; reword to read
something like "Install Zig 0.15.2 exactly" and "Ensure you're using Zig 0.15.2
(do not use 0.13.x or earlier 0.15.x releases prior to 0.15.2)" so readers
understand 0.15.2 is the only supported 0.15 version.
| try self.buffer.appendSlice(self.allocator, " var str = std.ArrayList(u8){};\n"); | ||
| try self.buffer.appendSlice(self.allocator, " defer str.deinit();\n\n"); | ||
| try self.buffer.appendSlice(self.allocator, " defer str.deinit(allocator);\n\n"); | ||
| try self.buffer.appendSlice(self.allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | ||
| try self.buffer.appendSlice(self.allocator, " const payload = str.items;\n\n"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/generators/unified/api_generator.zig | sed -n '230,250p'Repository: christianhelle/openapi2zig
Length of output: 1629
🏁 Script executed:
# Also check what other .writer() calls exist in the codebase to understand the pattern
rg -n '\.writer\(' --type=zig -C 2Repository: christianhelle/openapi2zig
Length of output: 2217
🏁 Script executed:
# Check build.zig for Zig version requirement
rg -n "zig_version\|\.builder\|minimum_zig" build.zig -A 3Repository: christianhelle/openapi2zig
Length of output: 52
🏁 Script executed:
# Check if there's a build.zig.zon or version file
ls -la build.zig.zon 2>/dev/null || echo "No build.zig.zon found"
head -20 build.zig.zon 2>/dev/null || echo "File not found"Repository: christianhelle/openapi2zig
Length of output: 1056
🏁 Script executed:
# Check what Zig version is mentioned anywhere in the repo
rg -i "zig.*0\.15\|0\.15.*zig" --type-list | head -20Repository: christianhelle/openapi2zig
Length of output: 459
🏁 Script executed:
# Check build.zig for Zig version constraints
cat -n build.zig | head -30Repository: christianhelle/openapi2zig
Length of output: 1224
🏁 Script executed:
# Check GitHub workflows for Zig version used
find .github -name "*.yml" -o -name "*.yaml" | xargs grep -l "zig" 2>/dev/null | head -3Repository: christianhelle/openapi2zig
Length of output: 156
🏁 Script executed:
# Search for any mention of Zig version in workflows
find .github -name "*.yml" -o -name "*.yaml" -exec grep -n "zig\|0\.15" {} + 2>/dev/null | head -20Repository: christianhelle/openapi2zig
Length of output: 52
🏁 Script executed:
# Check the CI workflow for Zig version
cat .github/workflows/ci.ymlRepository: christianhelle/openapi2zig
Length of output: 4339
🏁 Script executed:
# Check if there's generated code examples in the repo or test fixtures
find . -name "*.zig" -path "*/generated/*" 2>/dev/null | head -5Repository: christianhelle/openapi2zig
Length of output: 90
🏁 Script executed:
# Check test files to see how generated code is tested
rg -l "stringify\|ArrayList" src/tests/ --type=zig | head -5Repository: christianhelle/openapi2zig
Length of output: 52
🏁 Script executed:
# Let's check if there are any usages of ArrayList.writer() with allocator in the codebase
rg "\.writer\(.*allocator" --type=zigRepository: christianhelle/openapi2zig
Length of output: 52
🌐 Web query:
Zig 0.15.2 ArrayList writer method allocator parameter signature
💡 Result:
In Zig 0.15.2, std.ArrayList(T) is the unmanaged ArrayList, so the writer method (only available for u8) takes an allocator explicitly:
pub fn writer(self: *std.ArrayList(u8), allocator: std.mem.Allocator) WriterCall-site example:
var w = arraylist.writer(allocator);Writer here is the old std.io.Writer-style writer, commonly used via .adaptToNewApi(...) when you need the new std.Io interfaces. [1]
Why it takes an allocator: in 0.15.x, ArrayList no longer stores an allocator internally; you pass it to any operation that may allocate (including writing/appending). [2]
Sources: [1] [3]
🏁 Script executed:
# Let's verify the exact generated code content by checking line 240 again
sed -n '238,241p' src/generators/unified/api_generator.zig | cat -ARepository: christianhelle/openapi2zig
Length of output: 532
🏁 Script executed:
# Check if this is the complete picture - look at context around the generated code
sed -n '235,245p' src/generators/unified/api_generator.zigRepository: christianhelle/openapi2zig
Length of output: 973
Fix the generated code to pass allocator to str.writer() on line 240.
In Zig 0.15.2, ArrayList(u8).writer() requires an allocator parameter. The generated code currently emits str.writer() without the allocator, which will cause a compilation error. Change it to str.writer(allocator).
Current code
try self.buffer.appendSlice(self.allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n");
The same issue exists in src/generators/v2.0/apigenerator.zig (line 227) and src/generators/v3.0/apigenerator.zig (line 210).
🤖 Prompt for AI Agents
In `@src/generators/unified/api_generator.zig` around lines 238 - 241, The
generated Zig code calls str.writer() without passing an allocator which fails
in Zig 0.15.2; update the generator call site that writes the stringify line
(the appendSlice invocation that emits "try std.json.stringify(requestBody, .{},
str.writer());") to instead emit "try std.json.stringify(requestBody, .{},
str.writer(allocator));". Apply the same change wherever this string is emitted
(notably in the apigenerator implementations that produce that line for unified,
v2.0, and v3.0) so the generated code calls str.writer(allocator) and compiles.
There was a problem hiding this comment.
Pull request overview
This PR upgrades the repository to target Zig 0.15.2 and adjusts codegen, converters, and tests to accommodate Zig 0.15 API changes (notably ArrayList allocator-aware APIs), while also addressing a server-description memory leak in the OpenAPI v3 converter.
Changes:
- Bump Zig version references across build metadata, CI/release workflows, devcontainer setup, and docs to 0.15.2.
- Update generators/tests to use Zig 0.15-compatible formatting and
ArrayListdeinit/append APIs. - Fix OpenAPI converter server handling by tracking/freing duplicated server descriptions.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/unified_converter_tests.zig | Switch debug printing from {any} to {} for errors. |
| src/tests/swagger_v2_tests.zig | Switch debug printing from {any} to {} for errors. |
| src/tests/openapi_v3_tests.zig | Switch debug printing from {any} to {} for errors. |
| src/tests/comprehensive_converter_tests.zig | Switch debug printing from {any} to {} for errors. |
| src/models/common/document.zig | Track and free allocated server descriptions in Server.deinit. |
| src/main.zig | Use {} error formatting when reporting generation failures. |
| src/generator.zig | Remove redundant seek, use {} error formatting, and deduplicate output-file writing logic. |
| src/generators/converters/openapi_converter.zig | Mark duplicated server fields as allocated to prevent leaks. |
| src/generators/unified/api_generator.zig | Update generated code to call ArrayList.deinit(allocator) in Zig 0.15. |
| src/generators/v3.0/apigenerator.zig | Update legacy generator to allocator-aware ArrayList APIs and deinit signatures. |
| src/generators/v2.0/modelgenerator.zig | Thread allocator through schema generation helpers for Zig 0.15 ArrayList API. |
| src/generators/v2.0/apigenerator.zig | Update legacy generator to allocator-aware ArrayList APIs and deinit signatures. |
| build.zig.zon | Raise .minimum_zig_version to 0.15.2. |
| README.md | Update stated Zig prerequisite/version badge to 0.15.2. |
| examples/PACKAGE.md | Update package publishing/docs to Zig 0.15.2. |
| docs/index.html | Update website Zig badge and dev-env prerequisite text to 0.15.2. |
| .github/workflows/ci.yml | Run CI with Zig 0.15.2. |
| .github/workflows/release.yml | Run release workflow with Zig 0.15.2. |
| .github/copilot-instructions.md | Update contributor instructions to Zig 0.15.2. |
| .devcontainer/devcontainer.json | Update devcontainer Zig feature version to 0.15.2. |
| .devcontainer/README.md | Update devcontainer docs to Zig 0.15.2. |
| for (params) |paramOrReference| { | ||
| const parameter = paramOrReference.parameter; | ||
| if (!std.mem.eql(u8, parameter.in_field, "path") and !std.mem.eql(u8, parameter.in_field, "body")) { |
There was a problem hiding this comment.
In generateImplementation, paramOrReference is a union(enum) (parameter vs reference), but the code unconditionally reads paramOrReference.parameter. If a $ref parameter appears, this will trigger an invalid union field access at runtime (or a safety trap in Debug/ReleaseSafe). Handle the union with a switch and either resolve references or skip them before accessing Parameter fields.
| try parts.append(allocator, " var str = std.ArrayList([]const u8){};\n"); | ||
| try parts.append(allocator, " defer str.deinit(allocator);\n\n"); | ||
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | ||
| try parts.append(allocator, " const body = try std.mem.join(allocator, \"\", str.items);\n"); | ||
| try parts.append(allocator, " defer allocator.free(body);\n\n"); |
There was a problem hiding this comment.
The generated request-body code uses std.ArrayList([]const u8) with std.json.stringify(..., str.writer()). stringify writes bytes, so the generated code should use a byte buffer (e.g., std.ArrayList(u8)) rather than an array of slices; otherwise the generated client code will not compile/work correctly.
| try parts.append(allocator, " var str = std.ArrayList([]const u8){};\n"); | |
| try parts.append(allocator, " defer str.deinit(allocator);\n\n"); | |
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | |
| try parts.append(allocator, " const body = try std.mem.join(allocator, \"\", str.items);\n"); | |
| try parts.append(allocator, " defer allocator.free(body);\n\n"); | |
| try parts.append(allocator, " var str = std.ArrayList(u8).init(allocator);\n"); | |
| try parts.append(allocator, " defer str.deinit();\n\n"); | |
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | |
| try parts.append(allocator, " const body = str.items;\n\n"); |
| try parts.append(allocator, " var str = std.ArrayList([]const u8){};\n"); | ||
| try parts.append(allocator, " defer str.deinit(allocator);\n\n"); | ||
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | ||
| try parts.append(allocator, " const body = try std.mem.join(allocator, \"\", str.items);\n"); | ||
| try parts.append(allocator, " defer allocator.free(body);\n\n"); |
There was a problem hiding this comment.
The generated request-body code uses std.ArrayList([]const u8) with std.json.stringify(..., str.writer()). Since stringify writes bytes, the generated code should use a byte buffer (e.g., std.ArrayList(u8)) rather than an array of slices; otherwise the generated client code will not compile/work correctly.
| try parts.append(allocator, " var str = std.ArrayList([]const u8){};\n"); | |
| try parts.append(allocator, " defer str.deinit(allocator);\n\n"); | |
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | |
| try parts.append(allocator, " const body = try std.mem.join(allocator, \"\", str.items);\n"); | |
| try parts.append(allocator, " defer allocator.free(body);\n\n"); | |
| try parts.append(allocator, " var str = std.ArrayList(u8){};\n"); | |
| try parts.append(allocator, " defer str.deinit(allocator);\n\n"); | |
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | |
| try parts.append(allocator, " const body = str.items;\n\n"); |
| fn convertServers(self: *OpenApiConverter, servers: []Server3) ![]Server { | ||
| var converted_servers = try self.allocator.alloc(Server, servers.len); | ||
| for (servers, 0..) |server, i| { | ||
| const url = try self.allocator.dupe(u8, server.url); | ||
| const description = if (server.description) |desc| try self.allocator.dupe(u8, desc) else null; | ||
| converted_servers[i] = Server{ .url = url, .description = description }; | ||
| converted_servers[i] = Server{ | ||
| .url = url, | ||
| .description = description, | ||
| ._url_allocated = true, | ||
| ._description_allocated = description != null, | ||
| }; | ||
| } |
There was a problem hiding this comment.
convertServers now duplicates both url and description, but if an allocation fails partway through the loop, previously-duped strings (and the converted_servers array) will leak because there is no errdefer cleanup for the partially-built slice. Add error-path cleanup that frees any already-allocated Server fields before returning the error.
|
|
||
| <div class="manual-setup-note"> | ||
| <strong>Prerequisites:</strong> Zig v0.15.1 or later | ||
| <strong>Prerequisites:</strong> Zig v0.15.2 or later |
There was a problem hiding this comment.
docs/index.html says "Zig v0.15.2 or later", but other updated docs in this PR state 0.15.2 is required exactly. This is inconsistent and may confuse contributors/users; align the website text with the project’s stated compatibility (or update the other docs to match if newer Zig versions are actually supported).
| <strong>Prerequisites:</strong> Zig v0.15.2 or later | |
| <strong>Prerequisites:</strong> Zig v0.15.1 (exact version required) |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@christianhelle I've opened a new pull request, #39, to work on those changes. Once the pull request is ready, I'll request review from you. |
Summary by CodeRabbit
Chores
Bug Fixes