Skip to content

Conversation

@spencerarq
Copy link

@spencerarq spencerarq commented Nov 22, 2025

User description

🔗 Related Issues

Fixes: #14291
(This PR addresses the ongoing effort to adopt JSpecify annotations for null-safety.)

💥 What does this PR do?

This PR adds JSpecify annotations to the org.openqa.selenium.chromium.ChromiumDriver class, contributing to the improved null-safety of the Java core codebase.

🔧 Implementation Notes

1. Annotation: The ChromiumDriver class was annotated with @NullMarked (enforcing non-null defaults for parameters and return types).
2. Build Fix: The infrastructure required an update. I added org.jspecify:jspecify dependency to the chromium target in BUILD.bazel to resolve compilation dependencies.
3. Motivation: This provides better static analysis accuracy and improves Developer Experience (DX) by making explicit nullability contracts for the Chromium-based driver.

💡 Additional Considerations

Verification: The module was built successfully using Bazel to ensure dependency resolution and compilation:
bazel build //java/src/org/openqa/selenium/chromium:chromium
The build completed successfully.

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement


Description

  • Add JSpecify @NullMarked annotation to ChromiumDriver class

  • Mark nullable fields casting and cdp with @Nullable

  • Add org.jspecify:jspecify dependency to BUILD.bazel


Diagram Walkthrough

flowchart LR
  A["ChromiumDriver class"] -->|"Add @NullMarked"| B["Enforce non-null defaults"]
  A -->|"Mark nullable fields"| C["casting and cdp fields"]
  D["BUILD.bazel"] -->|"Add jspecify dependency"| E["Enable JSpecify support"]
Loading

File Walkthrough

Relevant files
Enhancement
ChromiumDriver.java
Add JSpecify null-safety annotations                                         

java/src/org/openqa/selenium/chromium/ChromiumDriver.java

  • Added JSpecify imports (@NullMarked and @Nullable)
  • Applied @NullMarked class-level annotation for null-safety enforcement
  • Annotated casting and cdp fields with @Nullable to indicate they can
    be null
+6/-2     
Configuration changes
BUILD.bazel
Add JSpecify dependency to build configuration                     

java/src/org/openqa/selenium/chromium/BUILD.bazel

  • Added org.jspecify:jspecify artifact dependency
  • Updated load statement to include artifact and java_library functions
+2/-0     

@selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations labels Nov 22, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 22, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No new logging: The added annotations and dependency changes do not add or modify any critical action
logging, so audit trail coverage is unaffected and cannot be assessed from these diffs.

Referred Code
@NullMarked
public class ChromiumDriver extends RemoteWebDriver
    implements HasAuthentication,
        HasBiDi,
        HasCasting,
        HasCdp,
        HasDevTools,
        HasLaunchApp,
        HasLogEvents,
        HasNetworkConditions,
        HasPermissions {

  public static final Predicate<String> IS_CHROMIUM_BROWSER =
      name -> CHROME.is(name) || EDGE.is(name) || OPERA.is(name);
  private static final Logger LOG = Logger.getLogger(ChromiumDriver.class.getName());

  private final Capabilities capabilities;
  private final HasNetworkConditions networkConditions;
  private final HasPermissions permissions;
  private final HasLaunchApp launch;
  private Optional<Connection> connection;


 ... (clipped 6 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Nullability change: Fields casting and cdp were marked @nullable without showing corresponding reads or null
checks, so it is unclear from this diff whether all usages handle null safely.

Referred Code
protected @Nullable HasCasting casting;
protected @Nullable HasCdp cdp;
private final Map<Integer, ScriptKey> scriptKeys = new HashMap<>();

Learn more about managing compliance generic rules or creating your own custom rules

  • 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 Nov 22, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Make nullable fields final

Make the casting and cdp fields final to enforce immutability, preventing them
from being reassigned after initialization.

java/src/org/openqa/selenium/chromium/ChromiumDriver.java [97-98]

-protected @Nullable HasCasting casting;
-protected @Nullable HasCdp cdp;
+protected final @Nullable HasCasting casting;
+protected final @Nullable HasCdp cdp;
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the casting and cdp fields can be made final to improve immutability, which is a good practice and consistent with other fields in the class.

Low
Learned
best practice
Pin dependency version
Suggestion Impact:The dependency was updated from an unpinned artifact to explicitly specify version 1.0.0.

code diff:

-        artifact("org.jspecify:jspecify"),
+        artifact("org.jspecify:jspecify:1.0.0"),

Pin the jspecify artifact to a specific version to avoid accidental upgrades and
non-reproducible builds.

java/src/org/openqa/selenium/chromium/BUILD.bazel [23]

-artifact("org.jspecify:jspecify"),
+artifact("org.jspecify:jspecify:1.0.0"),

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Guard external dependencies with explicit versioning to ensure reproducible builds.

Low
Document nullable fields
Suggestion Impact:The commit added the exact Javadoc comments above the nullable 'casting' and 'cdp' fields as suggested.

code diff:

+  /** May be null when the driver does not support casting; initialized during setup if available. */
   protected @Nullable HasCasting casting;
+  /** May be null when CDP is unavailable for the current browser/session. */
   protected @Nullable HasCdp cdp;

Document that casting and cdp may be null and describe when they are initialized
or remain absent to align with the new annotations.

java/src/org/openqa/selenium/chromium/ChromiumDriver.java [97-98]

+/** May be null when the driver does not support casting; initialized during setup if available. */
 protected @Nullable HasCasting casting;
+/** May be null when CDP is unavailable for the current browser/session. */
 protected @Nullable HasCdp cdp;

[Suggestion processed]

Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Enforce accurate and consistent documentation and naming to reflect nullability behavior.

Low
  • Update

@spencerarq
Copy link
Author

Thanks for the suggestions! I've reviewed them and pushed an update:

  1. final Modifier: I opted not to add final to the fields as it would enforce immediate initialization, changing the class design beyond the scope of this PR (which is strictly focused on JSpecify annotations).
  2. Javadoc: I applied the suggested documentation to clarify the nullability of casting and cdp.
  3. Build: I also pinned the jspecify dependency version to 1.0.0 in BUILD.bazel to ensure reproducible builds.

I've verified locally (WSL2) that the target //java/test/org/openqa/selenium/chromium:LoggingTest compiles and passes. Ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-java Java Bindings Review effort 1/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🚀 Feature]: JSpecify Nullness annotations for Java

2 participants