On Tue, 11 Aug 2026 18:24:50 GMT, Naoto Sato <[email protected]> wrote:
>> src/jdk.incubator.json/share/classes/jdk/incubator/json/JsonArray.java line
>> 78:
>>
>>> 76: .stream()
>>> 77: .map(Objects::requireNonNull)
>>> 78: .collect(Collectors.toCollection(ArrayList::new))
>>
>> Suggestion:
>>
>> .collect(Collectors.toList())
>>
>> Or you can use `src.stream().map(Objects::requireNonNull).toList()` - this
>> list does not NPE upon `contains(null)` but is immutable.
>
> Suggested one seems fine, but the other one (Stream.toList()) won't compile,
> as it produces List<? extends JsonValue)
I mistook the comment above as we don't want our returned list to fail with NPE
upon `contains` call; turns out we don't want to call `src.contains`. In this
case, we can just copy the `src` list with `List.copyOf`, which includes null
check for every list element.
>> src/jdk.incubator.json/share/classes/jdk/incubator/json/impl/JsonParser.java
>> line 222:
>>
>>> 220: // Check for empty case
>>> 221: if (charEquals(']')) {
>>> 222: return new JsonArrayImpl(List.of(), startO, doc);
>>
>> Similar observation for `List.of()` versus unmodifiable-wrapped `ArrayList`
>> as for `Map.of()` and unmodifiable-wrapped `LinkedHashMap`.
>
> same here
If that's the case, I would recommend `JsonArray.of` to simply become:
return new JsonArrayImpl(List.copyOf(src));
Where `List.copyOf` null checks the list itself and every single element in the
list.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3760772470
PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3760759734
