Skip to content

[java] added annotations and applied format.sh exception classes pt2 #16025

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 1 commit 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

🔧 Implementation Notes

This pull request introduces nullability annotations to improve type safety and clarify the handling of nullable values in several exception classes within the Selenium Java codebase. The changes ensure that parameters and fields that can accept or return null are explicitly marked as nullable, while the classes themselves are marked as null-safe.

Nullability Annotations Added to Exception Classes:

  • NoSuchSessionException:

    • Added @NullMarked annotation to the class.
    • Marked the reason parameter in constructors as @Nullable.
  • ScriptTimeoutException:

    • Added @NullMarked annotation to the class.
    • Marked the message and cause parameters in constructors as @Nullable.
  • SessionNotCreatedException:

    • Added @NullMarked annotation to the class.
    • Marked the msg and cause parameters in constructors as @Nullable.
  • TimeoutException:

    • Added @NullMarked annotation to the class.
    • Marked the message and cause parameters in constructors as @Nullable.
  • UnhandledAlertException:

    • Added @NullMarked annotation to the class.
    • Marked the alertText field and the message and alertText parameters in constructors as @Nullable.
    • Updated the getAlertText method to return @Nullable.

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Enhancement


Description

  • Added JSpecify nullability annotations to exception classes

  • Marked classes as @NullMarked for null safety

  • Annotated constructor parameters and fields as @Nullable

  • Enhanced type safety for exception handling


Changes diagram

flowchart LR
  A["Exception Classes"] --> B["@NullMarked Annotation"]
  A --> C["@Nullable Parameters"]
  B --> D["Type Safety"]
  C --> D
Loading

Changes walkthrough 📝

Relevant files
Enhancement
NoSuchSessionException.java
Added nullability annotations to NoSuchSessionException   

java/src/org/openqa/selenium/NoSuchSessionException.java

  • Added @NullMarked class annotation
  • Marked reason constructor parameters as @Nullable
  • +6/-2     
    ScriptTimeoutException.java
    Added nullability annotations to ScriptTimeoutException   

    java/src/org/openqa/selenium/ScriptTimeoutException.java

  • Added @NullMarked class annotation
  • Marked message and cause constructor parameters as @Nullable
  • +7/-3     
    SessionNotCreatedException.java
    Added nullability annotations to SessionNotCreatedException

    java/src/org/openqa/selenium/SessionNotCreatedException.java

  • Added @NullMarked class annotation
  • Marked msg and cause constructor parameters as @Nullable
  • +6/-2     
    TimeoutException.java
    Added nullability annotations to TimeoutException               

    java/src/org/openqa/selenium/TimeoutException.java

  • Added @NullMarked class annotation
  • Marked message and cause constructor parameters as @Nullable
  • +7/-3     
    UnhandledAlertException.java
    Added nullability annotations to UnhandledAlertException 

    java/src/org/openqa/selenium/UnhandledAlertException.java

  • Added @NullMarked class annotation
  • Marked alertText field and constructor parameters as @Nullable
  • Updated getAlertText method return type as @Nullable
  • +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.
  • Copy link
    Contributor

    qodo-merge-pro 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

    Null Handling

    The constructor logic concatenates a nullable msg parameter directly in string operations without null checks, which could result in "null" being displayed in error messages when msg is null.

    public SessionNotCreatedException(@Nullable String msg) {
      super(
          "Could not start a new session. "
              + msg
              + (msg != null && msg.contains("Host info") ? "" : " \n" + getHostInformation()));
    }
    Null Concatenation

    The constructor concatenates nullable message and alertText parameters directly without null checks, potentially resulting in "null" strings in the exception message.

    public UnhandledAlertException(@Nullable String message, @Nullable String alertText) {
      super(message + ": " + alertText);
      this.alertText = alertText;
    }

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 8, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Handle null string concatenation safely

    The string concatenation with a nullable msg parameter can result in "null"
    being literally concatenated to the message. Consider using a null-safe approach
    to avoid displaying "null" in error messages.

    java/src/org/openqa/selenium/SessionNotCreatedException.java [26-31]

     public SessionNotCreatedException(@Nullable String msg) {
       super(
           "Could not start a new session. "
    -          + msg
    +          + (msg != null ? msg : "")
               + (msg != null && msg.contains("Host info") ? "" : " \n" + getHostInformation()));
     }
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies that concatenating a null string msg will result in the literal "null" in the exception message, and the proposed fix prevents this, improving message quality.

    Medium
    • 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.

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