Skip to content

Conversation

mk868
Copy link
Contributor

@mk868 mk868 commented Oct 18, 2025

User description

🔗 Related Issues

Related #14291

💥 What does this PR do?

JSpecify annotations added to the:

  • org.openqa.selenium.federatedcredentialmanagement.FederatedCredentialManagementAccount
  • org.openqa.selenium.federatedcredentialmanagement.FederatedCredentialManagementDialog
  • org.openqa.selenium.federatedcredentialmanagement.HasFederatedCredentialManagement

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

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

PR Type

Enhancement


Description

  • Add JSpecify null-safety annotations to FedCM classes

  • Mark classes with @NullMarked for comprehensive null checking

  • Annotate nullable fields and return types with @nullable

  • Improve type safety across federated credential management API


Diagram Walkthrough

flowchart LR
  A["FedCM Classes"] -->|Add @NullMarked| B["Class-level annotation"]
  A -->|Add @Nullable| C["Field annotations"]
  A -->|Add @Nullable| D["Return type annotations"]
  B --> E["Enhanced null safety"]
  C --> E
  D --> E
Loading

File Walkthrough

Relevant files
Enhancement
FederatedCredentialManagementAccount.java
Add null-safety annotations to account class                         

java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementAccount.java

  • Added @NullMarked class-level annotation for comprehensive null
    checking
  • Added @Nullable annotations to all nine private String fields
  • Added @Nullable annotations to all nine getter method return types
  • Imported JSpecify annotation classes
+21/-18 
FederatedCredentialManagementDialog.java
Add null-safety annotations to dialog interface                   

java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementDialog.java

  • Added @NullMarked interface-level annotation
  • Added @Nullable annotations to three getter method return types
  • Imported JSpecify annotation classes
  • Methods affected: getDialogType(), getTitle(), getSubtitle()
+6/-3     
HasFederatedCredentialManagement.java
Add null-safety annotations to capability interface           

java/src/org/openqa/selenium/federatedcredentialmanagement/HasFederatedCredentialManagement.java

  • Added @NullMarked interface-level annotation
  • Added @Nullable annotation to getFederatedCredentialManagementDialog()
    return type
  • Imported JSpecify annotation classes
  • Maintains existing @Beta annotation
+4/-1     

@selenium-ci selenium-ci added the C-java Java Bindings label Oct 18, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 18, 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
🟡
🎫 #14291
🟢 Add JSpecify nullness annotations to Selenium Java code to indicate nullable parameters
and return values.
Prefer class/interface-level @NullMarked and annotate nullable members with @nullable.
Apply annotations to relevant APIs, e.g., methods like getters that may return null.
Improve IDE/static analysis visibility for potential nullability issues without changing
behavior.
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

Copy link
Contributor

qodo-merge-pro bot commented Oct 18, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use get() instead of getOrDefault()

Replace dict.getOrDefault(key, null) with dict.get(key) in the constructor. This
change aligns with the class's new @NullMarked annotation, as getOrDefault with
a null default is invalid for a map with a non-nullable value type, and it also
removes a redundant cast.

java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementAccount.java [60-70]

 public FederatedCredentialManagementAccount(Map<String, String> dict) {
-  accountId = (String) dict.getOrDefault("accountId", null);
-  email = (String) dict.getOrDefault("email", null);
-  name = (String) dict.getOrDefault("name", null);
-  givenName = (String) dict.getOrDefault("givenName", null);
-  pictureUrl = (String) dict.getOrDefault("pictureUrl", null);
-  idpConfigUrl = (String) dict.getOrDefault("idpConfigUrl", null);
-  loginState = (String) dict.getOrDefault("loginState", null);
-  termsOfServiceUrl = (String) dict.getOrDefault("termsOfServiceUrl", null);
-  privacyPolicyUrl = (String) dict.getOrDefault("privacyPolicyUrl", null);
+  accountId = dict.get("accountId");
+  email = dict.get("email");
+  name = dict.get("name");
+  givenName = dict.get("givenName");
+  pictureUrl = dict.get("pictureUrl");
+  idpConfigUrl = dict.get("idpConfigUrl");
+  loginState = dict.get("loginState");
+  termsOfServiceUrl = dict.get("termsOfServiceUrl");
+  privacyPolicyUrl = dict.get("privacyPolicyUrl");
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that getOrDefault(key, null) is inconsistent with the newly added @NullMarked annotation, which treats the map's value type as non-nullable. The proposed change to use dict.get() is the correct approach and aligns with the PR's goal of improving null-safety.

Low
  • Update

@mk868 mk868 force-pushed the jspecify-federatedcredentialmanagement branch from daefcd6 to 82f6a3f Compare October 18, 2025 11:25
@mk868 mk868 force-pushed the jspecify-federatedcredentialmanagement branch from 82f6a3f to c272612 Compare October 18, 2025 12:42
@diemol diemol merged commit da46ab0 into SeleniumHQ:trunk Oct 20, 2025
38 checks passed
@mk868 mk868 deleted the jspecify-federatedcredentialmanagement branch October 20, 2025 08:06
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