Skip to content

[java] Add nullness for interactions v2 #15106

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

Merged
merged 3 commits into from
Jan 20, 2025

Conversation

mk868
Copy link
Contributor

@mk868 mk868 commented Jan 17, 2025

User description

Description

In this PR I'm adding nullness annotations for classes:

  • org.openqa.selenium.interactions.Actions
  • org.openqa.selenium.interactions.Pause
  • org.openqa.selenium.interactions.Sequence
  • org.openqa.selenium.interactions.WheelInput

NullAway analysis: #14421

Motivation and Context

The JSpecify nullness annotations will give developers better exposure to potential problems with their code to avoid NullPointerExceptions.
Related issue: #14291

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement


Description

  • Added JSpecify @NullMarked annotations to improve nullness checks.

  • Introduced @Nullable annotations for nullable fields and parameters.

  • Enhanced null safety by using Require.nonNull for critical getters.

  • Improved IDE and static analysis support for nullability.


Changes walkthrough 📝

Relevant files
Enhancement
Actions.java
Add nullness annotations and enforce null checks                 

java/src/org/openqa/selenium/interactions/Actions.java

  • Added @NullMarked annotation to the class.
  • Marked fields activePointer, activeKeyboard, and activeWheel as
    @Nullable.
  • Updated getter methods to use Require.nonNull for null safety.
  • +9/-6     
    Pause.java
    Add nullness annotation to Pause class                                     

    java/src/org/openqa/selenium/interactions/Pause.java

    • Added @NullMarked annotation to the class.
    +2/-0     
    Sequence.java
    Add nullness annotation to Sequence class                               

    java/src/org/openqa/selenium/interactions/Sequence.java

    • Added @NullMarked annotation to the class.
    +2/-0     
    WheelInput.java
    Add nullness annotations to WheelInput class                         

    java/src/org/openqa/selenium/interactions/WheelInput.java

  • Added @NullMarked annotation to the class.
  • Marked constructor parameter name as @Nullable.
  • +4/-1     

    Need help?
  • Type /help how to ... in the comments thread for any question about Qodo Merge usage.
  • Check out the documentation for more information.
  • @mk868
    Copy link
    Contributor Author

    mk868 commented Jan 17, 2025

    Fixed NullAway error:

    java/src/org/openqa/selenium/interactions/Actions.java:63: error: [NullAway] initializer method does not guarantee @NonNull fields activePointer (line 54), activeKeyboard (line 55), activeWheel (line 56) are initialized along all control-flow paths (remember to check for exceptions or early returns).
      public Actions(WebDriver driver, Duration duration) {
             ^
        (see http://t.uber.com/nullaway )
    

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    14291 - PR Code Verified

    Compliant requirements:

    • Add JSpecify nullness annotations to Selenium framework code
    • Annotations specify which parameters and return values can be null
    • Annotations are transparent to IDEs and static code analyzers

    Requires further human verification:

    • Verify improved interoperability with Kotlin
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Null Safety

    Verify that the null checks in getActiveKeyboard(), getActivePointer(), and getActiveWheel() methods are sufficient and won't cause unexpected NullPointerExceptions in edge cases

    public KeyInput getActiveKeyboard() {
      if (this.activeKeyboard == null) {
        setActiveKeyboard("default keyboard");
      }
      return Require.nonNull("ActiveKeyboard", this.activeKeyboard);
    }
    
    public PointerInput getActivePointer() {
      if (this.activePointer == null) {
        setActivePointer(PointerInput.Kind.MOUSE, "default mouse");
      }
      return Require.nonNull("ActivePointer", this.activePointer);
    }
    
    public WheelInput getActiveWheel() {
      if (this.activeWheel == null) {
        setActiveWheel("default wheel");
      }
      return Require.nonNull("ActiveWheel", this.activeWheel);
    }

    Copy link
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @VietND96 VietND96 merged commit 6141259 into SeleniumHQ:trunk Jan 20, 2025
    33 of 34 checks passed
    @mk868 mk868 deleted the jspecify-interactions branch January 20, 2025 06:52
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Mar 23, 2025
    gryznar pushed a commit to gryznar/selenium that referenced this pull request May 17, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants