On Mon, 10 Aug 2026 17:36:35 GMT, Naoto Sato <[email protected]> wrote:

> This PR implements [JEP 540: Simple JSON API 
> (Incubator)](https://openjdk.org/jeps/540).
> 
> It adds the `jdk.incubator.json` module which provides APIs for reading and 
> writing JSON documents as specified by [RFC 
> 8259](https://datatracker.ietf.org/doc/html/rfc8259). This is an incubating 
> API.
> 
> API documentation: 
> https://cr.openjdk.org/~naoto/json/javadoc/api/jdk.incubator.json/module-summary.html
> Co-authored-by: Justin Lu 
> ([@justin-curtis-lu](https://github.com/justin-curtis-lu))
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

src/jdk.incubator.json/share/classes/jdk/incubator/json/Json.java line 62:

> 60:      * <p>
> 61:      * {@code JsonObject}s preserve the order of their members declared 
> in and parsed from
> 62:      * the JSON document.

It might be simpler to just say that it JsonObjects "preserve the order of 
members in the input JSON document."

src/jdk.incubator.json/share/classes/jdk/incubator/json/Json.java line 98:

> 96:      * @return the parsed {@code JsonValue}
> 97:      */
> 98:     public static JsonValue parse(char[] in) {

I assume one of the things to get feedback on while the API is incubator is 
which parse methods to add. If parse(char[]) stays then it will need to specify 
that changes to the array after it the payload has been parsed are ignored.

src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonArray.java line 66:

> 64:     /**
> 65:      * {@return the {@code JsonArray} created from the given
> 66:      * list of {@code JsonValue}s}

The method description doesn't say if this method makes a copy or provides a 
JsonArray view of the list. What if src is mutable and is modified after 
creating the JsonArray.

src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonNumber.java line 
102:

> 100:      * The conversion is performed using {@link 
> Double#parseDouble(String)}.
> 101:      * If the converted {@code double} value is {@link 
> Double#POSITIVE_INFINITY}
> 102:      * or {@link Double#NEGATIVE_INFINITY}, a {@code JsonValueException} 
> is thrown.

Does this need to say anything about rounding to the nearest double? Also I 
wonder about underflow to zero.

src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonObject.java line 
101:

> 99:      * @param map the map of {@code JsonValue}s. Non-null.
> 100:      * @throws IllegalArgumentException if duplicate member names are 
> given in
> 101:      *      {@code map}.

A Map can't have duplicate keys.  It might have to be expanded to be clear that 
it means duplicate keys encountering when iterating over the mappings.

src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonParseException.java 
line 58:

> 56:     /**
> 57:      * Constructs a JsonParseException with the specified detail message.
> 58:      * @param message the detail message

can be null I assume.

src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonString.java line 38:

> 36:  * A {@code JsonString} can be produced by a {@link Json#parse(String)}.
> 37:  * Within a valid JSON string, any character may be escaped using either a
> 38:  * two-character escape sequence (if applicable) or a Unicode escape 
> sequence.

Technically one or two Unicode escape sequences to allow for supplementary 
Unicode characters.

src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonString.java line 43:

> 41:  * <p> Alternatively, {@link #of(String)} can be used to obtain a {@code 
> JsonString}
> 42:  * directly from a {@code String}. The {@code String} values of {@code 
> JsonString}
> 43:  * instances produced by the following expressions are all equivalent,

equivalent, -> equivalent: ?

src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonValue.java line 150:

> 148:  * <h2 id="generation">Generating JSON documents</h2>
> 149:  * {@code JsonValue} overrides {@link Object#toString()} to generate RFC 
> 8259 compliant
> 150:  * JSON text in a compact representation with white spaces eliminated.

You may want to clarify this as it's insignificant whitespace that is 
eliminated, not all whitespace as string values and member names can include a 
space.

src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonValueException.java 
line 43:

> 41:  *     An access method is invoked for a non-existent value, such as
> 42:  *     {@code get(String)} for a missing member in a {@code JsonObject}, 
> or
> 43:  *     {@code get(int)} for an out-of-bounds index in a {@code JsonArray}.

Okay but there is also tryGet that will return an optional.empty when the 
member does not exist.

src/jdk.incubator.json/share/classes/jdk/incubator/json/package-info.java line 
28:

> 26: /**
> 27:  * Provides APIs for parsing JSON text, retrieving JSON values in the 
> text, and
> 28:  * generating JSON text.

The Parsing JSON section links to RFC 8259 but doesn't name it.  The first 
mention of the RFC is further down in the Generating JSON section. I would be 
tempted to add a sentence to the first paragraph so that it's clear that the 
JSON format is defined by RFC 8259.

src/jdk.incubator.json/share/classes/jdk/incubator/json/package-info.java line 
40:

> 38:  * <a href="https://datatracker.ietf.org/doc/html/rfc8259";>JSON 
> grammar</a>.
> 39:  * The parsing APIs provided do not accept JSON text that contains JSON 
> objects
> 40:  * with duplicate names.

The first sentence is the successful case, the second sentence is a specific 
failure case, it feels like something is missing. One way to fix this is to 
extend the success condition to say that it adheres to the JSON grammar and 
contains no objects with duplicate member names.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3759113565
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3759100522
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3758728822
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3758778371
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3758945362
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3758953552
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3759022250
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3758999627
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3759210513
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3759042547
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3758673992
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3758628401

Reply via email to