Skip to content

Conversation

iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Sep 27, 2025

User description

🔗 Related Issues

#14291

💥 What does this PR do?

This pull request introduces nullness annotations to the remote execution interfaces in the Selenium codebase, improving type safety and clarifying nullability expectations for method parameters and return values.

Nullness annotations and type safety:

  • Added @NullMarked and @Nullable annotations from org.jspecify.annotations to the ExecuteMethod interface in ExecuteMethod.java, marking the interface and its methods for nullness checking.
  • Updated the execute method signature in ExecuteMethod.java to explicitly allow null for both the parameters argument and the return value by adding @Nullable.
  • Added the @NullMarked annotation to the RemoteExecuteMethod class in RemoteExecuteMethod.java, ensuring nullness is enforced throughout the class.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Enhancement


Description

  • Added @NullMarked annotations to ExecuteMethod interface and RemoteExecuteMethod class

  • Added @Nullable annotations to method parameters and return values

  • Improved type safety with nullness checking annotations


Diagram Walkthrough

flowchart LR
  A["ExecuteMethod interface"] -- "add @NullMarked" --> B["Type-safe interface"]
  A -- "add @Nullable to execute method" --> C["Nullable parameters/return"]
  D["RemoteExecuteMethod class"] -- "add @NullMarked" --> E["Type-safe implementation"]
Loading

File Walkthrough

Relevant files
Enhancement
ExecuteMethod.java
Add nullability annotations to ExecuteMethod interface     

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

  • Added @NullMarked annotation to interface
  • Added @Nullable annotations to execute method parameters and return
    value
  • Imported org.jspecify.annotations classes
+4/-1     
RemoteExecuteMethod.java
Add nullability annotation to RemoteExecuteMethod class   

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

  • Added @NullMarked annotation to class
  • Imported org.jspecify.annotations.NullMarked
+2/-0     

@selenium-ci selenium-ci added the C-java Java Bindings label Sep 27, 2025
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

API Change

The signature of execute now allows null for parameters and return value; verify all implementors and call sites handle null correctly and that this does not break binary/source compatibility or NPE assumptions.

  @Nullable Object execute(String commandName, @Nullable Map<String, ?> parameters);
}
Consistency

With @NullMarked on the class, ensure overrides of execute and internal usages align with the new @Nullable contract from the interface and properly handle null parameters/returns.

@NullMarked
public class RemoteExecuteMethod implements ExecuteMethod, WrapsDriver {

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Clarify and limit nullness annotations

Since @NullMarked makes types non-null by default, only annotate parameters or
return types that are truly nullable; avoid annotating non-null parameters like
commandName as implicitly nullable and document nullability in Javadoc.

java/src/org/openqa/selenium/remote/ExecuteMethod.java [28-37]

 @NullMarked
 public interface ExecuteMethod {
-  ...
+  /**
+   * Execute the given command on the remote webdriver server. Any exceptions will be thrown by the
+   * underlying execute method.
+   *
+   * @param commandName The remote command to execute (must not be null)
+   * @param parameters The parameters to execute that command with; may be null
+   * @return The result of {@link Response#getValue()}; may be null
+   */
   @Nullable Object execute(String commandName, @Nullable Map<String, ?> parameters);
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Apply nullness annotations consistently and avoid redundant or misleading annotations under a @NullMarked scope.

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