mark JsonTypeCoercer.coerce as nullable

[asolntsev]

de-facto it can return null. Now IDEA will know it, and show warning for unsafe usages.

🔗 Related Issues

Fixes #14291

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s):
  • What was generated:
  • I reviewed all AI output and can explain the change

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Annotate JsonTypeCoercer.coerce as @nullable and propagate nullability through JSON coercers

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Mark JSON coercion APIs as nullable where JSON null is a valid outcome
• Propagate @Nullable through collection/map/object coercers and writers
• Add regression test coverage for reading arrays containing null values

Diagram

graph TD
  A["JsonInput.read()/readArray()"] --> B["JsonTypeCoercer.coerce() @Nullable"] --> C["TypeCoercer.apply() @Nullable"]
  C --> D["CollectionCoercer"]
  C --> E["MapCoercer"]
  C --> F["ObjectCoercer"]
  C --> G["Constructor/Instance Coercers"]

Loading

High-Level Assessment

Marking JsonTypeCoercer.coerce (and the TypeCoercer.apply contract) as @nullable is the most accurate representation of existing runtime behavior when encountering JSON null. Alternatives like introducing a separate sentinel value or forcing Optional everywhere would be more invasive and/or breaking; the chosen approach keeps behavior unchanged while improving static analysis and IDE warnings.

Files changed (10) +44 / -25

Bug fix (6) +20 / -15

CollectionCoercer.javaAllow nullable collection elements via JSpecify annotations +5/-4

Allow nullable collection elements via JSpecify annotations

• Updates the collection coercer generics and annotates the element consumer to accept @Nullable values. This aligns the container coercion path with JsonTypeCoercer.coerce potentially returning null for JSON null.

java/src/org/openqa/selenium/json/CollectionCoercer.java

ConstructorCoercer.javaAnnotate constructor parameter coercion helper as nullable +2/-0

Annotate constructor parameter coercion helper as nullable

• Marks the internal coerceValue helper as @Nullable, reflecting that coercion may legitimately yield null. This improves static null-safety when validating optional vs required constructor parameters.

java/src/org/openqa/selenium/json/ConstructorCoercer.java

JsonInput.javaReturn nullable element lists from readArray(Type) +2/-2

Return nullable element lists from readArray(Type)

• Changes readArray to return List<@Nullable T>, accurately modeling arrays that can contain null elements. The runtime behavior is unchanged; the signature now communicates nullability to tooling.

java/src/org/openqa/selenium/json/JsonInput.java

JsonTypeCoercer.javaMark JsonTypeCoercer.coerce as @Nullable +2/-4

Mark JsonTypeCoercer.coerce as @nullable

• Annotates the central coerce method as @Nullable since it returns null for JSON null (when not targeting Optional). This makes downstream coercer implementations and callers reflect the actual contract.

java/src/org/openqa/selenium/json/JsonTypeCoercer.java

MapCoercer.javaAllow nullable map values and enforce non-null map keys +7/-4

Allow nullable map values and enforce non-null map keys

• Updates the map consumer factory to accept @Nullable values and adds requireNonNull around coerced non-string keys. This matches JSON semantics (keys cannot be null) while allowing null values.

java/src/org/openqa/selenium/json/MapCoercer.java

ObjectCoercer.javaDeclare Object coercion result as nullable +2/-1

Declare Object coercion result as nullable

• Updates the ObjectCoercer apply signature to return a nullable Object, consistent with JsonTypeCoercer.coerce returning null for JSON null. No change to the dispatch logic beyond the contract.

java/src/org/openqa/selenium/json/ObjectCoercer.java

Refactor (3) +10 / -8

EnumCoercer.javaTighten enum generic bound to Enum<T> +1/-1

Tighten enum generic bound to Enum<T>

• Changes the generic parameter from raw Enum to Enum<T> for stronger type-safety. No behavioral change; this is a compile-time correctness improvement.

java/src/org/openqa/selenium/json/EnumCoercer.java

InstanceCoercer.javaPropagate nullable value handling through reflective writers +7/-6

Propagate nullable value handling through reflective writers

• Updates BiConsumer writer types (field/property writers) to accept @Nullable values. This matches the coercion pipeline where JSON null can flow into reflective assignment.

java/src/org/openqa/selenium/json/InstanceCoercer.java

TypeCoercer.javaMake TypeCoercer.apply contract explicitly nullable +2/-1

Make TypeCoercer.apply contract explicitly nullable

• Changes the abstract apply method to return a BiFunction producing @Nullable T. This propagates the correct nullability contract to all coercer implementations.

java/src/org/openqa/selenium/json/TypeCoercer.java

Tests (1) +14 / -2

JsonInputTest.javaUpdate tests for new helpers and add null-array coverage +14/-2

Update tests for new helpers and add null-array coverage

• Switches callers to readMap() and readNonNull(...) to align with updated nullability contracts. Adds a new test verifying readArray can return lists containing null elements.

java/test/org/openqa/selenium/json/JsonInputTest.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 18 rules

1. TypeCoercer nullness contract ✓ Resolved 🐞 Bug ≡ Correctness

Description

TypeCoercer still implements Function<Type, BiFunction<..., T>> (non-null result), but its
apply now returns BiFunction<..., @Nullable T>, creating a contradictory nullness contract for
JSpecify/IDE tooling. This undermines the PR’s goal by letting callers treat coercers as producing
non-null values when they are explicitly nullable.

Code

java/src/org/openqa/selenium/json/TypeCoercer.java[R26-27]

public abstract class TypeCoercer<T>
    implements Predicate<Class<?>>, Function<Type, BiFunction<JsonInput, PropertySetting, T>> {

Evidence

TypeCoercer’s declared supertype promises a BiFunction producing non-null T, while the
abstract apply now explicitly produces @Nullable T, which is inconsistent under nullness-aware
tooling.

java/src/org/openqa/selenium/json/TypeCoercer.java[26-34]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`TypeCoercer` implements `Function<Type, BiFunction<JsonInput, PropertySetting, T>>` but its `apply` method now returns `BiFunction<JsonInput, PropertySetting, @Nullable T>`. With JSpecify-aware nullness tools, this is an incompatible/contradictory contract.

### Issue Context
The PR is introducing nullability metadata; leaving `TypeCoercer`’s implemented `Function` signature non-null defeats or breaks the intended propagation.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/TypeCoercer.java[26-34]

### Expected fix
Update the implemented `Function` type argument to match the new nullable return:
- `Function<Type, BiFunction<JsonInput, PropertySetting, @Nullable T>>`
and adjust any resulting type-checker fallout accordingly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. EnumCoercer missing class Javadoc ✗ Dismissed 📘 Rule violation ✧ Quality

Description

The modified public class EnumCoercer has no Javadoc comment immediately preceding its
declaration. This violates the requirement that modified public API types include Javadoc and makes
the public API harder to understand and maintain.

Code

java/src/org/openqa/selenium/json/EnumCoercer.java[25]

+public class EnumCoercer<T extends Enum<T>> extends TypeCoercer<T> {

Evidence

The compliance rule requires Javadoc for all modified public API types. EnumCoercer is declared
public and its declaration was changed in this PR, but there is no /** ... */ Javadoc block
immediately above the class declaration.

Rule 330200: Require Javadoc for all public API types and methods
java/src/org/openqa/selenium/json/EnumCoercer.java[20-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`EnumCoercer` is a public API type modified in this PR, but it has no class-level Javadoc immediately above the class declaration.

## Issue Context
This PR changes the generic bound to `T extends Enum<T>`, which is a user-visible API refinement and should be documented.

## Fix Focus Areas
- java/src/org/openqa/selenium/json/EnumCoercer.java[25-25]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Nullable coerce returned as nonnull 🐞 Bug ≡ Correctness

Description

JsonTypeCoercer.coerce is now @Nullable, but core APIs still return it as non-null (e.g.,
Json#toType) or dereference it without a non-null assertion (e.g., ConstructorCoercer uses
properties.keySet()), which will trigger nullness-checker errors and hides real nullable behavior
(JSON null -> Java null).

Code

java/src/org/openqa/selenium/json/JsonTypeCoercer.java[R140-143]

+  @Nullable <T> T coerce(JsonInput json, Type typeOfT, PropertySetting setter) {
    BiFunction<JsonInput, PropertySetting, Object> coercer =
        knownCoercers.computeIfAbsent(typeOfT, this::buildCoercer);

Evidence

coerce is explicitly nullable and returns null on JsonType.NULL; Json#toType returns that
value as non-null T, and ConstructorCoercer stores it into a non-null Map and immediately
dereferences it, both of which become invalid under the new nullability contract.

java/src/org/openqa/selenium/json/JsonTypeCoercer.java[140-155]
java/src/org/openqa/selenium/json/Json.java[209-214]
java/src/org/openqa/selenium/json/ConstructorCoercer.java[57-66]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`JsonTypeCoercer.coerce` is now annotated `@Nullable`, but callers like `Json#toType` return the result as non-null and `ConstructorCoercer` dereferences the result without a non-null assertion. This creates inconsistent nullness contracts and will cause IDE/JSpecify warnings (and can propagate unexpected nulls at API boundaries).

### Issue Context
Runtime behavior already allows `coerce` to return null when the JSON token is `null` (for non-Optional targets). The PR now exposes this via annotations, so callers must either:
- accept/annotate nullable returns, or
- enforce non-null with `Require.nonNull(...)` / `Objects.requireNonNull(...)` and throw a domain exception.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/JsonTypeCoercer.java[140-155]
- java/src/org/openqa/selenium/json/Json.java[209-214]
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[57-66]

### Expected fix
Pick one consistent policy per API:
- For `Json#toType(...)`: either change return type to `@Nullable <T> T` (most accurate), or wrap with `Require.nonNull` and throw `JsonException` when JSON is `null`.
- For `ConstructorCoercer`: assert non-null for `properties` (e.g., `Require.nonNull("properties", ...)`) before `properties.keySet()` since this coercer is only invoked for non-null JSON objects.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (1)

4. TypeCoercer.apply missing Javadoc ✗ Dismissed 📘 Rule violation ✧ Quality

Description

The modified public API method TypeCoercer.apply(Type) has no Javadoc block, including required
free-text description and @param/@return tags. This violates the requirement for complete
Javadoc on changed public methods and reduces API clarity for users and implementers.

Code

java/src/org/openqa/selenium/json/TypeCoercer.java[33]

+  public abstract BiFunction<JsonInput, PropertySetting, @Nullable T> apply(Type type);

Evidence

The compliance rule requires complete Javadoc for each changed public method. In TypeCoercer.java,
the modified public abstract BiFunction<JsonInput, PropertySetting, @Nullable T> apply(Type type);
has no preceding Javadoc block and therefore lacks the required descriptive sentence and tags.

Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/json/TypeCoercer.java[26-34]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TypeCoercer.apply(Type)` is a changed public method but has no Javadoc. The compliance rule requires a Javadoc block with at least one descriptive sentence plus complete `@param` and `@return` tags.

## Issue Context
This PR introduces `@Nullable` in the return type of `apply`, making correct documentation more important for API consumers and implementers.

## Fix Focus Areas
- java/src/org/openqa/selenium/json/TypeCoercer.java[26-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Tests use nonnull List 🐞 Bug ⚙ Maintainability

Description

JsonInput.readArray now returns List<@Nullable T>, but JsonInputTest assigns it to
List<Integer> and even asserts null elements, which conflicts with the new nullness contract and
will be flagged by IDE/nullness tooling.

Code

java/test/org/openqa/selenium/json/JsonInputTest.java[R266-275]

+  @Test
+  void canReadListOfType_null() {
+    String raw = "[null, null]";
+
+    try (JsonInput in = new JsonInput(new StringReader(raw), new JsonTypeCoercer(), BY_NAME)) {
+      List<Integer> array = in.readArray(Integer.class);
+
+      assertThat(array).containsExactly(null, null);
+    }
+  }

Evidence

The production signature now explicitly allows null elements in the returned list, while the
modified test code still uses a non-null element type and verifies nulls, which is precisely the
unsafe usage the PR aims to surface.

java/src/org/openqa/selenium/json/JsonInput.java[543-552]
java/test/org/openqa/selenium/json/JsonInputTest.java[255-275]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`JsonInput.readArray(Type)` now returns a list whose elements may be null (`List<@Nullable T>`). Tests currently store that into `List<Integer>` and assert nulls, which is inconsistent with the new type contract.

### Issue Context
This PR’s goal is to make IDEA warn on unsafe usages; test code should model the correct (nullable) types to avoid nullness-checker failures.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/JsonInput.java[543-552]
- java/test/org/openqa/selenium/json/JsonInputTest.java[255-275]

### Expected fix
Update tests to use nullable element types, e.g.:
- `List<@Nullable Integer> array = in.readArray(Integer.class);`
(and add `import org.jspecify.annotations.Nullable;`). Consider doing this for both `canReadListOfType` and `canReadListOfType_null`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

4. Nullable coerce returned as nonnull 🐞 Bug ≡ Correctness

JsonTypeCoercer.coerce is now @Nullable, but core APIs still return it as non-null (e.g.,
Json#toType) or dereference it without a non-null assertion (e.g., ConstructorCoercer uses
properties.keySet()), which will trigger nullness-checker errors and hides real nullable behavior
(JSON null -> Java null).

Agent Prompt

### Issue description
`JsonTypeCoercer.coerce` is now annotated `@Nullable`, but callers like `Json#toType` return the result as non-null and `ConstructorCoercer` dereferences the result without a non-null assertion. This creates inconsistent nullness contracts and will cause IDE/JSpecify warnings (and can propagate unexpected nulls at API boundaries).

### Issue Context
Runtime behavior already allows `coerce` to return null when the JSON token is `null` (for non-Optional targets). The PR now exposes this via annotations, so callers must either:
- accept/annotate nullable returns, or
- enforce non-null with `Require.nonNull(...)` / `Objects.requireNonNull(...)` and throw a domain exception.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/JsonTypeCoercer.java[140-155]
- java/src/org/openqa/selenium/json/Json.java[209-214]
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[57-66]

### Expected fix
Pick one consistent policy per API:
- For `Json#toType(...)`: either change return type to `@Nullable <T> T` (most accurate), or wrap with `Require.nonNull` and throw `JsonException` when JSON is `null`.
- For `ConstructorCoercer`: assert non-null for `properties` (e.g., `Require.nonNull("properties", ...)`) before `properties.keySet()` since this coercer is only invoked for non-null JSON objects.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools