Skip to content

Conversation

@mk868
Copy link
Contributor

@mk868 mk868 commented Oct 22, 2025

User description

🔗 Related Issues

Related #14291

💥 What does this PR do?

JSpecify annotations added to the:

  • org.openqa.selenium.remote.CommandInfo
  • org.openqa.selenium.remote.CommandPayload
  • org.openqa.selenium.remote.DriverCommand

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement


Description

  • Add JSpecify @NullMarked annotations to remote command classes

  • Mark nullable parameters with @nullable in CommandPayload and DriverCommand

  • Update Map type hints to explicitly denote nullable values


Diagram Walkthrough

flowchart LR
  A["CommandInfo<br/>CommandPayload<br/>DriverCommand"] -->|"Add @NullMarked"| B["Class-level<br/>null annotations"]
  A -->|"Add @Nullable"| C["Parameter &<br/>Map value<br/>annotations"]
  B --> D["Improved null-safety<br/>type checking"]
  C --> D
Loading

File Walkthrough

Relevant files
Enhancement
CommandInfo.java
Add JSpecify null-safety annotation to CommandInfo             

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

  • Add import for org.jspecify.annotations.NullMarked
  • Apply @NullMarked annotation to class declaration
+2/-0     
CommandPayload.java
Add JSpecify annotations and nullable Map types to CommandPayload

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

  • Add imports for @NullMarked and @Nullable annotations
  • Apply @NullMarked to class declaration
  • Update parameters field type to Map
    Object>
  • Update constructor parameter type with nullable Map value annotation
  • Update getParameters() return type with nullable Map value annotation
+6/-3     
DriverCommand.java
Add JSpecify annotations to DriverCommand interface           

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

  • Add imports for @NullMarked and @Nullable annotations
  • Apply @NullMarked to interface declaration
  • Mark frame parameter as @Nullable in SWITCH_TO_FRAME() method
+4/-1     

@selenium-ci selenium-ci added the C-java Java Bindings label Oct 22, 2025
@qodo-merge-pro
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡
🎫 #14291
🟢 Prefer class-level @NullMarked to establish non-null by default semantics.
Mark nullable parameters and return values explicitly with @nullable.
Update collection/map type declarations to reflect potentially nullable values where
applicable.
Ensure improved IDE/static analyzer null-safety without changing runtime behavior.
Add JSpecify nullness annotations across Selenium code to indicate which parameters and
return values can be null.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid NullPointerException with nullable parameters

In SWITCH_TO_FRAME, replace singletonMap with a HashMap to correctly handle the
@Nullable frame parameter and avoid a NullPointerException when the frame is
null.

java/src/org/openqa/selenium/remote/DriverCommand.java [259-261]

 static CommandPayload SWITCH_TO_FRAME(@Nullable Object frame) {
-  return new CommandPayload(SWITCH_TO_FRAME, singletonMap("id", frame));
+  Map<String, @Nullable Object> parameters = new java.util.HashMap<>();
+  parameters.put("id", frame);
+  return new CommandPayload(SWITCH_TO_FRAME, parameters);
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a NullPointerException that will occur when frame is null, as singletonMap does not accept null values, thus fixing a bug introduced by the PR.

High
Learned
best practice
Enforce non-null parameters map

Clarify nullability by documenting whether parameters itself can be null and
enforce non-null for the map while allowing nullable values to prevent
ambiguity.

java/src/org/openqa/selenium/remote/CommandPayload.java [30-33]

 private final Map<String, ? extends @Nullable Object> parameters;
 
 public CommandPayload(String name, Map<String, ? extends @Nullable Object> parameters) {
   this.name = name;
-  this.parameters = parameters;
+  this.parameters = java.util.Objects.requireNonNull(parameters, "parameters must not be null");
 }
 
 public Map<String, ? extends @Nullable Object> getParameters() {
   return parameters;
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Keep API and documentation accurate and consistent by aligning nullness annotations with usage and intent.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants