Skip to content

[java] Feat 14291/add jspecify annotations to exception classes pt4 #16028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Jul 8, 2025

User description

🔗 Related Issues
partially fixes #14291

💥 What does this PR do?
same as
#16024
#16025
#16026

🔧 Implementation Notes
This pull request introduces updates to several exception classes across the Selenium codebase to improve null-safety and enhance clarity. The changes primarily involve adding @NullMarked annotations to the classes and @nullable annotations to method parameters where applicable.

Null-safety improvements:
java/src/org/openqa/selenium/devtools/RequestFailedException.java: Added @NullMarked annotation to the class to enforce null-safety.

java/src/org/openqa/selenium/grid/sessionmap/jdbc/JdbcException.java: Added @NullMarked annotation to the class and @nullable annotations to method parameters (message and cause) to handle potential null values.

java/src/org/openqa/selenium/remote/NoSuchDriverException.java: Added @NullMarked annotation to the class and updated constructors to include @nullable annotations for reason and cause parameters.

java/src/org/openqa/selenium/remote/UnreachableBrowserException.java: Added @NullMarked annotation to the class and updated constructors with @nullable annotations for message and cause parameters.

💡 Additional Considerations
🔄 Types of changes
Cleanup (formatting, renaming)


PR Type

Enhancement


Description

  • Add JSpecify null-safety annotations to exception classes

  • Include @NullMarked and @nullable annotations for better IDE support

  • Update build dependencies to include jspecify library

  • Improve Kotlin interoperability and static analysis


Changes diagram

flowchart LR
  A["Exception Classes"] --> B["Add @NullMarked"]
  A --> C["Add @Nullable to parameters"]
  D["BUILD.bazel files"] --> E["Add jspecify dependency"]
  B --> F["Enhanced null-safety"]
  C --> F
  E --> F
Loading

Changes walkthrough 📝

Relevant files
Enhancement
6 files
RequestFailedException.java
Add @NullMarked annotation to class                                           
+2/-0     
JdbcException.java
Add null-safety annotations to constructors                           
+6/-3     
NoSuchDriverException.java
Add @NullMarked and @Nullable annotations                               
+5/-2     
UnreachableBrowserException.java
Add null-safety annotations to constructors                           
+5/-2     
ConnectionFailedException.java
Add @NullMarked and @Nullable annotations                               
+5/-2     
ConnectionClosedException.java
Add null-safety annotations to constructor                             
+4/-1     
Dependencies
3 files
BUILD.bazel
Add jspecify dependency to build                                                 
+2/-0     
BUILD.bazel
Add jspecify dependency to build                                                 
+1/-0     
BUILD.bazel
Add jspecify dependency to build                                                 
+1/-0     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added B-grid Everything grid and server related C-java Java Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Jul 8, 2025
    @iampopovich iampopovich changed the title Feat 14291/add jspecify annotations to exception classes pt4 [java] Feat 14291/add jspecify annotations to exception classes pt4 Jul 8, 2025
    @iampopovich iampopovich marked this pull request as ready for review July 8, 2025 20:01
    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 8, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 0fef631)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    14291 - PR Code Verified

    Compliant requirements:

    • Add JSpecify Nullness annotations to the Selenium framework code
    • Specify which parameters and return values can be null using annotations
    • Use @nullable annotation for parameters that can accept null values

    Requires further human verification:

    • Improve transparency to IDEs and static code analyzers
    • Help developers avoid NullPointerExceptions
    • Improve interoperability with Kotlin

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

    Missing Constructors

    The class only has @NullMarked annotation but no constructors are defined, which may not provide the expected null-safety benefits for exception instantiation

    @NullMarked
    public class RequestFailedException extends WebDriverException {}

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 8, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @iampopovich iampopovich marked this pull request as draft July 9, 2025 08:50
    @iampopovich iampopovich marked this pull request as ready for review July 9, 2025 11:20
    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 9, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    B-devtools Includes everything BiDi or Chrome DevTools related B-grid Everything grid and server related C-java Java Bindings Review effort 2/5
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    [🚀 Feature]: JSpecify Nullness annotations for Java
    2 participants