Skip to content

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.

    Copy link
    Contributor

    @pujagani pujagani left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Thank you! @iampopovich

    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
    3 participants