Skip to content

[java] specify nullability in package org.openqa.selenium.docker#17191

Open
asolntsev wants to merge 1 commit intoSeleniumHQ:trunkfrom
asolntsev:nullability-in-docker
Open

[java] specify nullability in package org.openqa.selenium.docker#17191
asolntsev wants to merge 1 commit intoSeleniumHQ:trunkfrom
asolntsev:nullability-in-docker

Conversation

@asolntsev
Copy link
Contributor

🔗 Related Issues

Partially implements #14291

💥 What does this PR do?

Adds JSpecify nullability annotations to packages

  • org.openqa.selenium.docker.*

🔄 Types of changes

  • Cleanup (formatting, renaming)

@selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations labels Mar 8, 2026
@asolntsev asolntsev removed the B-build Includes scripting, bazel and CI integrations label Mar 8, 2026
@asolntsev asolntsev self-assigned this Mar 8, 2026
@asolntsev asolntsev added this to the 4.42.0 milestone Mar 8, 2026
@qodo-code-review
Copy link
Contributor

Review Summary by Qodo

Add JSpecify nullability annotations to docker package and improve code quality

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Add JSpecify nullability annotations to org.openqa.selenium.docker package
• Create package-info files with @NullMarked for three subpackages
• Add nonBlank() validation method to Require utility class
• Improve code quality with null-safety checks and string formatting
• Enhance test coverage for version comparison and adapter functionality

Grey Divider

File Changes

1. java/src/org/openqa/selenium/docker/ContainerConfig.java ✨ Enhancement +4/-2

Add nullability annotations to name field and getter

java/src/org/openqa/selenium/docker/ContainerConfig.java


2. java/src/org/openqa/selenium/docker/ContainerInfo.java 🐞 Bug fix +2/-2

Add final modifiers to hostConfig and labels fields

java/src/org/openqa/selenium/docker/ContainerInfo.java


3. java/src/org/openqa/selenium/docker/ContainerLogs.java ✨ Enhancement +1/-1

Improve toString() with String.format for clarity

java/src/org/openqa/selenium/docker/ContainerLogs.java


View more (23)
4. java/src/org/openqa/selenium/docker/Device.java ✨ Enhancement +4/-2

Add nullability annotation and improve null check

java/src/org/openqa/selenium/docker/Device.java


5. java/src/org/openqa/selenium/docker/Docker.java ✨ Enhancement +3/-2

Add nullability annotation to apiVersion field and parameter

java/src/org/openqa/selenium/docker/Docker.java


6. java/src/org/openqa/selenium/docker/Version.java 🐞 Bug fix +0/-7

Remove unnecessary null checks in compare method

java/src/org/openqa/selenium/docker/Version.java


7. java/src/org/openqa/selenium/docker/VersionCommand.java ✨ Enhancement +2/-1

Add nullability annotation to requestedVersion parameter

java/src/org/openqa/selenium/docker/VersionCommand.java


8. java/src/org/openqa/selenium/docker/client/AdapterFactory.java ✨ Enhancement +2/-3

Replace manual validation with Require.nonBlank() call

java/src/org/openqa/selenium/docker/client/AdapterFactory.java


9. java/src/org/openqa/selenium/docker/client/ApiVersionAdapter.java ✨ Enhancement +5/-3

Add nullability annotations to adapter method signatures

java/src/org/openqa/selenium/docker/client/ApiVersionAdapter.java


10. java/src/org/openqa/selenium/docker/client/CreateContainer.java ✨ Enhancement +4/-10

Simplify URLEncoder usage and remove exception handling

java/src/org/openqa/selenium/docker/client/CreateContainer.java


11. java/src/org/openqa/selenium/docker/client/V140Adapter.java ✨ Enhancement +7/-3

Add nullability annotations to adapter method implementations

java/src/org/openqa/selenium/docker/client/V140Adapter.java


12. java/src/org/openqa/selenium/docker/client/V144Adapter.java ✨ Enhancement +7/-3

Add nullability annotations to adapter method implementations

java/src/org/openqa/selenium/docker/client/V144Adapter.java


13. java/src/org/openqa/selenium/docker/client/V148Adapter.java ✨ Enhancement +7/-4

Add nullability annotations and remove redundant suppressions

java/src/org/openqa/selenium/docker/client/V148Adapter.java


14. java/src/org/openqa/selenium/docker/client/package-info.java ✨ Enhancement +21/-0

Create package-info with NullMarked annotation

java/src/org/openqa/selenium/docker/client/package-info.java


15. java/src/org/openqa/selenium/docker/internal/ImageSummary.java ✨ Enhancement +1/-0

Add suppression annotations for unused field warnings

java/src/org/openqa/selenium/docker/internal/ImageSummary.java


16. java/src/org/openqa/selenium/docker/internal/Reference.java ✨ Enhancement +9/-5

Add nullability annotations to tag and digest fields

java/src/org/openqa/selenium/docker/internal/Reference.java


17. java/src/org/openqa/selenium/docker/internal/package-info.java ✨ Enhancement +21/-0

Create package-info with NullMarked annotation

java/src/org/openqa/selenium/docker/internal/package-info.java


18. java/src/org/openqa/selenium/docker/package-info.java ✨ Enhancement +21/-0

Create package-info with NullMarked annotation

java/src/org/openqa/selenium/docker/package-info.java


19. java/src/org/openqa/selenium/internal/Require.java ✨ Enhancement +9/-0

Add nonBlank() validation method for string arguments

java/src/org/openqa/selenium/internal/Require.java


20. java/test/org/openqa/selenium/RequireTest.java 🧪 Tests +3/-1

Add test coverage for nonBlank() validation method

java/test/org/openqa/selenium/RequireTest.java


21. java/test/org/openqa/selenium/docker/VersionTest.java 🧪 Tests +33/-0

Add tests for toString() and version comparison with non-numbers

java/test/org/openqa/selenium/docker/VersionTest.java


22. java/test/org/openqa/selenium/docker/client/AdapterFactoryTest.java 🧪 Tests +4/-3

Update error message assertions for new validation method

java/test/org/openqa/selenium/docker/client/AdapterFactoryTest.java


23. java/test/org/openqa/selenium/docker/client/V140AdapterTest.java 🧪 Tests +11/-5

Improve test assertions with more detailed map validation

java/test/org/openqa/selenium/docker/client/V140AdapterTest.java


24. java/test/org/openqa/selenium/docker/client/V144AdapterTest.java 🧪 Tests +10/-4

Improve test assertions with more detailed map validation

java/test/org/openqa/selenium/docker/client/V144AdapterTest.java


25. java/test/org/openqa/selenium/docker/client/V148AdapterTest.java 🧪 Tests +6/-5

Improve test assertions with more detailed map validation

java/test/org/openqa/selenium/docker/client/V148AdapterTest.java


26. java/src/org/openqa/selenium/docker/BUILD.bazel Dependencies +1/-0

Add jspecify dependency to docker library

java/src/org/openqa/selenium/docker/BUILD.bazel


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 8, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (1)

Grey Divider


Action required

1. adaptImageResponse nullness mismatch 📎 Requirement gap ✓ Correctness
Description
ApiVersionAdapter#adaptImageResponse declares a non-null response parameter under @NullMarked,
but implementations now accept @Nullable and handle null. This inconsistency weakens the nullness
contract and can trigger nullness checker/tooling warnings or confuse Kotlin nullability inference.
Code

java/src/org/openqa/selenium/docker/client/V140Adapter.java[R48-53]

+  @Nullable
  @Override
-  public Map<String, Object> adaptImageResponse(Map<String, Object> response) {
+  public Map<String, Object> adaptImageResponse(@Nullable Map<String, Object> response) {
    if (response == null) {
      return response;
    }
Evidence
The package is @NullMarked, so unannotated parameters are non-null by default; however,
adaptImageResponse leaves response unannotated in the interface while implementations annotate
it @Nullable and explicitly handle null, creating an inconsistent nullness contract.

Add JSpecify nullness annotations to Selenium Java APIs where null is possible
java/src/org/openqa/selenium/docker/client/package-info.java[18-19]
java/src/org/openqa/selenium/docker/client/ApiVersionAdapter.java[50-50]
java/src/org/openqa/selenium/docker/client/V140Adapter.java[48-53]

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

## Issue description
`ApiVersionAdapter#adaptImageResponse` has inconsistent nullness contracts: the interface parameter is implicitly non-null under `@NullMarked`, but implementations accept and handle `null`.

## Issue Context
Because `org.openqa.selenium.docker.client` is `@NullMarked`, any unannotated parameter is treated as non-null. Having implementations widen the parameter to `@Nullable` undermines the intended JSpecify signal and may cause nullness-tooling override inconsistencies.

## Fix Focus Areas
- java/src/org/openqa/selenium/docker/client/package-info.java[18-19]
- java/src/org/openqa/selenium/docker/client/ApiVersionAdapter.java[47-51]
- java/src/org/openqa/selenium/docker/client/V140Adapter.java[48-53]
- java/src/org/openqa/selenium/docker/client/V144Adapter.java[50-55]
- java/src/org/openqa/selenium/docker/client/V148Adapter.java[51-56]

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


2. Nullable adapter returns unchecked 🐞 Bug ⛯ Reliability
Description
ApiVersionAdapter now allows @Nullable returns for
adaptContainerCreateRequest/adaptContainerInspectResponse, but core call sites immediately
dereference the returned maps without null checks. This can cause NPEs and will be flagged by
NullAway if enabled.
Code

java/src/org/openqa/selenium/docker/client/ApiVersionAdapter.java[R61-64]

+  @Nullable Map<String, Object> adaptContainerCreateRequest(@Nullable Map<String, Object> request);

  /**
   * Adapts a container inspection response to normalize field names and structure.
Evidence
The adapter interface explicitly returns @Nullable for both methods, and implementations return null
when passed null. Callers in CreateContainer and InspectContainer assign results into non-null typed
variables and dereference them, so a null return would crash; additionally this is inconsistent with
the new nullability contract and likely violates NullAway when enabled.

java/src/org/openqa/selenium/docker/client/ApiVersionAdapter.java[61-74]
java/src/org/openqa/selenium/docker/client/CreateContainer.java[62-72]
java/src/org/openqa/selenium/docker/client/InspectContainer.java[67-74]
java/private/java_library.bzl[16-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
Core call sites treat adapter return values as non-null, but the interface now declares them `@Nullable`.

## Issue Context
This is both a runtime NPE risk and a likely NullAway violation when enabled.

## Fix Focus Areas
- java/src/org/openqa/selenium/docker/client/ApiVersionAdapter.java[58-74]
- java/src/org/openqa/selenium/docker/client/CreateContainer.java[59-80]
- java/src/org/openqa/selenium/docker/client/InspectContainer.java[56-88]
- java/src/org/openqa/selenium/docker/client/V140Adapter.java[67-108]
- java/src/org/openqa/selenium/docker/client/V144Adapter.java[78-151]
- java/src/org/openqa/selenium/docker/client/V148Adapter.java[112-310]

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


3. NullMarked conflicts in fromJson 🐞 Bug ✓ Correctness
Description
With org.openqa.selenium.docker.internal now @NullMarked, ImageSummary.fromJson assigns null to
non-null locals and assigns @Nullable JsonInput.read results into non-null variables, then passes
possibly-null id into a constructor that requires non-null. This undermines the new nullness model
and is likely to trigger NullAway warnings/errors when enabled.
Code

java/src/org/openqa/selenium/docker/internal/package-info.java[R18-21]

+@NullMarked
+package org.openqa.selenium.docker.internal;
+
+import org.jspecify.annotations.NullMarked;
Evidence
The package is marked @NullMarked, so unannotated locals are considered non-null. However `ImageId
id = null; and List<String> tags = input.read(...)` (read is @Nullable) violate that assumption,
and the constructor requires a non-null id via Require.nonNull.

java/src/org/openqa/selenium/docker/internal/package-info.java[18-21]
java/src/org/openqa/selenium/docker/internal/ImageSummary.java[51-81]
java/src/org/openqa/selenium/json/JsonInput.java[449-456]
java/src/org/openqa/selenium/docker/internal/ImageSummary.java[38-41]
java/private/java_library.bzl[16-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
`ImageSummary.fromJson` currently violates the non-null-by-default model introduced by `@NullMarked` for `org.openqa.selenium.docker.internal`.

## Issue Context
`JsonInput.read(Type)` is `@Nullable`, and the code also initializes `id` to null before reading.

## Fix Focus Areas
- java/src/org/openqa/selenium/docker/internal/package-info.java[18-21]
- java/src/org/openqa/selenium/docker/internal/ImageSummary.java[51-81]
- java/src/org/openqa/selenium/json/JsonInput.java[449-463]

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



Remediation recommended

4. Wrong ContainerLogs toString 🐞 Bug ✧ Quality
Description
ContainerLogs.toString() returns a string starting with "ContainerInfo{...}", which is misleading
during debugging/logging and was modified in this PR. The label should match the actual class name.
Code

java/src/org/openqa/selenium/docker/ContainerLogs.java[43]

+    return String.format("ContainerInfo{containerLogs=%s, id=%s}", logLines, id);
Evidence
The class is ContainerLogs, but the string literal says ContainerInfo, so logs/debug output will
misidentify the object type.

java/src/org/openqa/selenium/docker/ContainerLogs.java[23-45]

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

## Issue description
`ContainerLogs.toString()` mislabels the class as `ContainerInfo`.

## Issue Context
This impacts log readability and debugging.

## Fix Focus Areas
- java/src/org/openqa/selenium/docker/ContainerLogs.java[41-44]

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@asolntsev asolntsev force-pushed the nullability-in-docker branch from 98025a6 to f933980 Compare March 9, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants