[java] Add nullness for AppCacheStatus, Credential, and Either

[mk868]

User description

Description

Changed files

• org.openqa.selenium.html5.AppCacheStatus - nullable annotations added
• org.openqa.selenium.internal.Either - nullable annotations added (docs: Declaring generic types)
• org.openqa.selenium.virtualauthenticator.Credential - added null value checking to avoid unexpected NPEs

NullAway analysis: #14421

Motivation and Context

The JSpecify nullness annotations will give developers better exposure to potential problems with their code to avoid NullPointerExceptions.
Related issue: #14291

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Bug fix

---

Description

• Added JSpecify nullness annotations to improve null safety.

  • Annotated AppCacheStatus enum with @NullMarked and @Nullable.
  • Enhanced Either class with nullness annotations for generic types.

• Improved null value handling in Credential.fromMap method.

  • Added explicit null checks for map values.
  • Enhanced readability and safety of credential creation logic.

---

Changes walkthrough 📝

	Relevant files
Enhancement	AppCacheStatus.java Add nullness annotations to AppCacheStatus                              java/src/org/openqa/selenium/html5/AppCacheStatus.java Added @NullMarked annotation to the class. Marked getEnum methods with @Nullable for null safety. +6/-2      Either.java Add nullness annotations to Either class                                  java/src/org/openqa/selenium/internal/Either.java Added @NullMarked annotation to the class. Enhanced generic types with @Nullable to indicate nullability. +6/-3
Bug fix	Credential.java Improve null handling in Credential.fromMap                            java/src/org/openqa/selenium/virtualauthenticator/Credential.java Added explicit null checks for map values in fromMap method. Improved safety and clarity of credential creation logic. +13/-6

---

Need help?

Type /help how to ... in the comments thread for any question about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 14291 - PR Code Verified Compliant requirements: Add JSpecify Nullness annotations to Selenium framework code Annotations should specify which parameters and return values can be null Annotations should be transparent to IDEs and static code analyzers Requires further human verification: Improve interoperability with Kotlin - needs testing with Kotlin codebase

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Type Safety The generic type bounds using @nullable might affect type safety in existing code using the Either class. Need to verify this change doesn't break existing usage patterns. public class Either<A extends @Nullable Object, B extends @Nullable Object> implements Iterable<B> { Error Handling The fromMap method now throws exceptions for null values but doesn't document this behavior in its method javadoc. This could surprise developers. public static Credential fromMap(Map<String, Object> map) { Object credentialId = Require.nonNull("credentialId", map.get("credentialId")); Object isResidentCredential = Require.nonNull("isResidentCredential", map.get("isResidentCredential")); Object rpId = Require.nonNull("rpId", map.get("rpId")); Object privateKey = Require.nonNull("privateKey", map.get("privateKey")); Object userHandle = map.get("userHandle"); Object signCount = Require.nonNull("signCount", map.get("signCount")); Base64.Decoder decoder = Base64.getUrlDecoder(); return new Credential( decoder.decode((String) credentialId), (boolean) isResidentCredential, (String) rpId, new PKCS8EncodedKeySpec(decoder.decode((String) privateKey)), userHandle == null ? null : decoder.decode((String) userHandle), ((Long) signCount).intValue()); }

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Validate map value types Add type validation before casting map values to prevent potential ClassCastException when invalid types are provided. java/src/org/openqa/selenium/virtualauthenticator/Credential.java [74-80] +if (!(credentialId instanceof String) || !(isResidentCredential instanceof Boolean) || + !(rpId instanceof String) || !(privateKey instanceof String) || + !(signCount instanceof Long)) { + throw new IllegalArgumentException("Invalid type in credential map"); +} return new Credential( decoder.decode((String) credentialId), (boolean) isResidentCredential, (String) rpId, new PKCS8EncodedKeySpec(decoder.decode((String) privateKey)), userHandle == null ? null : decoder.decode((String) userHandle), ((Long) signCount).intValue()); Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion adds crucial type validation before casting, preventing potential runtime ClassCastExceptions that could occur if the map contains values of unexpected types. This is a significant safety improvement.	8
	Handle invalid enum values explicitly Add a default case to handle invalid values in the getEnum method to avoid returning null silently. Consider throwing an IllegalArgumentException for invalid inputs. java/src/org/openqa/selenium/html5/AppCacheStatus.java [50-57] public static @Nullable AppCacheStatus getEnum(int value) { for (AppCacheStatus status : AppCacheStatus.values()) { if (value == status.value()) { return status; } } - return null; + throw new IllegalArgumentException("Invalid AppCacheStatus value: " + value); } Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion improves error handling by making invalid enum values fail fast with a clear error message instead of silently returning null, which could lead to NullPointerException issues later.	7

[mk868]

Fixed NullAway errors:

/src/org/openqa/selenium/html5/AppCacheStatus.java:52: error: [NullAway] returning @Nullable expression from method with @NonNull return type
    return null;
    ^
    (see http://t.uber.com/nullaway )
java/src/org/openqa/selenium/html5/AppCacheStatus.java:61: error: [NullAway] returning @Nullable expression from method with @NonNull return type
    return null;
    ^
    (see http://t.uber.com/nullaway )
/src/org/openqa/selenium/virtualauthenticator/Credential.java:69: error: [NullAway] passing @Nullable parameter '(boolean) map.get("isResidentCredential")' where @NonNull is required
        (boolean) map.get("isResidentCredential"),
        ^
    (see http://t.uber.com/nullaway )
java/src/org/openqa/selenium/virtualauthenticator/Credential.java:70: error: [NullAway] passing @Nullable parameter '(String) map.get("rpId")' where @NonNull is required
        (String) map.get("rpId"),
        ^
    (see http://t.uber.com/nullaway )
java/src/org/openqa/selenium/virtualauthenticator/Credential.java:69: error: [NullAway] unboxing of a @Nullable value
        (boolean) map.get("isResidentCredential"),
                         ^
    (see http://t.uber.com/nullaway )
java/src/org/openqa/selenium/virtualauthenticator/Credential.java:73: error: [NullAway] dereferenced expression ((Long) map.get("signCount")) is @Nullable
        ((Long) map.get("signCount")).intValue());
                                     ^
    (see http://t.uber.com/nullaway )

[pujagani]

Thank you @mk868! This looks good to me. Just waiting for the test to pass before I merge it.