docs: document standards compliance and parse() vs operator>> strictness - #5326
docs: document standards compliance and parse() vs operator>> strictness#5326youdie006 wants to merge 1 commit into
Conversation
…rator>> strictness The compliance story lived only in tests/src/unit-testsuites.cpp, so drive-by comparisons kept claiming the library "does not fully pass JSONTestSuite". Make it discoverable: - README: add a "Standards compliance" note stating that both nst JSONTestSuite revisions run in CI, that all mandatory y_/n_ cases pass through the strict parse() entry point, and listing the deliberate implementation-defined i_ choices (unbounded nesting, silent BOM stripping, noncharacters forwarded, strict rejection of invalid UTF-8 and lone surrogates, out_of_range.406 on numeric overflow). - features/parsing: add a "Strictness and trailing data" section documenting that parse() is strict and rejects trailing data while operator>> follows relaxed iostream semantics (parses one value and leaves the stream positioned after it) -- the single place a naive test yields a "non-compliant" result. Documentation only; no parser behavior change. Closes nlohmann#5290. Signed-off-by: manon <youdie006@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b65b09d9f2
ℹ️ 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".
|
|
||
| ### Standards compliance | ||
|
|
||
| The library targets strict conformance with [RFC 8259](https://tools.ietf.org/html/rfc8259.html). Both the original [JSONTestSuite](https://github.com/nst/JSONTestSuite) and its updated revision are vendored and exercised in CI (see [`tests/src/unit-testsuites.cpp`](https://github.com/nlohmann/json/blob/develop/tests/src/unit-testsuites.cpp)): |
There was a problem hiding this comment.
Do not describe downloaded test data as vendored
In a source checkout or offline build, these suites are not vendored in this repository. A repo-wide file search finds no nst_json_testsuite data, while cmake/download_test_data.cmake downloads the external json_test_data v3.1.0 archive unless JSON_TestDataDirectory is supplied. This statement contradicts the later test instructions and may lead contributors to expect the conformance tests to run without separately obtaining test data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
cmake/download_test_data.cmake downloads the test suite data from a separate repo (nlohmann/json_test_data) at configure time; it isn't committed/bundled in this repo. "Vendored" implies the latter.
|
|
||
| The library targets strict conformance with [RFC 8259](https://tools.ietf.org/html/rfc8259.html). Both the original [JSONTestSuite](https://github.com/nst/JSONTestSuite) and its updated revision are vendored and exercised in CI (see [`tests/src/unit-testsuites.cpp`](https://github.com/nlohmann/json/blob/develop/tests/src/unit-testsuites.cpp)): | ||
|
|
||
| - All mandatory `y_` (must-accept) and `n_` (must-reject) cases pass through the strict [`parse()`](https://json.nlohmann.me/api/basic_json/parse/) entry point. |
There was a problem hiding this comment.
Avoid claiming every suite case uses strict parse
The referenced test does not run all mandatory cases through the strict parse() entry point: the original suite's y loop ends with CHECK_NOTHROW(f >> j) at tests/src/unit-testsuites.cpp:583, and cases such as y_string_utf16.json and n_number_then_00.json are commented out. Either exercise every advertised case with parse() or narrow this conformance claim so it accurately describes the coverage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The original JSONTestSuite's y (must-accept) section in tests/src/unit-testsuites.cpp:583 actually uses operator>>, not parse(). Only the n section and the second/updated suite's y+n sections use strict parse(). y_string_utf16.json is also commented out and not exercised at all.
There was a problem hiding this comment.
The claim is true for nst_json_testsuite2, which uses json::parse(f) for both y (:1010) and n (:1212).
| [`operator>>`](../../api/operator_gtgt.md) follows relaxed `#!cpp std::istream` semantics instead: it parses one JSON | ||
| value and leaves the stream positioned right after it, without requiring the rest of the stream to be consumed. This is | ||
| what makes it possible to read several concatenated values from the same stream, but it also means that "a valid |
There was a problem hiding this comment.
Qualify stream positioning after numeric values
This positioning guarantee is false when the extracted value is a number: the lexer reads one byte past the number, and its internal simulated unget() does not restore that byte to the underlying stream. With the current implementation, extracting from 1true returns 1 but leaves the stream at r, so the next extraction fails rather than reading true; repeated extraction only works in this case when a separator such as whitespace can be consumed. The documentation should not promise arbitrary concatenated values or exact positioning.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This description is correct.
There was a problem hiding this comment.
Indeed. the lexer's unget() is simulated (lexer.hpp:1437 — "The input is not changed"), and input_stream_adapter::get_character() uses sbumpc() with no sungetc() (input_adapters.hpp:127). So the one byte that terminates a number is eaten. Literals and structural values self-terminate, so they're unaffected.
Please add one caveat sentence (also to operator_gtgt.md), e.g. "A number is only terminated by the character following it, which operator>> consumes; concatenated numbers must therefore be whitespace-separated (1 2 works, 1true does not)."
nlohmann
left a comment
There was a problem hiding this comment.
The AI-generated comments are not helpful btw. I will review this by the end of the week.
|
|
||
| ### Standards compliance | ||
|
|
||
| The library targets strict conformance with [RFC 8259](https://tools.ietf.org/html/rfc8259.html). Both the original [JSONTestSuite](https://github.com/nst/JSONTestSuite) and its updated revision are vendored and exercised in CI (see [`tests/src/unit-testsuites.cpp`](https://github.com/nlohmann/json/blob/develop/tests/src/unit-testsuites.cpp)): |
There was a problem hiding this comment.
cmake/download_test_data.cmake downloads the test suite data from a separate repo (nlohmann/json_test_data) at configure time; it isn't committed/bundled in this repo. "Vendored" implies the latter.
|
|
||
| The library targets strict conformance with [RFC 8259](https://tools.ietf.org/html/rfc8259.html). Both the original [JSONTestSuite](https://github.com/nst/JSONTestSuite) and its updated revision are vendored and exercised in CI (see [`tests/src/unit-testsuites.cpp`](https://github.com/nlohmann/json/blob/develop/tests/src/unit-testsuites.cpp)): | ||
|
|
||
| - All mandatory `y_` (must-accept) and `n_` (must-reject) cases pass through the strict [`parse()`](https://json.nlohmann.me/api/basic_json/parse/) entry point. |
There was a problem hiding this comment.
The original JSONTestSuite's y (must-accept) section in tests/src/unit-testsuites.cpp:583 actually uses operator>>, not parse(). Only the n section and the second/updated suite's y+n sections use strict parse(). y_string_utf16.json is also commented out and not exercised at all.
|
|
||
| The library targets strict conformance with [RFC 8259](https://tools.ietf.org/html/rfc8259.html). Both the original [JSONTestSuite](https://github.com/nst/JSONTestSuite) and its updated revision are vendored and exercised in CI (see [`tests/src/unit-testsuites.cpp`](https://github.com/nlohmann/json/blob/develop/tests/src/unit-testsuites.cpp)): | ||
|
|
||
| - All mandatory `y_` (must-accept) and `n_` (must-reject) cases pass through the strict [`parse()`](https://json.nlohmann.me/api/basic_json/parse/) entry point. |
There was a problem hiding this comment.
The claim is true for nst_json_testsuite2, which uses json::parse(f) for both y (:1010) and n (:1212).
| [`operator>>`](../../api/operator_gtgt.md) follows relaxed `#!cpp std::istream` semantics instead: it parses one JSON | ||
| value and leaves the stream positioned right after it, without requiring the rest of the stream to be consumed. This is | ||
| what makes it possible to read several concatenated values from the same stream, but it also means that "a valid |
There was a problem hiding this comment.
Indeed. the lexer's unget() is simulated (lexer.hpp:1437 — "The input is not changed"), and input_stream_adapter::get_character() uses sbumpc() with no sungetc() (input_adapters.hpp:127). So the one byte that terminates a number is eaten. Literals and structural values self-terminate, so they're unaffected.
Please add one caveat sentence (also to operator_gtgt.md), e.g. "A number is only terminated by the character following it, which operator>> consumes; concatenated numbers must therefore be whitespace-separated (1 2 works, 1true does not)."
This documents the library's RFC 8259 / JSONTestSuite compliance and the
parse()vsoperator>>strictness difference, so the (good) compliance story is discoverable and citable instead of living only intests/src/unit-testsuites.cpp.What
README.md-> new "Standards compliance" note under "Notes": both nst JSONTestSuite revisions run in CI; all mandatoryy_/n_cases pass through the strictparse()entry point; the deliberate implementation-definedi_choices are listed (unbounded nesting, silent BOM stripping, noncharacters forwarded, strict rejection of invalid UTF-8 and lone surrogates,out_of_range.406on numeric overflow); and theparse()vsoperator>>nuance is called out.docs/mkdocs/docs/features/parsing/index.md-> new "Strictness and trailing data" section documenting thatparse()is strict and rejects trailing data, whileoperator>>follows relaxed iostream semantics (parses one value and leaves the stream positioned after it).Why
Drive-by comparisons keep claiming the library "does not fully pass JSONTestSuite", usually by feeding "valid document + trailing garbage" through
operator>>(which accepts it) rather thanparse()(which rejects it). Making the compliance story and the two-API strictness difference discoverable addresses that.Documentation only; no parser behavior change. The
i_choices and the strictness behavior were verified against the source before writing:out_of_range.406(docs/mkdocs/docs/home/exceptions.md), BOM handling (api/basic_json/parse.md), noncharacters/surrogates (README "Character encoding"), and theoperator>>"parses one value and leaves the stream positioned after it" note (api/operator_gtgt.md).Verified locally with
mkdocs build: the docs site builds cleanly and the new parsing section and its links render without new warnings.Closes #5290.
Disclosure: prepared with AI assistance; I reviewed the change and verified every claim against the source files cited above.
make amalgamate- N/A (no source code change).