Skip to content

Document JSON5 duplicate key behavior in test#1368

Merged
j178 merged 2 commits intojson5from
copilot/sub-pr-1367
Jan 16, 2026
Merged

Document JSON5 duplicate key behavior in test#1368
j178 merged 2 commits intojson5from
copilot/sub-pr-1367

Conversation

Copy link
Contributor

Copilot AI commented Jan 16, 2026 •
edited
Loading

The test_duplicate_keys test in check-json5 expects code 0 (pass), while check-json explicitly fails on duplicates. This intentional difference lacked documentation.

Added comment explaining:

  • JSON5 follows JavaScript object semantics where duplicate keys are valid (last value wins)
  • The json5 crate parses duplicates without error per spec
  • Differs from check-json which enforces strict uniqueness validation
#[tokio::test]
async fn test_duplicate_keys() -> anyhow::Result<()> {
    // JSON5 allows duplicate keys following JavaScript object semantics (last value wins).
    // This differs from check-json which explicitly rejects duplicate keys.
    // The json5 crate parses duplicate keys without error, treating them as valid JSON5.
    let dir = tempdir()?;
    // ...
}

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI mentioned this pull request Jan 16, 2026
Merged
@j178 j178 marked this pull request as ready for review January 16, 2026 06:48
Copilot AI changed the title [WIP] Address feedback on adding check-json5 as builtin hooks Document JSON5 duplicate key behavior in test Jan 16, 2026
Copilot AI requested a review from j178 January 16, 2026 06:50
@j178 j178 merged commit 8f4f345 into json5 Jan 16, 2026
@j178 j178 deleted the copilot/sub-pr-1367 branch January 16, 2026 06:50
j178 added a commit that referenced this pull request Jan 16, 2026
@j178
Add check-json5 as builtin hooks (#1367)
352685f 
* Add `check-json5` as builtin hooks

* Refactor

* Add tests

* Tweak

* Document JSON5 duplicate key behavior in test (#1368)

* Initial plan

* Add documentation explaining duplicate key behavior in JSON5

Co-authored-by: j178 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: j178 <[email protected]>

---------

Co-authored-by: Copilot <[email protected]>
@ulgens
Copy link
Contributor

ulgens commented Jan 17, 2026 •
edited
Loading

JSON5 follows JavaScript object semantics where duplicate keys are valid (last value wins)

Where is this coming from? It's not wrong, but I couldn't find anything about it in the spec, and Copilot is generally not the best at analyzing these sorts of details so I'm worrying it may be missing a detail there.

There is one notable difference from the JSON api: the load() and loads() methods support optionally checking for (and rejecting) duplicate object keys; pass allow_duplicate_keys=False to do so (duplicates are allowed by default).

https://github.com/dpranke/pyjson5/blob/04546807bf9bd1fefa17a653c971f294b7ff328d/README.md

Is there any chance of replicating that option?

@j178 j178 mentioned this pull request Jan 18, 2026
Merged
@j178
Copy link
Owner

j178 commented Jan 18, 2026

Yeah, done in #1387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@j178 j178 Awaiting requested review from j178

Assignees

@j178 j178

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ulgens @j178