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
