Skip to content

Conversation

mk868
Copy link
Contributor

@mk868 mk868 commented Oct 14, 2025

User description

🔗 Related Issues

Related #14291

💥 What does this PR do?

JSpecify annotations added to the:

  • org.openqa.selenium.bidi.permissions.PermissionState

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

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

PR Type

Enhancement


Description

  • Added JSpecify nullability annotations to PermissionState enum

  • Marked findByName method return type as @Nullable

  • Added @NullMarked annotation at class level for null safety

  • Updated build configuration to include JSpecify dependency


Diagram Walkthrough

flowchart LR
  A["PermissionState enum"] --> B["Add @NullMarked annotation"]
  A --> C["Mark findByName as @Nullable"]
  D["BUILD.bazel"] --> E["Add JSpecify dependency"]
Loading

File Walkthrough

Relevant files
Enhancement
PermissionState.java
Add JSpecify nullability annotations to PermissionState enum

java/src/org/openqa/selenium/bidi/permissions/PermissionState.java

  • Added @NullMarked annotation at class level
  • Imported JSpecify annotations (NullMarked, Nullable)
  • Marked findByName method return type as @Nullable
+5/-1     
Dependencies
BUILD.bazel
Add JSpecify dependency to permissions build configuration

java/src/org/openqa/selenium/bidi/permissions/BUILD.bazel

  • Added artifact import to build file
  • Added JSpecify dependency to deps list
+4/-1     

@selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Oct 14, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 14, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Null return handling

Description: The method now explicitly returns @nullable, which may propagate null to callers; ensure
all call sites handle null to avoid NullPointerExceptions.
PermissionState.java [40-44]

Referred Code
public static @Nullable PermissionState findByName(String name) {
  PermissionState result = null;
  for (PermissionState state : values()) {
    if (state.toString().equalsIgnoreCase(name)) {
      result = state;
Ticket Compliance
🟢
🎫 #14291
🟢 Add JSpecify Nullness annotations to Selenium Java code to indicate nullable parameters
and return values.
Ensure methods that can return null are annotated with @nullable.
Apply appropriate class- or package-level nullness defaults (e.g., @NullMarked) for
clarity.
Include necessary build/dependency changes so annotations compile and are available.
Improve interoperability with IDEs/static analyzers by leveraging JSpecify annotations.
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

@mk868 mk868 force-pushed the jspecify-bidi-permissions branch from 636a2da to faf5e89 Compare October 14, 2025 17:04
Copy link
Contributor

qodo-merge-pro bot commented Oct 14, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Pin dependency version explicitly

Pin the JSpecify dependency with an explicit version (and bounds if applicable)
to ensure deterministic builds.

java/src/org/openqa/selenium/bidi/permissions/BUILD.bazel [17-19]

 deps = [
-    artifact("org.jspecify:jspecify"),
+    artifact("org.jspecify:jspecify:1.0.0"),
 ],
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Use correct dependency spec syntax with explicit versions and bounds for build reproducibility.

Low
General
Simplify method by returning directly

Refactor the findByName method to improve readability by returning directly from
the loop upon finding a match, thus eliminating the need for a temporary
variable and a break statement.

java/src/org/openqa/selenium/bidi/permissions/PermissionState.java [40-49]

 public static @Nullable PermissionState findByName(String name) {
-  PermissionState result = null;
   for (PermissionState state : values()) {
     if (state.toString().equalsIgnoreCase(name)) {
-      result = state;
-      break;
+      return state;
     }
   }
-  return result;
+  return null;
 }
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly proposes simplifying the findByName method by using an early return, which improves code readability and conciseness without changing the logic.

Low
  • Update

@mk868 mk868 force-pushed the jspecify-bidi-permissions branch from faf5e89 to f039b3f Compare October 15, 2025 17:53
@mk868 mk868 force-pushed the jspecify-bidi-permissions branch from f039b3f to 7f171dc Compare October 16, 2025 06:47
@diemol diemol enabled auto-merge (squash) October 16, 2025 22:45
@diemol diemol merged commit 10e563a into SeleniumHQ:trunk Oct 16, 2025
25 checks passed
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-devtools Includes everything BiDi or Chrome DevTools related C-java Java Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants