test: jspecify auth spanner#13862
Conversation
… ServiceAccountCredentials
There was a problem hiding this comment.
Code Review
This pull request introduces JSpecify nullability annotations (@NullMarked and @Nullable) across the codebase to improve null-safety, updating build configurations and adjusting tests accordingly. The review feedback identifies two critical null-safety issues: first, changing getRequestMetadata in Credentials to accept a @Nullable URI causes compilation errors in implementing subclasses unless their overrides are also updated; second, passing a @Nullable URI to getUriForSelfSignedJWT in ServiceAccountCredentials violates null-safety since that method expects a non-null argument.
| public abstract Map<String, List<String>> getRequestMetadata(@Nullable URI uri) | ||
| throws IOException; |
There was a problem hiding this comment.
Changing the abstract method getRequestMetadata to accept @Nullable URI requires all implementing subclasses (such as OAuth2Credentials, ApiKeyCredentials, etc.) to also be updated to accept @Nullable URI in their overrides. Since those subclasses are also marked with @NullMarked in this PR, their unannotated parameters will default to non-null, which will result in static analysis compilation errors (e.g., incompatible override in NullAway or Checker Framework) due to a nullability mismatch.
| JwtCredentials createSelfSignedJwtCredentials(final @Nullable URI uri) { | ||
| return createSelfSignedJwtCredentials(uri, scopes.isEmpty() ? defaultScopes : scopes); |
There was a problem hiding this comment.
The parameter uri is marked as @Nullable, but it is passed directly to getUriForSelfSignedJWT(uri) (which is not annotated to accept @Nullable and expects a non-null URI under @NullMarked). If uri is indeed null, this will violate null-safety and potentially lead to a NullPointerException when audience.toString() is called on the result. Please ensure that getUriForSelfSignedJWT is updated to handle @Nullable URI safely, or add a null check before calling it.
testing