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.support.Color class, as part of the effort to improve null-safety across the codebase.

🔧 Implementation Notes

1. Annotation: The Color class was annotated with @NullMarked (defaulting parameters and return types to non-null).
2. Build Fix: The infrastructure required an update. I added org.jspecify:jspecify dependency to the support-lib target in BUILD.bazel to resolve the indirect dependency error from Bazel's strict policy.
3. Motivation: This provides better static analysis support and improves Kotlin interoperability by making nullability contracts explicit for developers using the Java bindings.

💡 Additional Considerations

Verification: All related tests were executed successfully using Bazel:
bazel test //java/test/org/openqa/selenium/support:ColorTest
The build completed successfully and 1 test passed.

🔄 Types of changes

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

PR Type

Enhancement, Documentation


Description

  • Add JSpecify @NullMarked and @Nullable annotations to Color class

  • Improve null-safety and Kotlin interoperability with explicit nullability contracts

  • Add org.jspecify:jspecify dependency to support-lib BUILD target

  • Mark Converter.getColor() method return type as nullable


Diagram Walkthrough

flowchart LR
  A["Color class"] -->|"Add @NullMarked"| B["Class-level annotation"]
  A -->|"Add @Nullable"| C["getColor method"]
  D["BUILD.bazel"] -->|"Add dependency"| E["org.jspecify:jspecify"]
  B --> F["Explicit nullability contracts"]
  C --> F
Loading

File Walkthrough

Relevant files
Enhancement
Color.java
Add JSpecify null-safety annotations to Color class           

java/src/org/openqa/selenium/support/Color.java

  • Added @NullMarked class-level annotation to default parameters and
    return types to non-null
  • Added @Nullable annotation to Converter.getColor() method return type
  • Imported JSpecify annotation classes (NullMarked and Nullable)
+4/-1     
Configuration changes
BUILD.bazel
Add JSpecify dependency to BUILD configuration                     

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

  • Added org.jspecify:jspecify artifact dependency to support-lib
    java_export target
  • Resolves indirect dependency error from Bazel's strict dependency
    policy
+1/-0     

@CLAassistant
Copy link

CLAassistant commented Nov 22, 2025

CLA assistant check
All committers have signed the CLA.

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

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: Comprehensive Audit Trails

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

Status:
No audit logs: The added annotations and minor signature change introduce no logging for critical
actions, but the Color utility likely does not perform auditable actions; confirm no
sensitive operations occur here.

Referred Code
@NullMarked
public class Color {
  private final int red;
  private final int green;
  private final int blue;
  private double alpha;

  private static final Converter[] CONVERTERS = {

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: The method return type was changed to @nullable but the diff does not show added
null-handling at call sites or explicit edge-case documentation for null returns.

Referred Code
public @Nullable Color getColor(String value) {
  Matcher matcher = getPattern().matcher(value);
  if (matcher.find()) {
    double a = 1.0;
    if (matcher.groupCount() == 4) {
      a = Double.parseDouble(matcher.group(4));

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:
Input parsing risk: The converter parses strings to numbers without visible validation and now may return
null; while unchanged parsing remains, the nullable contract could require additional
input validation by callers not shown in this diff.

Referred Code
Matcher matcher = getPattern().matcher(value);
if (matcher.find()) {
  double a = 1.0;
  if (matcher.groupCount() == 4) {
    a = Double.parseDouble(matcher.group(4));

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

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

@selenium-ci
Copy link
Member

Thank you, @spencerarq for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

@spencerarq spencerarq changed the title [java] Add JSpecify annotations to Color class [java] Add JSpecify annotations to Color class (Issue #14291) Nov 22, 2025
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 B-support Issue or PR related to support classes 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

3 participants