Skip to content

[java] specify nullability in packages org.openqa.selenium.chrom*#17152

Merged
asolntsev merged 1 commit intoSeleniumHQ:trunkfrom
asolntsev:nullability
Mar 2, 2026
Merged

[java] specify nullability in packages org.openqa.selenium.chrom*#17152
asolntsev merged 1 commit intoSeleniumHQ:trunkfrom
asolntsev:nullability

Conversation

@asolntsev
Copy link
Contributor

🔗 Related Issues

Partially implements #14291

💥 What does this PR do?

Added JSpecify nullability annotations to packages:

  • org.openqa.selenium.chrome
  • org.openqa.selenium.chromium
  • (and a little bit to org.openqa.selenium.remote, as much as needed for previous two)

🔄 Types of changes

  • Cleanup (formatting, renaming)

@asolntsev asolntsev requested a review from iampopovich March 1, 2026 21:27
@asolntsev asolntsev self-assigned this Mar 1, 2026
@asolntsev asolntsev added this to the 4.42.0 milestone Mar 1, 2026
@selenium-ci selenium-ci added the C-java Java Bindings label Mar 1, 2026
@qodo-code-review
Copy link
Contributor

Review Summary by Qodo

Add JSpecify nullability annotations to chrome and chromium packages

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Add JSpecify nullability annotations to chrome and chromium packages
• Introduce helper methods for safe capability retrieval and execution
• Refactor code to eliminate unsafe casts and improve type safety
• Clean up unused imports and simplify implementation patterns

Grey Divider

File Changes

1. java/src/org/openqa/selenium/Capabilities.java ✨ Enhancement +12/-0

Add generic helper methods for capability access

java/src/org/openqa/selenium/Capabilities.java


2. java/src/org/openqa/selenium/chrome/ChromeDriverInfo.java Cleanup +0/-3

Remove unused logger import

java/src/org/openqa/selenium/chrome/ChromeDriverInfo.java


3. java/src/org/openqa/selenium/chrome/package-info.java ✨ Enhancement +21/-0

Add NullMarked annotation to chrome package

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


View more (12)
4. java/src/org/openqa/selenium/chromium/AddHasCasting.java ✨ Enhancement +5/-3

Replace unsafe casts with safe null handling

java/src/org/openqa/selenium/chromium/AddHasCasting.java


5. java/src/org/openqa/selenium/chromium/AddHasCdp.java ✨ Enhancement +2/-5

Use executeRequired for non-null result handling

java/src/org/openqa/selenium/chromium/AddHasCdp.java


6. java/src/org/openqa/selenium/chromium/AddHasNetworkConditions.java ✨ Enhancement +1/-4

Replace requireNonNull with executeRequired method

java/src/org/openqa/selenium/chromium/AddHasNetworkConditions.java


7. java/src/org/openqa/selenium/chromium/ChromiumDriver.java ✨ Enhancement +4/-9

Add nullability annotations and remove redundant code

java/src/org/openqa/selenium/chromium/ChromiumDriver.java


8. java/src/org/openqa/selenium/chromium/ChromiumDriverLogLevel.java ✨ Enhancement +1/-1

Add nullable annotation to fromString parameter

java/src/org/openqa/selenium/chromium/ChromiumDriverLogLevel.java


9. java/src/org/openqa/selenium/chromium/ChromiumOptions.java ✨ Enhancement +39/-43

Refactor to use self() method and improve type safety

java/src/org/openqa/selenium/chromium/ChromiumOptions.java


10. java/src/org/openqa/selenium/chromium/HasCasting.java 📝 Documentation +1/-1

Update javadoc from array to list terminology

java/src/org/openqa/selenium/chromium/HasCasting.java


11. java/src/org/openqa/selenium/remote/ExecuteMethod.java ✨ Enhancement +7/-1

Add generic type parameter and executeRequired method

java/src/org/openqa/selenium/remote/ExecuteMethod.java


12. java/src/org/openqa/selenium/remote/FedCmDialogImpl.java ✨ Enhancement +18/-16

Add nullability annotations and simplify casting

java/src/org/openqa/selenium/remote/FedCmDialogImpl.java


13. java/src/org/openqa/selenium/remote/LocalExecuteMethod.java ✨ Enhancement +3/-1

Add NullMarked annotation and generic type parameter

java/src/org/openqa/selenium/remote/LocalExecuteMethod.java


14. java/src/org/openqa/selenium/remote/RemoteExecuteMethod.java ✨ Enhancement +3/-2

Add generic type parameter with cast suppression

java/src/org/openqa/selenium/remote/RemoteExecuteMethod.java


15. java/src/org/openqa/selenium/remote/RemoteWebDriver.java ✨ Enhancement +2/-1

Add nullability annotations to executeScript method

java/src/org/openqa/selenium/remote/RemoteWebDriver.java


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 1, 2026

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. ExecuteMethod.execute() made generic 📘 Rule violation ✓ Correctness
Description
The public ExecuteMethod.execute signature was changed to a generic method, which can be
source-incompatible for downstream implementations that previously implemented `Object
execute(...)`. This risks breaking user upgrades without a compatibility strategy.
Code

java/src/org/openqa/selenium/remote/ExecuteMethod.java[R40-44]

+  @Nullable <T> T execute(String commandName, @Nullable Map<String, ?> parameters);
+
+  default <T> T executeRequired(String commandName, @Nullable Map<String, ?> parameters) {
+    return requireNonNull(execute(commandName, parameters));
+  }
Evidence
Compliance requires public interfaces to remain backward compatible; changing
ExecuteMethod.execute from a concrete Object return to a generic <T> T changes the method’s
source-level contract for implementers and callers. The diff shows the updated generic signature on
a public interface method.

AGENTS.md
java/src/org/openqa/selenium/remote/ExecuteMethod.java[40-44]

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

## Issue description
A public interface method (`ExecuteMethod.execute`) was changed to a generic method. This can break downstream implementations that previously implemented `Object execute(String, Map)` and violates the project’s API/ABI compatibility expectations.

## Issue Context
`ExecuteMethod` is a public extension point in `org.openqa.selenium.remote`. To keep upgrades safe, avoid source-incompatible signature changes; prefer adding a new helper method (with a new name) that performs the generic cast and/or required-null enforcement.

## Fix Focus Areas
- java/src/org/openqa/selenium/remote/ExecuteMethod.java[40-44]

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


2. Mutable extensions capability list 🐞 Bug ⛯ Reliability
Description
ChromiumOptions now stores a mutable List for the "extensions" capability inside an otherwise
unmodifiable options map, creating an immutability inconsistency (and allowing unexpected post-hoc
mutation by callers). This can lead to subtle bugs where capability maps appear immutable but
contain mutable nested state.
Code

java/src/org/openqa/selenium/chromium/ChromiumOptions.java[R251-261]

+    options.put("args", List.copyOf(args));
+    options.put("extensions", extensionsArgument());
    options.putAll(androidOptions);

    return unmodifiableMap(options);
  }

+  private List<String> extensionsArgument() {
+    return Stream.concat(extensionFiles.stream().map(this::fileContentBase64), extensions.stream())
+        .collect(toList());
+  }
Evidence
getExtraCapability returns unmodifiableMap(options) but inserts extensionsArgument() directly;
extensionsArgument() uses Collectors.toList() which produces a mutable list. In the same method,
args is explicitly made immutable via List.copyOf(args), suggesting the intent is to return
immutable collections for list-valued capabilities.

java/src/org/openqa/selenium/chromium/ChromiumOptions.java[245-256]
java/src/org/openqa/selenium/chromium/ChromiumOptions.java[258-261]

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

### Issue description
`ChromiumOptions#getExtraCapability` returns an unmodifiable map, but the `&quot;extensions&quot;` value is currently a mutable `List` (built via `Collectors.toList()`), which breaks the implicit immutability expectation established by `&quot;args&quot;` using `List.copyOf(args)`.

### Issue Context
Callers may treat capability maps as effectively immutable; mutating nested collections can produce surprising behavior and makes debugging harder.

### Fix Focus Areas
- java/src/org/openqa/selenium/chromium/ChromiumOptions.java[251-256]
- java/src/org/openqa/selenium/chromium/ChromiumOptions.java[258-261]

ⓘ 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 merged commit cae8d22 into SeleniumHQ:trunk Mar 2, 2026
73 of 77 checks passed
@asolntsev asolntsev deleted the nullability branch March 2, 2026 21:16
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