Skip to content

[java] added jspecify annotations Nullable and NullMarked to exception classes #16024

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.

Sign up for GitHub

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

Jump to bottom
Open
iampopovich wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

[java] added jspecify annotations Nullable and NullMarked to exception classes #16024

iampopovich wants to merge 5 commits into from

Conversation

@iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Jul 8, 2025 •
edited by qodo-code-review bot
Loading

User description

🔗 Related Issues

partially fixes #14291

💥 What does this PR do?

This pull request adds annotations to classes that extend WebDriverException.

To make the changes easier to review, I have split the task of annotating all exception classes into several smaller pull requests. This PR contains updates for 5-7 classes.

🔧 Implementation Notes

This pull request introduces updates to exception classes in the Selenium Java package to improve null safety by integrating jspecify annotations. The changes include marking classes with @NullMarked and updating method parameters to use @Nullable where applicable.

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Enhancement

Description

  • Add @NullMarked annotations to 7 exception classes

  • Add @Nullable annotations to constructor parameters

  • Improve null safety with jspecify annotations

  • Part of larger effort to annotate all exception classes

Changes diagram

flowchart LR
  A["Exception Classes"] --> B["Add @NullMarked"]
  A --> C["Add @Nullable to parameters"]
  B --> D["Improved null safety"]
  C --> D
Loading

Changes walkthrough 📝

Relevant files
Enhancement
7 files
DetachedShadowRootException.java
Add null safety annotations                                                           
+6/-2     
ElementClickInterceptedException.java
Add null safety annotations                                                           
+6/-2     
HealthCheckFailedException.java
Add nullable parameter annotations                                             
+3/-1     
InsecureCertificateException.java
Add null safety annotations                                                           
+7/-3     
InvalidArgumentException.java
Add null safety annotations                                                           
+6/-2     
InvalidElementStateException.java
Add null safety annotations                                                           
+7/-3     
UnsupportedCommandException.java
Add null safety annotations                                                           
+7/-3     
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 the C-java Java Bindings label Jul 8, 2025
    This was referenced Jul 8, 2025
    Open
    Merged
    Closed
    Merged
    @iampopovich iampopovich marked this pull request as ready for review July 8, 2025 19:57
    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jul 8, 2025

    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

    Missing Annotation

    This class is missing the @NullMarked annotation that is present in all other exception classes in this PR, which creates inconsistency in the null safety approach.

     
    public class HealthCheckFailedException extends WebDriverException {

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jul 8, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add missing NullMarked annotation

    Add the @NullMarked annotation to the class to maintain consistency with other
    exception classes in this PR. This ensures uniform null safety annotation
    coverage across all exception classes.

    java/src/org/openqa/selenium/HealthCheckFailedException.java [20-23]

    +import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 
 /** Indicates that a Node health check failed. */
+@NullMarked
 public class HealthCheckFailedException extends WebDriverException {
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies that the @NullMarked annotation is missing on the HealthCheckFailedException class, which is inconsistent with all other exception classes modified in this PR.

    		Medium
    • Update

    pujagani
    pujagani approved these changes Aug 5, 2025
    View reviewed changes
    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

    @iampopovich
    Copy link
    Contributor Author

    iampopovich commented Oct 24, 2025

    @pujagani check please🙏

    Copilot AI review requested due to automatic review settings December 4, 2025 11:53
    Copilot AI reviewed Dec 4, 2025
    View reviewed changes
    Copy link
    Contributor

    Copilot AI left a comment

    Choose a reason for hiding this comment

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

    Pull request overview

    This PR adds jspecify null safety annotations to 7 exception classes in the Selenium Java package, continuing a larger effort to improve type safety across all exception classes. The changes apply the @NullMarked annotation at the class level and @Nullable annotations to constructor parameters that accept nullable String messages and Throwable causes, matching the pattern already established in the parent WebDriverException class.

    Key Changes:

    • Applied @NullMarked class-level annotation to 6 exception classes
    • Added @Nullable annotations to constructor parameters accepting nullable messages and causes
    • Added appropriate jspecify import statements

    Reviewed changes

    Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

    Show a summary per file
    File Description
    DetachedShadowRootException.java Added @NullMarked class annotation and @Nullable to String/Throwable constructor parameters
    ElementClickInterceptedException.java Added @NullMarked class annotation and @Nullable to String/Throwable constructor parameters
    HealthCheckFailedException.java Added @Nullable to constructor parameters (missing @NullMarked annotation)
    InsecureCertificateException.java Added @NullMarked class annotation and @Nullable to String/Throwable constructor parameters
    InvalidArgumentException.java Added @NullMarked class annotation and @Nullable to String/Throwable constructor parameters
    InvalidElementStateException.java Added @NullMarked class annotation and @Nullable to String/Throwable constructor parameters
    UnsupportedCommandException.java Added @NullMarked class annotation and @Nullable to String/Throwable constructor parameters

    💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    java/src/org/openqa/selenium/HealthCheckFailedException.java
    Comment on lines +20 to +21
    import org.jspecify.annotations.Nullable;

    Copy link

    Copilot AI Dec 4, 2025

    Choose a reason for hiding this comment

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

    The @NullMarked annotation is missing from this class. All other exception classes in this PR have the @NullMarked annotation at the class level (see UnsupportedCommandException, InvalidElementStateException, etc.), and the parent class WebDriverException also uses this annotation. For consistency with the rest of the codebase, this class should also include:

    import org.jspecify.annotations.NullMarked;

@NullMarked
public class HealthCheckFailedException extends WebDriverException {
    Suggested change
    import org.jspecify.annotations.Nullable;
    import org.jspecify.annotations.Nullable;
    import org.jspecify.annotations.NullMarked;
    @NullMarked

    Copilot uses AI. Check for mistakes.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    Copilot code review Copilot Copilot left review comments

    @asolntsev asolntsev asolntsev approved these changes

    @pujagani pujagani Awaiting requested review from pujagani

    Assignees

    No one assigned

    Labels

    C-java Java Bindings Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    [🚀 Feature]: JSpecify Nullness annotations for Java

    5 participants

    @iampopovich @asolntsev @pujagani @selenium-ci @diemol