Add YAML OpenAPI parsing#50
Merged
Merged
Conversation
Normalize YAML input to JSON so existing detection, parsing, conversion, and generation paths can handle OpenAPI and Swagger YAML specifications. Add YAML library helpers, regression tests, documentation, and a vendored Zig 0.16-compatible yaml parser build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughThis pull request adds comprehensive YAML support to the project by introducing a vendored YAML parsing library, implementing YAML-to-JSON conversion logic, exposing new public APIs for YAML parsing, and updating build configuration and documentation to reflect the expanded format support. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Generator as generator.zig
participant YamlLoader as yaml_loader.zig
participant JSONParser as JSON Parser<br/>(existing)
participant Detector as Version Detector<br/>(existing)
User->>Generator: generateCode(input_path: .yaml)
Generator->>YamlLoader: yamlToJson(yaml_content)
YamlLoader->>YamlLoader: Preprocess YAML text<br/>(normalize scalars, keys)
YamlLoader->>YamlLoader: Parse YAML to nodes<br/>(Tokenizer + Parser)
YamlLoader->>YamlLoader: Serialize nodes to JSON
YamlLoader-->>Generator: json_bytes
Generator->>Detector: Detect version from JSON
Detector-->>Generator: ApiVersion
Generator->>JSONParser: Parse JSON to document<br/>(based on version)
JSONParser-->>Generator: Parsed Document
Generator-->>User: Generated code
sequenceDiagram
participant Caller as Library Caller
participant LibAPI as lib.zig<br/>(new YAML functions)
participant YamlLoader as yaml_loader.zig
participant JSONParser as Existing JSON<br/>Parser
Caller->>LibAPI: parseOpenApiYaml(yaml_content)
LibAPI->>YamlLoader: yamlToJson(yaml_content)
YamlLoader-->>LibAPI: json_bytes
LibAPI->>JSONParser: parseFromJson(json_bytes)
JSONParser-->>LibAPI: OpenApiDocument
LibAPI->>LibAPI: Defer free(json_bytes)
LibAPI-->>Caller: Parsed document
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class YAML support to openapi2zig by converting YAML OpenAPI/Swagger specs into normalized JSON, then reusing the existing JSON detection/parsing/conversion/codegen pipeline.
Changes:
- Introduces
src/yaml_loader.zigto load YAML, normalize common OpenAPI YAML patterns, and stringify to JSON. - Routes
.yaml/.ymlinputs through the generator and exposes YAML helpers via the library API. - Vendors
zig-yaml, wires it into the build, and adds YAML-focused regression tests + README updates.
Reviewed changes
Copilot reviewed 8 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/zig-yaml/test/test.zig | Adds upstream-style file-based tests for the vendored YAML library. |
| vendor/zig-yaml/test/spec.zig | Adds YAML spec-suite test generator plumbing for the vendored library. |
| vendor/zig-yaml/test/single_lib.tbd | Test fixture for YAML parsing (tbd format). |
| vendor/zig-yaml/test/simple.yaml | Test fixture for YAML parsing basics. |
| vendor/zig-yaml/test/multi_lib.tbd | Multi-document YAML test fixture (tbd format). |
| vendor/zig-yaml/src/stringify.zig | Adds YAML value stringification helper. |
| vendor/zig-yaml/src/lib.zig | Defines the vendored library’s public module surface. |
| vendor/zig-yaml/src/Yaml/test.zig | Adds extensive YAML library tests (typed parsing, stringify, etc.). |
| vendor/zig-yaml/src/Yaml.zig | Core YAML load/parse/stringify implementation (vendored). |
| vendor/zig-yaml/src/Tree.zig | YAML parse tree representation used by the vendored parser. |
| vendor/zig-yaml/src/Tokenizer.zig | YAML tokenizer implementation + tokenizer tests (vendored). |
| vendor/zig-yaml/src/Parser/test.zig | Parser-focused tests for the vendored YAML parser. |
| vendor/zig-yaml/src/Parser.zig | YAML parser implementation + parser test imports (vendored). |
| vendor/zig-yaml/build.zig.zon | Declares vendored zig-yaml package metadata (Zig 0.16). |
| vendor/zig-yaml/build.zig | Build script for vendored zig-yaml (module + test step). |
| vendor/zig-yaml/README.md | Documentation for the vendored YAML library. |
| vendor/zig-yaml/LICENSE | License for vendored zig-yaml. |
| src/yaml_loader.zig | New YAML-to-JSON normalization layer used by generator/library APIs. |
| src/tests/yaml_loader_tests.zig | Regression tests for YAML loading, version detection, and unified conversion. |
| src/tests.zig | Registers the new YAML loader tests in the test aggregator. |
| src/lib.zig | Exposes yamlToJson and YAML-based detect/parse helpers via the library API. |
| src/generator.zig | Enables .yaml/.yml inputs by normalizing YAML to JSON before generation. |
| build.zig.zon | Adds the local vendored yaml dependency and includes vendor/openapi in package paths. |
| build.zig | Wires the yaml module import into library, CLI, and test builds. |
| README.md | Updates docs to state YAML support and documents YAML usage in CLI/library APIs. |
Comment on lines
+415
to
+419
| if (std.ascii.eqlIgnoreCase(value, "true")) return true; | ||
| if (std.ascii.eqlIgnoreCase(value, "false")) return false; | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
parseBoolScalar only recognizes true/false. Common YAML boolean spellings like yes/no, on/off, y/n (which appear in real-world OpenAPI YAML and are supported by the vendored yaml parser’s typed boolean parsing) will currently be emitted as JSON strings instead of JSON booleans, which can break downstream OpenAPI parsing. Expand boolean normalization to cover these spellings and add a regression test for at least NO/TRUE variants.
Suggested change
| if (std.ascii.eqlIgnoreCase(value, "true")) return true; | |
| if (std.ascii.eqlIgnoreCase(value, "false")) return false; | |
| return null; | |
| } | |
| if (std.ascii.eqlIgnoreCase(value, "true") or | |
| std.ascii.eqlIgnoreCase(value, "yes") or | |
| std.ascii.eqlIgnoreCase(value, "on") or | |
| std.ascii.eqlIgnoreCase(value, "y")) | |
| { | |
| return true; | |
| } | |
| if (std.ascii.eqlIgnoreCase(value, "false") or | |
| std.ascii.eqlIgnoreCase(value, "no") or | |
| std.ascii.eqlIgnoreCase(value, "off") or | |
| std.ascii.eqlIgnoreCase(value, "n")) | |
| { | |
| return false; | |
| } | |
| return null; | |
| } | |
| test "parseBoolScalar supports common YAML boolean spellings" { | |
| try std.testing.expectEqual(@as(?bool, false), parseBoolScalar("NO")); | |
| try std.testing.expectEqual(@as(?bool, true), parseBoolScalar("TRUE")); | |
| try std.testing.expectEqual(@as(?bool, true), parseBoolScalar("on")); | |
| try std.testing.expectEqual(@as(?bool, false), parseBoolScalar("n")); | |
| try std.testing.expectEqual(@as(?bool, null), parseBoolScalar("maybe")); | |
| } |
Comment on lines
+249
to
+253
| if (key_end >= line.len or key_end + 1 >= line.len or line[key_end + 1] != ':') { | ||
| try out.appendSlice(allocator, line); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Quoted map-key normalization only triggers when the closing quote is immediately followed by : (e.g. '200':). Valid YAML like '200' : (whitespace before the colon) will be left unchanged and then rejected by the yaml parser (since it requires .literal keys). Consider allowing optional whitespace between the closing quote and : so more real-world OpenAPI/Swagger YAML parses successfully.
Suggested change
| if (key_end >= line.len or key_end + 1 >= line.len or line[key_end + 1] != ':') { | |
| try out.appendSlice(allocator, line); | |
| return; | |
| } | |
| if (key_end >= line.len) { | |
| try out.appendSlice(allocator, line); | |
| return; | |
| } | |
| var colon_index = key_end + 1; | |
| while (colon_index < line.len and (line[colon_index] == ' ' or line[colon_index] == '\t')) : (colon_index += 1) {} | |
| if (colon_index >= line.len or line[colon_index] != ':') { | |
| try out.appendSlice(allocator, line); | |
| return; | |
| } |
| }, | ||
| else => { | ||
| std.debug.print("Unsupported OpenAPI version: {s}\n", .{detector.getOpenApiVersionString(version)}); | ||
| return GeneratorErrors.UnsupportedExtension; |
There was a problem hiding this comment.
The CLI returns UnsupportedExtension when the OpenAPI version is unsupported. This error type/message is misleading (the extension may be valid) and makes it harder for callers to distinguish “unsupported spec version” from “unsupported file type”. Consider introducing a dedicated UnsupportedVersion error (and/or updating the printed message) for this branch.
Suggested change
| return GeneratorErrors.UnsupportedExtension; | |
| return error.UnsupportedVersion; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/generator.zig (1)
21-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReturn a version-specific error here.
This branch reports an unsupported OpenAPI version but returns
GeneratorErrors.UnsupportedExtension, which conflates two different failure modes for callers ofgenerateCode.Suggested fix
-const GeneratorErrors = error{UnsupportedExtension}; +const GeneratorErrors = error{ + UnsupportedExtension, + UnsupportedOpenApiVersion, +}; ... else => { std.debug.print("Unsupported OpenAPI version: {s}\n", .{detector.getOpenApiVersionString(version)}); - return GeneratorErrors.UnsupportedExtension; + return GeneratorErrors.UnsupportedOpenApiVersion; },Also applies to: 101-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generator.zig` at line 21, The code uses GeneratorErrors.UnsupportedExtension for an unsupported OpenAPI version branch; add a new error variant like UnsupportedOpenAPIVersion to the GeneratorErrors error set and return that variant from the version-checking branches (where currently UnsupportedExtension is used) so callers of generateCode can distinguish version errors from extension errors; update all occurrences referenced (including the other instances around the 101-103 area) to return the new UnsupportedOpenAPIVersion instead of UnsupportedExtension.README.md (1)
578-584:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove YAML from planned features - it's now implemented.
Line 580 lists "YAML specification format support" as a planned feature, but this PR implements that feature. This creates an inconsistency with the rest of the README which documents YAML support as available.
📝 Proposed fix
Planned features and enhancements: -- YAML specification format support - Enhanced authentication/authorization client support - Automatic API documentation generation - Performance optimizations for large specifications🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 578 - 584, Remove the now-implemented "YAML specification format support" entry from the "Planned features and enhancements:" list in README.md (the bullet that exactly reads "YAML specification format support") so the README no longer lists YAML as planned; ensure any nearby text or bullets remain properly formatted and that YAML support is instead documented in the existing "Available features" or relevant section if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/yaml_loader.zig`:
- Around line 315-340: The function writeJsonScalar currently reinterprets
scalar text as null/boolean/number using isNullScalar, parseBoolScalar, and
numeric schema heuristics even though yaml.Yaml.Value no longer preserves
whether the source was quoted, causing quoted strings like "false" to be
coerced; change writeJsonScalar to stop guessing types: remove or disable the
early branches that call isNullScalar, parseBoolScalar, and the numeric-schema
conversions (the checks using isNullScalar, parseBoolScalar,
isFloatSchemaKeyword/isIntegerSchemaKeyword/hasFractionOrExponent) and always
call std.json.Stringify.value(scalar, .{}, writer) unless you have explicit,
reliable metadata that proves the scalar was unquoted (in which case gate the
conversions behind that metadata flag); keep references to the existing helpers
(isNullScalar, parseBoolScalar, isFloatSchemaKeyword, isIntegerSchemaKeyword,
hasFractionOrExponent) only if you implement and check an "originally_unquoted"
indicator before performing any non-string conversions.
- Around line 135-140: The appendYamlDoubleQuotedChar function fails to escape
backslashes, causing sequences like C:\temp to be misinterpreted; update the
switch in appendYamlDoubleQuotedChar to handle the backslash character ('\\')
and append an escaped backslash sequence (i.e. two backslashes) via
out.appendSlice(allocator, "\\\\") so raw '\' are emitted as '\\' in the
synthetic double-quoted YAML string.
- Around line 71-75: The folded-block handling currently discards blank lines
for header.style '>' causing paragraph breaks to be lost; inside the block
processing where trimmed_block_line.len == 0 (the same place that calls
appendYamlDoubleQuotedChar for '|' literal blocks), detect header.style == '>'
and emit a paragraph break (two newlines) into out instead of a single newline
or discarding it — e.g. call appendYamlDoubleQuotedChar twice (or otherwise
append "\n\n") when !first_block_line and header.style == '>' so blank lines
become preserved paragraph breaks; keep using the existing
allocator/out/appendYamlDoubleQuotedChar and update first_block_line/line_index
handling consistently.
In `@vendor/zig-yaml/README.md`:
- Around line 79-85: The snippet uses untyped but never declares it; change the
first line to capture the result of yaml.load into the untyped variable (e.g.,
replace "try yaml.load(gpa, source);" with "const untyped = try yaml.load(gpa,
source);" so subsequent references to untyped, untyped.docs, and map operate on
the loaded value.
- Around line 18-29: Update the three unlabeled fenced code blocks in the
README: add "sh" to the fences around the "zig fetch --save
https://github.com/kubkon/zig-yaml/archive/refs/tags/[RELEASE_VERSION].tar.gz"
and "zig fetch --save=yaml
https://github.com/kubkon/zig-yaml/archive/refs/tags/0.1.1.tar.gz" examples, and
add "zig" to the fence for the snippet beginning with the comment "// add that
code after \"b.installArtifact(exe)\" line" so the blocks are language-labeled
for proper syntax highlighting and linting.
In `@vendor/zig-yaml/test/test.zig`:
- Around line 111-190: LibTbd.eql is incomplete: it never compares uuids, uses
one-sided optional checks (so null on the parsed side can match non-null
expected), and tests elsewhere allow truncated parses by not asserting
collection lengths; update LibTbd.eql to (1) compare the uuid fields (both
sides) using mem.eql(u8, self.uuids, other.uuids) or equivalent, (2) make every
optional-field comparison symmetric by using orelse to fetch both sides and
returning false if either is missing (e.g., const o_x = other.x orelse return
false; const s_x = self.x orelse return false; then compare), and (3) ensure
every collection length is explicitly compared (including top-level result
length in the multi-lib tbd test) so truncated/extra entries fail. Target the
eql function and the multi lib tbd test assertions referenced in the comment.
- Around line 40-60: The eql implementation in pub fn eql(self: `@This`(), other:
`@This`()) bool is missing comparisons for the boolean fields, so add explicit
equality checks for self.nested.ok, self.isyaml, and self.hasBoolean (e.g.
return false if any of those differ) alongside the existing checks for names,
numbers, finally, and nested string fields to ensure boolean regressions are
detected.
---
Outside diff comments:
In `@README.md`:
- Around line 578-584: Remove the now-implemented "YAML specification format
support" entry from the "Planned features and enhancements:" list in README.md
(the bullet that exactly reads "YAML specification format support") so the
README no longer lists YAML as planned; ensure any nearby text or bullets remain
properly formatted and that YAML support is instead documented in the existing
"Available features" or relevant section if necessary.
In `@src/generator.zig`:
- Line 21: The code uses GeneratorErrors.UnsupportedExtension for an unsupported
OpenAPI version branch; add a new error variant like UnsupportedOpenAPIVersion
to the GeneratorErrors error set and return that variant from the
version-checking branches (where currently UnsupportedExtension is used) so
callers of generateCode can distinguish version errors from extension errors;
update all occurrences referenced (including the other instances around the
101-103 area) to return the new UnsupportedOpenAPIVersion instead of
UnsupportedExtension.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae71bffc-d7bf-4035-85b2-3ce1d8798d1e
📒 Files selected for processing (25)
README.mdbuild.zigbuild.zig.zonsrc/generator.zigsrc/lib.zigsrc/tests.zigsrc/tests/yaml_loader_tests.zigsrc/yaml_loader.zigvendor/zig-yaml/LICENSEvendor/zig-yaml/README.mdvendor/zig-yaml/build.zigvendor/zig-yaml/build.zig.zonvendor/zig-yaml/src/Parser.zigvendor/zig-yaml/src/Parser/test.zigvendor/zig-yaml/src/Tokenizer.zigvendor/zig-yaml/src/Tree.zigvendor/zig-yaml/src/Yaml.zigvendor/zig-yaml/src/Yaml/test.zigvendor/zig-yaml/src/lib.zigvendor/zig-yaml/src/stringify.zigvendor/zig-yaml/test/multi_lib.tbdvendor/zig-yaml/test/simple.yamlvendor/zig-yaml/test/single_lib.tbdvendor/zig-yaml/test/spec.zigvendor/zig-yaml/test/test.zig
Comment on lines
+71
to
+75
| if (trimmed_block_line.len == 0) { | ||
| if (!first_block_line and header.style == '|') try appendYamlDoubleQuotedChar(allocator, &out, '\n'); | ||
| line_index += 1; | ||
| continue; | ||
| } |
Contributor
There was a problem hiding this comment.
Folded > block scalars are losing paragraph breaks.
Blank lines are discarded for folded blocks, so input like first\n\nsecond is normalized to first second. That silently changes description/example content before version detection and model parsing.
Suggested fix
- if (trimmed_block_line.len == 0) {
- if (!first_block_line and header.style == '|') try appendYamlDoubleQuotedChar(allocator, &out, '\n');
+ if (trimmed_block_line.len == 0) {
+ if (!first_block_line) try appendYamlDoubleQuotedChar(allocator, &out, '\n');
+ first_block_line = false;
line_index += 1;
continue;
}Also applies to: 80-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/yaml_loader.zig` around lines 71 - 75, The folded-block handling
currently discards blank lines for header.style '>' causing paragraph breaks to
be lost; inside the block processing where trimmed_block_line.len == 0 (the same
place that calls appendYamlDoubleQuotedChar for '|' literal blocks), detect
header.style == '>' and emit a paragraph break (two newlines) into out instead
of a single newline or discarding it — e.g. call appendYamlDoubleQuotedChar
twice (or otherwise append "\n\n") when !first_block_line and header.style ==
'>' so blank lines become preserved paragraph breaks; keep using the existing
allocator/out/appendYamlDoubleQuotedChar and update first_block_line/line_index
handling consistently.
Comment on lines
+135
to
+140
| fn appendYamlDoubleQuotedChar(allocator: std.mem.Allocator, out: *std.ArrayList(u8), char: u8) !void { | ||
| switch (char) { | ||
| '\n' => try out.appendSlice(allocator, "\\n"), | ||
| '\t' => try out.appendSlice(allocator, "\\t"), | ||
| '"' => try out.appendSlice(allocator, "\\\""), | ||
| else => try out.append(allocator, char), |
Contributor
There was a problem hiding this comment.
Escape backslashes when re-emitting block scalars.
This helper converts block scalar content into a synthetic double-quoted YAML string, but raw \ is written through unchanged. Values like C:\temp\foo or Markdown escapes are then reinterpreted by the YAML parser and the emitted JSON changes.
Suggested fix
switch (char) {
'\n' => try out.appendSlice(allocator, "\\n"),
'\t' => try out.appendSlice(allocator, "\\t"),
'"' => try out.appendSlice(allocator, "\\\""),
+ '\\' => try out.appendSlice(allocator, "\\\\"),
else => try out.append(allocator, char),
}📝 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
| fn appendYamlDoubleQuotedChar(allocator: std.mem.Allocator, out: *std.ArrayList(u8), char: u8) !void { | |
| switch (char) { | |
| '\n' => try out.appendSlice(allocator, "\\n"), | |
| '\t' => try out.appendSlice(allocator, "\\t"), | |
| '"' => try out.appendSlice(allocator, "\\\""), | |
| else => try out.append(allocator, char), | |
| fn appendYamlDoubleQuotedChar(allocator: std.mem.Allocator, out: *std.ArrayList(u8), char: u8) !void { | |
| switch (char) { | |
| '\n' => try out.appendSlice(allocator, "\\n"), | |
| '\t' => try out.appendSlice(allocator, "\\t"), | |
| '"' => try out.appendSlice(allocator, "\\\""), | |
| '\\' => try out.appendSlice(allocator, "\\\\"), | |
| else => try out.append(allocator, char), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/yaml_loader.zig` around lines 135 - 140, The appendYamlDoubleQuotedChar
function fails to escape backslashes, causing sequences like C:\temp to be
misinterpreted; update the switch in appendYamlDoubleQuotedChar to handle the
backslash character ('\\') and append an escaped backslash sequence (i.e. two
backslashes) via out.appendSlice(allocator, "\\\\") so raw '\' are emitted as
'\\' in the synthetic double-quoted YAML string.
Comment on lines
+315
to
+340
| fn writeJsonScalar(writer: *std.Io.Writer, scalar: []const u8, current_key: ?[]const u8) !void { | ||
| const trimmed = std.mem.trim(u8, scalar, " \t\r\n"); | ||
|
|
||
| if (isNullScalar(trimmed)) { | ||
| try writer.writeAll("null"); | ||
| return; | ||
| } | ||
|
|
||
| if (parseBoolScalar(trimmed)) |boolean| { | ||
| try writer.writeAll(if (boolean) "true" else "false"); | ||
| return; | ||
| } | ||
|
|
||
| if (current_key) |key| { | ||
| if (isFloatSchemaKeyword(key) and isJsonNumber(trimmed)) { | ||
| try writer.writeAll(trimmed); | ||
| if (!hasFractionOrExponent(trimmed)) try writer.writeAll(".0"); | ||
| return; | ||
| } | ||
| if (isIntegerSchemaKeyword(key) and isJsonNumber(trimmed)) { | ||
| try writer.writeAll(trimmed); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| try std.json.Stringify.value(scalar, .{}, writer); |
Contributor
There was a problem hiding this comment.
Quoted scalars cannot be safely retyped in this layer.
By the time this code runs, it only has yaml.Yaml.Value, so .scalar no longer tells you whether the source was false or 'false'. The isNullScalar / parseBoolScalar coercion therefore turns quoted string defaults, examples, and enums into JSON null/booleans.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/yaml_loader.zig` around lines 315 - 340, The function writeJsonScalar
currently reinterprets scalar text as null/boolean/number using isNullScalar,
parseBoolScalar, and numeric schema heuristics even though yaml.Yaml.Value no
longer preserves whether the source was quoted, causing quoted strings like
"false" to be coerced; change writeJsonScalar to stop guessing types: remove or
disable the early branches that call isNullScalar, parseBoolScalar, and the
numeric-schema conversions (the checks using isNullScalar, parseBoolScalar,
isFloatSchemaKeyword/isIntegerSchemaKeyword/hasFractionOrExponent) and always
call std.json.Stringify.value(scalar, .{}, writer) unless you have explicit,
reliable metadata that proves the scalar was unquoted (in which case gate the
conversions behind that metadata flag); keep references to the existing helpers
(isNullScalar, parseBoolScalar, isFloatSchemaKeyword, isIntegerSchemaKeyword,
hasFractionOrExponent) only if you implement and check an "originally_unquoted"
indicator before performing any non-string conversions.
Comment on lines
+18
to
+29
| ``` | ||
| zig fetch --save https://github.com/kubkon/zig-yaml/archive/refs/tags/[RELEASE_VERSION].tar.gz | ||
| ``` | ||
|
|
||
| It's more convenient to save the library with a desired name, for example, like this (assuming you are targeting latest release of Zig): | ||
| ``` | ||
| zig fetch --save=yaml https://github.com/kubkon/zig-yaml/archive/refs/tags/0.1.1.tar.gz | ||
| ``` | ||
|
|
||
| And then add those lines to your project's `build.zig` file: | ||
| ``` | ||
| // add that code after "b.installArtifact(exe)" line |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Lines 18, 23, and 28 use unlabeled fences, which triggers markdown lint and reduces readability.
Proposed update
-```
+```sh
zig fetch --save https://github.com/kubkon/zig-yaml/archive/refs/tags/[RELEASE_VERSION].tar.gz@@
- +sh
zig fetch --save=yaml https://github.com/kubkon/zig-yaml/archive/refs/tags/0.1.1.tar.gz
@@
-```
+```zig
// add that code after "b.installArtifact(exe)" line
const yaml = b.dependency("yaml", .{
.target = target,
📝 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
| ``` | |
| zig fetch --save https://github.com/kubkon/zig-yaml/archive/refs/tags/[RELEASE_VERSION].tar.gz | |
| ``` | |
| It's more convenient to save the library with a desired name, for example, like this (assuming you are targeting latest release of Zig): | |
| ``` | |
| zig fetch --save=yaml https://github.com/kubkon/zig-yaml/archive/refs/tags/0.1.1.tar.gz | |
| ``` | |
| And then add those lines to your project's `build.zig` file: | |
| ``` | |
| // add that code after "b.installArtifact(exe)" line |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/zig-yaml/README.md` around lines 18 - 29, Update the three unlabeled
fenced code blocks in the README: add "sh" to the fences around the "zig fetch
--save
https://github.com/kubkon/zig-yaml/archive/refs/tags/[RELEASE_VERSION].tar.gz"
and "zig fetch --save=yaml
https://github.com/kubkon/zig-yaml/archive/refs/tags/0.1.1.tar.gz" examples, and
add "zig" to the fence for the snippet beginning with the comment "// add that
code after \"b.installArtifact(exe)\" line" so the blocks are language-labeled
for proper syntax highlighting and linting.
Comment on lines
+79
to
+85
| try yaml.load(gpa, source); | ||
|
|
||
| try std.testing.expectEqual(untyped.docs.items.len, 1); | ||
|
|
||
| const map = untyped.docs.items[0].map; | ||
| try std.testing.expect(map.contains("names")); | ||
| try std.testing.expectEqual(map.get("names").?.list.len, 3); |
Contributor
There was a problem hiding this comment.
Fix undefined variable in the untyped example.
Line 81 references untyped without declaration in the snippet; this will not compile as shown.
Proposed update
-try std.testing.expectEqual(untyped.docs.items.len, 1);
+try std.testing.expectEqual(yaml.docs.items.len, 1);
@@
-const map = untyped.docs.items[0].map;
+const map = yaml.docs.items[0].map;📝 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
| try yaml.load(gpa, source); | |
| try std.testing.expectEqual(untyped.docs.items.len, 1); | |
| const map = untyped.docs.items[0].map; | |
| try std.testing.expect(map.contains("names")); | |
| try std.testing.expectEqual(map.get("names").?.list.len, 3); | |
| try yaml.load(gpa, source); | |
| try std.testing.expectEqual(yaml.docs.items.len, 1); | |
| const map = yaml.docs.items[0].map; | |
| try std.testing.expect(map.contains("names")); | |
| try std.testing.expectEqual(map.get("names").?.list.len, 3); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/zig-yaml/README.md` around lines 79 - 85, The snippet uses untyped but
never declares it; change the first line to capture the result of yaml.load into
the untyped variable (e.g., replace "try yaml.load(gpa, source);" with "const
untyped = try yaml.load(gpa, source);" so subsequent references to untyped,
untyped.docs, and map operate on the loaded value.
Comment on lines
+40
to
+60
| pub fn eql(self: @This(), other: @This()) bool { | ||
| if (self.names.len != other.names.len) return false; | ||
| if (self.numbers.len != other.numbers.len) return false; | ||
| if (self.finally.len != other.finally.len) return false; | ||
|
|
||
| for (self.names, 0..) |lhs, i| { | ||
| if (!mem.eql(u8, lhs, other.names[i])) return false; | ||
| } | ||
|
|
||
| for (self.numbers, 0..) |lhs, i| { | ||
| if (lhs != other.numbers[i]) return false; | ||
| } | ||
|
|
||
| for (self.finally, 0..) |lhs, i| { | ||
| if (lhs != other.finally[i]) return false; | ||
| } | ||
|
|
||
| if (!mem.eql(u8, self.nested.some, other.nested.some)) return false; | ||
| if (!mem.eql(u8, self.nested.wick, other.nested.wick)) return false; | ||
|
|
||
| return true; |
Contributor
There was a problem hiding this comment.
Simple.eql is not validating the boolean fields.
A parser regression on nested.ok, isyaml, or hasBoolean still passes this test today because those fields are never compared.
Suggested fix
pub fn eql(self: `@This`(), other: `@This`()) bool {
if (self.names.len != other.names.len) return false;
if (self.numbers.len != other.numbers.len) return false;
if (self.finally.len != other.finally.len) return false;
@@
if (!mem.eql(u8, self.nested.some, other.nested.some)) return false;
if (!mem.eql(u8, self.nested.wick, other.nested.wick)) return false;
+ if (self.nested.ok != other.nested.ok) return false;
+ if (self.isyaml != other.isyaml) return false;
+ if (self.hasBoolean != other.hasBoolean) return false;
return true;
}📝 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 eql(self: @This(), other: @This()) bool { | |
| if (self.names.len != other.names.len) return false; | |
| if (self.numbers.len != other.numbers.len) return false; | |
| if (self.finally.len != other.finally.len) return false; | |
| for (self.names, 0..) |lhs, i| { | |
| if (!mem.eql(u8, lhs, other.names[i])) return false; | |
| } | |
| for (self.numbers, 0..) |lhs, i| { | |
| if (lhs != other.numbers[i]) return false; | |
| } | |
| for (self.finally, 0..) |lhs, i| { | |
| if (lhs != other.finally[i]) return false; | |
| } | |
| if (!mem.eql(u8, self.nested.some, other.nested.some)) return false; | |
| if (!mem.eql(u8, self.nested.wick, other.nested.wick)) return false; | |
| return true; | |
| pub fn eql(self: `@This`(), other: `@This`()) bool { | |
| if (self.names.len != other.names.len) return false; | |
| if (self.numbers.len != other.numbers.len) return false; | |
| if (self.finally.len != other.finally.len) return false; | |
| for (self.names, 0..) |lhs, i| { | |
| if (!mem.eql(u8, lhs, other.names[i])) return false; | |
| } | |
| for (self.numbers, 0..) |lhs, i| { | |
| if (lhs != other.numbers[i]) return false; | |
| } | |
| for (self.finally, 0..) |lhs, i| { | |
| if (lhs != other.finally[i]) return false; | |
| } | |
| if (!mem.eql(u8, self.nested.some, other.nested.some)) return false; | |
| if (!mem.eql(u8, self.nested.wick, other.nested.wick)) return false; | |
| if (self.nested.ok != other.nested.ok) return false; | |
| if (self.isyaml != other.isyaml) return false; | |
| if (self.hasBoolean != other.hasBoolean) return false; | |
| return true; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/zig-yaml/test/test.zig` around lines 40 - 60, The eql implementation
in pub fn eql(self: `@This`(), other: `@This`()) bool is missing comparisons for the
boolean fields, so add explicit equality checks for self.nested.ok, self.isyaml,
and self.hasBoolean (e.g. return false if any of those differ) alongside the
existing checks for names, numbers, finally, and nested string fields to ensure
boolean regressions are detected.
Comment on lines
+111
to
+190
| pub fn eql(self: LibTbd, other: LibTbd) bool { | ||
| if (self.tbd_version != other.tbd_version) return false; | ||
| if (self.targets.len != other.targets.len) return false; | ||
|
|
||
| for (self.targets, 0..) |target, i| { | ||
| if (!mem.eql(u8, target, other.targets[i])) return false; | ||
| } | ||
|
|
||
| if (!mem.eql(u8, self.install_name, other.install_name)) return false; | ||
|
|
||
| switch (self.current_version) { | ||
| .string => |string| { | ||
| if (other.current_version != .string) return false; | ||
| if (!mem.eql(u8, string, other.current_version.string)) return false; | ||
| }, | ||
| .int => |int| { | ||
| if (other.current_version != .int) return false; | ||
| if (int != other.current_version.int) return false; | ||
| }, | ||
| } | ||
|
|
||
| if (self.reexported_libraries) |reexported_libraries| { | ||
| const o_reexported_libraries = other.reexported_libraries orelse return false; | ||
|
|
||
| if (reexported_libraries.len != o_reexported_libraries.len) return false; | ||
|
|
||
| for (reexported_libraries, 0..) |reexport, i| { | ||
| const o_reexport = o_reexported_libraries[i]; | ||
| if (reexport.targets.len != o_reexport.targets.len) return false; | ||
| if (reexport.libraries.len != o_reexport.libraries.len) return false; | ||
|
|
||
| for (reexport.targets, 0..) |target, j| { | ||
| const o_target = o_reexport.targets[j]; | ||
| if (!mem.eql(u8, target, o_target)) return false; | ||
| } | ||
|
|
||
| for (reexport.libraries, 0..) |library, j| { | ||
| const o_library = o_reexport.libraries[j]; | ||
| if (!mem.eql(u8, library, o_library)) return false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (self.parent_umbrella) |parent_umbrella| { | ||
| const o_parent_umbrella = other.parent_umbrella orelse return false; | ||
|
|
||
| if (parent_umbrella.len != o_parent_umbrella.len) return false; | ||
|
|
||
| for (parent_umbrella, 0..) |pumbrella, i| { | ||
| const o_pumbrella = o_parent_umbrella[i]; | ||
| if (pumbrella.targets.len != o_pumbrella.targets.len) return false; | ||
|
|
||
| for (pumbrella.targets, 0..) |target, j| { | ||
| const o_target = o_pumbrella.targets[j]; | ||
| if (!mem.eql(u8, target, o_target)) return false; | ||
| } | ||
|
|
||
| if (!mem.eql(u8, pumbrella.umbrella, o_pumbrella.umbrella)) return false; | ||
| } | ||
| } | ||
|
|
||
| if (self.exports.len != other.exports.len) return false; | ||
|
|
||
| for (self.exports, 0..) |exp, i| { | ||
| const o_exp = other.exports[i]; | ||
| if (exp.targets.len != o_exp.targets.len) return false; | ||
| if (exp.symbols.len != o_exp.symbols.len) return false; | ||
|
|
||
| for (exp.targets, 0..) |target, j| { | ||
| const o_target = o_exp.targets[j]; | ||
| if (!mem.eql(u8, target, o_target)) return false; | ||
| } | ||
|
|
||
| for (exp.symbols, 0..) |symbol, j| { | ||
| const o_symbol = o_exp.symbols[j]; | ||
| if (!mem.eql(u8, symbol, o_symbol)) return false; | ||
| } | ||
| } | ||
|
|
||
| return true; |
Contributor
There was a problem hiding this comment.
LibTbd assertions still allow malformed results to pass.
LibTbd.eql never compares uuids, and its optional checks are one-sided, so result.eql(expected) treats null on the parsed side as equal to a non-null expected value. The multi lib tbd test also never asserts result.len == expected.len, so a truncated parse can still pass.
Also applies to: 277-331
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/zig-yaml/test/test.zig` around lines 111 - 190, LibTbd.eql is
incomplete: it never compares uuids, uses one-sided optional checks (so null on
the parsed side can match non-null expected), and tests elsewhere allow
truncated parses by not asserting collection lengths; update LibTbd.eql to (1)
compare the uuid fields (both sides) using mem.eql(u8, self.uuids, other.uuids)
or equivalent, (2) make every optional-field comparison symmetric by using
orelse to fetch both sides and returning false if either is missing (e.g., const
o_x = other.x orelse return false; const s_x = self.x orelse return false; then
compare), and (3) ensure every collection length is explicitly compared
(including top-level result length in the multi-lib tbd test) so truncated/extra
entries fail. Target the eql function and the multi lib tbd test assertions
referenced in the comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OpenAPI and Swagger specs are often published as YAML, but the generator only accepted JSON despite allowing YAML file extensions. This adds YAML support by normalizing YAML input to JSON before the existing detection, parsing, conversion, and code generation pipeline runs.
Summary
.yamland.ymlgeneration through the existing JSON-based pipeline for OpenAPI 3.x and Swagger 2.0.zig-yamlparser build so the dependency is reproducible.Validation
zig fmt --check src\yaml_loader.zig src\tests\yaml_loader_tests.zig build.zig build.zig.zon vendor\zig-yaml\build.zigzig build -Doptimize=Debugzig build testzig run generated\main.zigSummary by CodeRabbit
New Features
--inputflag now accepts YAML filesDocumentation