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.support.ui.ExpectedCondition
  • org.openqa.selenium.support.ui.ExpectedConditions

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

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

PR Type

Enhancement


Description

  • Add JSpecify null-safety annotations to ExpectedCondition interface

  • Add JSpecify null-safety annotations to ExpectedConditions utility class

  • Mark methods returning nullable types with @Nullable annotation

  • Add null-check in urlMatches() method to handle nullable URL


Diagram Walkthrough

flowchart LR
  A["ExpectedCondition<br/>Interface"] -->|"Add @NullMarked<br/>and @Nullable"| B["Null-safe<br/>Type Parameters"]
  C["ExpectedConditions<br/>Utility Class"] -->|"Add @NullMarked<br/>and @Nullable"| D["Null-safe<br/>Method Returns"]
  D -->|"Add null-check<br/>in urlMatches()"| E["Runtime Safety"]
Loading

File Walkthrough

Relevant files
Documentation
ExpectedCondition.java
Add JSpecify annotations to ExpectedCondition interface   

java/src/org/openqa/selenium/support/ui/ExpectedCondition.java

  • Add @NullMarked class-level annotation for null-safety
  • Import JSpecify annotations (@NullMarked, @Nullable)
  • Update generic type parameter to T extends @Nullable Object to allow
    nullable return types
+4/-1     
ExpectedConditions.java
Add JSpecify null-safety annotations throughout ExpectedConditions

java/src/org/openqa/selenium/support/ui/ExpectedConditions.java

  • Add @NullMarked class-level annotation for null-safety
  • Import JSpecify annotations (@NullMarked, @Nullable)
  • Mark 30+ method return types with @Nullable where methods can return
    null
  • Mark local variables with @Nullable annotation (e.g., currentTitle,
    currentUrl, currentValue)
  • Add null-check in urlMatches() method before calling
    pattern.matcher(currentUrl)
+80/-74 
Dependencies
BUILD.bazel
Add JSpecify dependency to build configuration                     

java/src/org/openqa/selenium/support/ui/BUILD.bazel

  • Add org.jspecify:jspecify artifact dependency to support JSpecify
    annotations
+1/-0     

@selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations B-support Issue or PR related to support classes labels Oct 22, 2025
@qodo-merge-pro
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Behavior change risk

Description: The new null check in urlMatches() changes behavior by returning false when currentUrl is
null, which is likely correct but may silently mask unexpected nulls; verify that
driver.getCurrentUrl() can be null and that this change does not hide errors in existing
consumers relying on previous behavior.
ExpectedConditions.java [146-153]

Referred Code
private final Pattern pattern = Pattern.compile(regex);
private @Nullable String currentUrl;

@Override
public Boolean apply(WebDriver driver) {
  currentUrl = driver.getCurrentUrl();
  return currentUrl != null && pattern.matcher(currentUrl).find();
}
Ticket Compliance
🟢
🎫 #14291
🟢 Add JSpecify Nullness annotations to Selenium Java code to indicate nullable parameters
and return values.
Ensure IDEs and static analyzers can detect potential nullability issues via annotations
(e.g., @nullable).
Apply annotations to relevant APIs (example shows interface methods like
WebElement.getAttribute returning @nullable).
Maintain runtime safety where necessary when handling potentially null values.
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

@selenium-ci
Copy link
Member

Thank you, @mk868 for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Null-check before regex match

Add a null check for currentValue before calling pattern.matcher(...) to avoid
potential NPEs when getText() returns null.

java/src/org/openqa/selenium/support/ui/ExpectedConditions.java [922-933]

 public static ExpectedCondition<Boolean> textMatches(final By locator, final Pattern pattern) {
   return new ExpectedCondition<Boolean>() {
     private @Nullable String currentValue = null;
 
     @Override
     public Boolean apply(WebDriver driver) {
       try {
         currentValue = driver.findElement(locator).getText();
-        return pattern.matcher(currentValue).find();
+        return currentValue != null && pattern.matcher(currentValue).find();
       } catch (Exception e) {
         return false;
       }
     }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate inputs and states early to prevent NPEs and logic errors by guarding nullable values before use.

Low
  • More

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

Labels

B-build Includes scripting, bazel and CI integrations B-support Issue or PR related to support classes C-java Java Bindings Review effort 3/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants