Add nst/JSONTestSuite RFC 8259 conformance runner#2
Conversation
Adds JSONTestSuite as a git submodule and a test runner that validates parsing in strict mode (disallowComments: true). y_* must accept, n_* must reject, i_* recorded only. Known-lenient jsonic relaxations (leading zeros, unquoted keys, etc.) are pinned so future divergences in either direction are caught. https://claude.ai/code/session_01RfFhJAs67hitVXQyd1PMfE
MIT requires preserving copyright notices. Adds THIRD_PARTY_NOTICES.md, README Acknowledgments section, and source-file attribution headers for: - nst/JSONTestSuite (git submodule; its LICENSE travels in place) - microsoft/node-jsonc-parser (test cases ported into test/jsonc.test.ts) https://claude.ai/code/session_01RfFhJAs67hitVXQyd1PMfE
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a5508651f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (0 === files.length) { | ||
| test('suite unavailable', () => { | ||
| console.warn(`JSONTestSuite not found at ${SUITE_DIR} — skipping.`) | ||
| }) | ||
| return |
There was a problem hiding this comment.
Fail when JSONTestSuite corpus is unavailable
This branch turns a missing corpus into a passing test, which means the new conformance suite can silently never run. I checked the current CI workflow (.github/workflows/build.yml), and actions/checkout@v4 is used without submodule checkout, so test/JSONTestSuite is absent by default and files is always empty; as a result all y_*/n_* assertions are skipped and RFC 8259 regressions won’t be caught. The test should fail (or otherwise hard-require submodule initialization) when the suite directory is missing.
Useful? React with 👍 / 👎.
Replaces file:../jsonic devDependency (which only resolves locally) with github:jsonicjs/jsonic#main so CI can install. Also enables recursive submodule checkout in the build job so test/JSONTestSuite is populated before npm test. https://claude.ai/code/session_01RfFhJAs67hitVXQyd1PMfE
Go: drops the local replace directive and bumps github.com/jsonicjs/jsonic/go to v0.1.19 so builds work without the sibling checkout. TS already pulls jsonic main (2.25.1) via npm. Docs: reorganizes README into Diátaxis quadrants — Quick start (tutorial), How-to guides, Reference, and JSONC format (explanation, including a conformance-notes subsection about jsonic lenience vs. RFC 8259). Corrects the Go section to match the current single-export Jsonc plugin API. https://claude.ai/code/session_01RfFhJAs67hitVXQyd1PMfE
Adds JSONTestSuite as a git submodule and a test runner that validates
parsing in strict mode (disallowComments: true). y_* must accept, n_*
must reject, i_* recorded only. Known-lenient jsonic relaxations (leading
zeros, unquoted keys, etc.) are pinned so future divergences in either
direction are caught.
https://claude.ai/code/session_01RfFhJAs67hitVXQyd1PMfE