Skip to content

Conversation

@mk868
Copy link
Contributor

@mk868 mk868 commented Oct 21, 2025

User description

🔗 Related Issues

Related #14291

💥 What does this PR do?

JSpecify annotations added to the:

  • org.openqa.selenium.remote.FileDetector
  • org.openqa.selenium.remote.LocalFileDetector
  • org.openqa.selenium.remote.UselessFileDetector

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement, Documentation


Description

  • Add JSpecify null-safety annotations to FileDetector interface

  • Annotate LocalFileDetector and UselessFileDetector implementations

  • Mark getLocalFile() return type as @nullable

  • Apply @NullMarked to interface and implementations for consistency


Diagram Walkthrough

flowchart LR
  FileDetector["FileDetector<br/>interface"]
  LocalFileDetector["LocalFileDetector<br/>implementation"]
  UselessFileDetector["UselessFileDetector<br/>implementation"]
  JSpecify["JSpecify<br/>annotations"]
  
  JSpecify -- "@NullMarked" --> FileDetector
  JSpecify -- "@Nullable" --> FileDetector
  JSpecify -- "@NullMarked" --> LocalFileDetector
  JSpecify -- "@Nullable" --> LocalFileDetector
  JSpecify -- "@Nullable" --> UselessFileDetector
Loading

File Walkthrough

Relevant files
Documentation
FileDetector.java
Add JSpecify annotations to FileDetector interface             

java/src/org/openqa/selenium/remote/FileDetector.java

  • Import JSpecify annotations (@NullMarked and @Nullable)
  • Add @NullMarked class-level annotation to interface
  • Annotate getLocalFile() return type with @Nullable
+4/-1     
LocalFileDetector.java
Add JSpecify annotations to LocalFileDetector                       

java/src/org/openqa/selenium/remote/LocalFileDetector.java

  • Import JSpecify annotations (@NullMarked and @Nullable)
  • Add @NullMarked class-level annotation
  • Annotate getLocalFile() return type with @Nullable
+4/-1     
UselessFileDetector.java
Add JSpecify null-safety annotation to UselessFileDetector

java/src/org/openqa/selenium/remote/UselessFileDetector.java

  • Import JSpecify @Nullable annotation
  • Annotate getLocalFile() return type with @Nullable
+2/-1     

@selenium-ci selenium-ci added the C-java Java Bindings label Oct 21, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 21, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Null return contract

Description: Returning null from a method annotated @nullable without clear documentation may still
lead to NPEs if callers ignore the annotation; verify downstream usage and null checks.
UselessFileDetector.java [26-27]

Referred Code
public @Nullable File getLocalFile(CharSequence... keys) {
  return null;
Ticket Compliance
🟡
🎫 #14291
🟢 Add JSpecify nullness annotations to Selenium Java code to indicate nullable parameters
and return values.
Prefer class-level/package-level nullness defaults (e.g., @NullMarked) for consistency.
Ensure IDEs and static analyzers can detect potential NPEs through these annotations.
🔴 Annotate methods like getAttribute in interfaces/classes to reflect potential nullability
using @nullable.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 21, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Add missing class-level nullness annotation

Add the @NullMarked annotation to the UselessFileDetector class to align with
the FileDetector interface and other implementations.

java/src/org/openqa/selenium/remote/UselessFileDetector.java [20-29]

 import java.io.File;
+import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 
 /** A file detector that never finds anything. */
+@NullMarked
 public class UselessFileDetector implements FileDetector {
   @Override
   public @Nullable File getLocalFile(CharSequence... keys) {
     return null;
   }
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the @NullMarked annotation is missing on the UselessFileDetector class, which is inconsistent with the implemented interface and another implementation in the PR, thus improving code consistency.

Low
Learned
best practice
Guard against null inputs

Add defensive null checks for keys and its elements to avoid potential NPEs when
appending to the StringBuilder.

java/src/org/openqa/selenium/remote/LocalFileDetector.java [32-36]

 public @Nullable File getLocalFile(CharSequence... keys) {
+  if (keys == null || keys.length == 0) {
+    return null;
+  }
   StringBuilder builder = new StringBuilder();
   for (CharSequence chars : keys) {
-    builder.append(chars);
+    if (chars != null) {
+      builder.append(chars);
+    }
   }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate inputs and states early to prevent NPEs and logic errors.

Low
  • Update

@mk868 mk868 force-pushed the jspecify-FileDetector branch from 345db4f to 6911765 Compare October 22, 2025 06:58
@diemol diemol merged commit 345bee3 into SeleniumHQ:trunk Oct 22, 2025
38 checks passed
@mk868 mk868 deleted the jspecify-FileDetector branch October 22, 2025 12:58
This was referenced Oct 25, 2025
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.

3 participants