Skip to content

fix(auth): refine JSpecify nullability annotations for external account credentials - #14164

Open
lqiu96 wants to merge 1 commit into
fix_auth_adc_credentials_nullabilityfrom
fix_auth_external_account_credentials_nullability
Open

fix(auth): refine JSpecify nullability annotations for external account credentials#14164
lqiu96 wants to merge 1 commit into
fix_auth_adc_credentials_nullabilityfrom
fix_auth_external_account_credentials_nullability

Conversation

@lqiu96

@lqiu96 lqiu96 commented Aug 21, 2026

Copy link
Copy Markdown
Member

Stacked on #14162.

Refines JSpecify nullability annotations across ExternalAccountCredentials and subclasses:

  • Type-Use Placement & Nullability: Annotate optional fields, getters, and builder setters/fields with @Nullable across:
    • ExternalAccountCredentials
    • IdentityPoolCredentials
    • AwsCredentials
    • PluggableAuthCredentials
    • ExternalAccountAuthorizedUserCredentials
  • Preconditions: Standardize on com.google.common.base.Preconditions over com.google.api.client.util.Preconditions.
  • Builder Copy Constructors: Ensure builder copy constructors properly preserve all fields.

@lqiu96
lqiu96 requested review from a team as code owners August 21, 2026 22:10

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves nullability safety across several credential classes (including AwsCredentials, ExternalAccountAuthorizedUserCredentials, ExternalAccountCredentials, IdentityPoolCredentials, and PluggableAuthCredentials) by adding @nullable annotations to fields, getters, and builder setters, as well as adding missing @OverRide annotations. Feedback on the changes suggests restoring the @nullable annotation on getExecutableHandler() in PluggableAuthCredentials.java because the handler can indeed be null.


@VisibleForTesting
@Nullable ExecutableHandler getExecutableHandler() {
ExecutableHandler getExecutableHandler() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since handler is optional (for instance, when configured via an executable command in the JSON configuration rather than a custom programmatic handler), this.handler can be null. Therefore, getExecutableHandler() can return null and should retain its @Nullable annotation to correctly reflect its nullability contract. Additionally, please verify if any callers of getExecutableHandler() pass the returned value directly to APIs that do not accept nulls (such as Guava's ImmutableMap.Builder), and ensure proper null checks are in place.

Suggested change
ExecutableHandler getExecutableHandler() {
@Nullable ExecutableHandler getExecutableHandler() {
References
  1. When annotating a method with @Nullable, verify if any callers pass the returned value directly to APIs that do not accept nulls (such as Guava's ImmutableMap.Builder). If null checks are missing, they should be added or tracked for follow-up work.

@lqiu96
lqiu96 force-pushed the fix_auth_adc_credentials_nullability branch from 8b30685 to 0c59b08 Compare August 21, 2026 22:21
…nt credentials

Refine JSpecify nullability annotations across ExternalAccountCredentials and subclasses:
- Annotate optional fields, getters, and builder setters/fields with @nullable across ExternalAccountCredentials, IdentityPoolCredentials, AwsCredentials, PluggableAuthCredentials, and ExternalAccountAuthorizedUserCredentials.
- Ensure correct JSpecify type-use annotation placements on fields, return types, and parameters.
- Migrate Preconditions imports to com.google.common.base.Preconditions.
- Ensure builder copy constructors properly preserve all fields.
@lqiu96
lqiu96 force-pushed the fix_auth_external_account_credentials_nullability branch from 91b799f to ffa7391 Compare August 21, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant