Add YAML parsing and module imports#166
Merged
Merged
Conversation
- Add YAML parser support and `YAML.parse`/`parseDocuments` - Allow `.yaml`/`.yml` globals and module imports - Document YAML behavior and add parser coverage tests
Contributor
📝 WalkthroughWalkthroughThis PR adds comprehensive YAML parsing support to GocciaScript, including new built-in Changes
Sequence DiagramsequenceDiagram
participant Client
participant Engine
participant YAMLParser
participant ModuleLoader
participant FileSystem
Client->>Engine: YAML.parse(text) or YAML.parseDocuments(text)
Engine->>YAMLParser: TGocciaYAMLParser.Parse/ParseDocuments(text)
YAMLParser->>YAMLParser: Parse YAML constructs (scalars, sequences, mappings, tags, aliases)
YAMLParser->>Engine: Return TGocciaValue or TGocciaArrayValue[]
Engine->>Client: Return parsed result
Note over Client,FileSystem: Parallel: YAML Module Import Flow
Client->>ModuleLoader: import './config.yaml'
ModuleLoader->>FileSystem: Read file (isStructuredDataExtension check)
FileSystem->>ModuleLoader: Return file content
ModuleLoader->>YAMLParser: ParseDocuments(content) for YAML file
YAMLParser->>ModuleLoader: Return TGocciaArrayValue with documents
ModuleLoader->>ModuleLoader: Validate single top-level document
ModuleLoader->>ModuleLoader: Extract own property keys as exports
ModuleLoader->>Client: Return module with exported bindings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Owner
Author
|
Literal-string named module support has been deferred as #165 |
Contributor
Benchmark Results263 benchmarks Interpreted: 🟢 30 improved · 🔴 122 regressed · 111 unchanged · avg -1.4% arraybuffer.js — Interp: 🟢 4, 🔴 6, 4 unch. · avg -0.8% · Bytecode: 🟢 13, 🔴 1 · avg +9.7%
arrays.js — Interp: 🟢 1, 🔴 8, 10 unch. · avg -0.7% · Bytecode: 🟢 17, 2 unch. · avg +8.4%
async-await.js — Interp: 🔴 6 · avg -3.8% · Bytecode: 🟢 6 · avg +8.1%
classes.js — Interp: 🔴 23, 8 unch. · avg -2.0% · Bytecode: 🟢 10, 🔴 4, 17 unch. · avg +0.7%
closures.js — Interp: 🟢 1, 🔴 5, 5 unch. · avg -0.8% · Bytecode: 🟢 2, 9 unch. · avg +2.3%
collections.js — Interp: 🟢 4, 🔴 5, 3 unch. · avg -3.0% · Bytecode: 🟢 10, 🔴 1, 1 unch. · avg +6.9%
destructuring.js — Interp: 🟢 1, 🔴 13, 8 unch. · avg -2.4% · Bytecode: 🟢 6, 🔴 8, 8 unch. · avg +0.4%
fibonacci.js — Interp: 🟢 2, 6 unch. · avg +0.5% · Bytecode: 🟢 5, 🔴 2, 1 unch. · avg +7.0%
for-of.js — Interp: 🟢 2, 5 unch. · avg +0.4% · Bytecode: 🟢 5, 2 unch. · avg +6.1%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🟢 9, 🔴 1, 10 unch. · avg +1.9% · Bytecode: 🟢 9, 🔴 3, 8 unch. · avg +2.3%
json.js — Interp: 🟢 2, 🔴 8, 10 unch. · avg -1.6% · Bytecode: 🟢 13, 7 unch. · avg +3.8%
jsx.jsx — Interp: 🔴 14, 7 unch. · avg -2.4% · Bytecode: 🟢 3, 🔴 6, 12 unch. · avg -1.0%
modules.js — Interp: 🔴 9 · avg -5.7% · Bytecode: 🟢 2, 🔴 5, 2 unch. · avg +0.3%
numbers.js — Interp: 🔴 4, 7 unch. · avg -1.2% · Bytecode: 🟢 8, 🔴 2, 1 unch. · avg +2.6%
objects.js — Interp: 🟢 1, 🔴 1, 5 unch. · avg -0.4% · Bytecode: 🟢 4, 3 unch. · avg +2.9%
promises.js — Interp: 🟢 1, 🔴 4, 7 unch. · avg -1.2% · Bytecode: 🟢 10, 2 unch. · avg +2.5%
strings.js — Interp: 🔴 5, 6 unch. · avg -1.7% · Bytecode: 🟢 11 · avg +6.4%
typed-arrays.js — Interp: 🟢 2, 🔴 10, 10 unch. · avg -1.2% · Bytecode: 🟢 14, 🔴 4, 4 unch. · avg +3.8%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
![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: 1
🧹 Nitpick comments (3)
tests/language/modules/helpers/tagged-values.yaml (1)
1-10: Consider adding explicit document start marker after directives.While YAML 1.2 allows content to follow directives without
---, some parsers (including YAMLlint) expect an explicit document start marker. Adding---after the%TAGdirective would improve compatibility without changing semantics.♻️ Suggested improvement
%YAML 1.2 %TAG !e! tag:example.com,2026: +--- name: !!str true count: !!int "42"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/language/modules/helpers/tagged-values.yaml` around lines 1 - 10, The YAML file currently places content directly after the %TAG directive which can confuse linters; add an explicit document start marker '---' immediately after the %TAG !e! tag:example.com,2026: directive so the document begins with '---' before the mapping (i.e., insert '---' following the %TAG directive to improve compatibility with parsers like YAMLlint).units/Goccia.FileExtensions.pas (1)
68-74: Minor: redundantLowerCasecall inIsStructuredDataExtension.
Extis already lowercased on line 72, butIsYAMLExtension(Ext)on line 73 will lowercase it again internally. This is functionally correct but slightly inefficient.♻️ Suggested optimization
function IsStructuredDataExtension(const AExtension: string): Boolean; var Ext: string; begin Ext := LowerCase(AExtension); - Result := (Ext = EXT_JSON) or IsYAMLExtension(Ext); + Result := (Ext = EXT_JSON) or (Ext = EXT_YAML) or (Ext = EXT_YML); end;Alternatively, add an internal variant of
IsYAMLExtensionthat assumes pre-lowercased input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.FileExtensions.pas` around lines 68 - 74, IsStructuredDataExtension lowercases AExtension then calls IsYAMLExtension which itself lowercases again; remove this redundant work by adding a lowercased-aware helper and using it: introduce a new function IsYAMLExtensionLower(const AExtensionLower: string): Boolean that assumes its input is already lowercased (or adapt IsYAMLExtension to call it), change IsStructuredDataExtension to compute Ext := LowerCase(AExtension) once and call IsYAMLExtensionLower(Ext) (and compare Ext to EXT_JSON), and update IsYAMLExtension to delegate to IsYAMLExtensionLower so other callers keep existing behavior.docs/testing.md (1)
231-231: Use consistent heading level for this section.Other test-related sections like "TestRunner Options" use
###for subsections. This section should follow the same pattern for consistency.-**Official YAML Suite** +### Official YAML Suite🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/testing.md` at line 231, The "Official YAML Suite" section is currently formatted as bold text ("**Official YAML Suite**") instead of a subsection heading; change it to the same subsection level used elsewhere by replacing the bold line with a level-3 heading ("### Official YAML Suite") so it matches the "TestRunner Options" and other `###` subsections and keeps heading hierarchy consistent in docs/testing.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/run_yaml_test_suite.py`:
- Around line 42-59: The Halt(0) inside the try block makes the subsequent
Parser.Free, Source.Free and TGarbageCollector.Shutdown unreachable; wrap the
resource lifecycle in a try..finally so Source (TStringList), Parser
(TGocciaYAMLParser) and any Documents are freed in the finally section
regardless of success or failure, and move the Halt calls out of the cleanup (or
call Halt after the finally). Specifically, ensure ParseDocuments/Documents
freeing happens inside the main try, catch exceptions to Writeln(E.Message) and
set exit code, and perform Parser.Free, Source.Free and
TGarbageCollector.Shutdown in the outer finally block so resources are always
released.
---
Nitpick comments:
In `@docs/testing.md`:
- Line 231: The "Official YAML Suite" section is currently formatted as bold
text ("**Official YAML Suite**") instead of a subsection heading; change it to
the same subsection level used elsewhere by replacing the bold line with a
level-3 heading ("### Official YAML Suite") so it matches the "TestRunner
Options" and other `###` subsections and keeps heading hierarchy consistent in
docs/testing.md.
In `@tests/language/modules/helpers/tagged-values.yaml`:
- Around line 1-10: The YAML file currently places content directly after the
%TAG directive which can confuse linters; add an explicit document start marker
'---' immediately after the %TAG !e! tag:example.com,2026: directive so the
document begins with '---' before the mapping (i.e., insert '---' following the
%TAG directive to improve compatibility with parsers like YAMLlint).
In `@units/Goccia.FileExtensions.pas`:
- Around line 68-74: IsStructuredDataExtension lowercases AExtension then calls
IsYAMLExtension which itself lowercases again; remove this redundant work by
adding a lowercased-aware helper and using it: introduce a new function
IsYAMLExtensionLower(const AExtensionLower: string): Boolean that assumes its
input is already lowercased (or adapt IsYAMLExtension to call it), change
IsStructuredDataExtension to compute Ext := LowerCase(AExtension) once and call
IsYAMLExtensionLower(Ext) (and compare Ext to EXT_JSON), and update
IsYAMLExtension to delegate to IsYAMLExtensionLower so other callers keep
existing behavior.
🪄 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: 6a1ee642-79e3-4ee5-8b15-76ab4b5b5327
📒 Files selected for processing (39)
README.mdScriptLoader.dprdocs/build-system.mddocs/built-ins.mddocs/design-decisions.mddocs/language-restrictions.mddocs/testing.mdscripts/run_yaml_test_suite.pytests/built-ins/YAML/parse.jstests/built-ins/YAML/parseDocuments.jstests/language/modules/helpers/alias-graph.yamltests/language/modules/helpers/block-scalars.yamltests/language/modules/helpers/config.yamltests/language/modules/helpers/explicit-keys.yamltests/language/modules/helpers/merged.yamltests/language/modules/helpers/multiline-scalars.yamltests/language/modules/helpers/quoted-escapes.yamltests/language/modules/helpers/simple-values.ymltests/language/modules/helpers/tagged-values.yamltests/language/modules/helpers/yaml-re-exporter.jstests/language/modules/yaml-alias-graph.jstests/language/modules/yaml-block-scalars.jstests/language/modules/yaml-explicit-keys.jstests/language/modules/yaml-import-aliased.jstests/language/modules/yaml-import.jstests/language/modules/yaml-merge.jstests/language/modules/yaml-multiline-scalars.jstests/language/modules/yaml-quoted-escapes.jstests/language/modules/yaml-re-export.jstests/language/modules/yaml-tags.jsunits/Goccia.Builtins.YAML.pasunits/Goccia.Constants.PropertyNames.pasunits/Goccia.Engine.Backend.pasunits/Goccia.Engine.pasunits/Goccia.FileExtensions.pasunits/Goccia.Modules.Loader.pasunits/Goccia.Runtime.Bootstrap.pasunits/Goccia.ScriptLoader.Globals.pasunits/Goccia.YAML.pas
This was referenced Apr 4, 2026
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
1 participant
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.
Summary
YAML.parse(...)andYAML.parseDocuments(...)..yamland.ymlmodule imports, plus YAML-backed--globalsloading inScriptLoader.yaml-test-suiteparse-validity check.Testing
tests/built-ins/YAML/parse.jsandtests/built-ins/YAML/parseDocuments.js.tests/language/modules/for YAML files and re-exports.scripts/run_yaml_test_suite.pyfor local parse-validity verification against the upstream YAML suite.Summary by CodeRabbit
Release Notes
New Features
.yaml/.ymlfiles to be imported as named exports.YAML.parse()andYAML.parseDocuments()builtin functions for runtime YAML parsing with Bun-compatible semantics.--globals=fileCLI to accept both JSON and YAML globals files based on file extension.Documentation