Skip to content

feat(auth): add JSpecify Null annotations to Auth#13842

Draft
nnicolee wants to merge 5 commits into
mainfrom
feat/jspecify-auth
Draft

feat(auth): add JSpecify Null annotations to Auth#13842
nnicolee wants to merge 5 commits into
mainfrom
feat/jspecify-auth

Conversation

@nnicolee

Copy link
Copy Markdown
Contributor

tba

@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 introduces JSpecify nullness annotations (@NullMarked and @nullable) across several modules, including GAX and Google Cloud Bigtable, and updates tests to configure Mockito mocks to ignore annotations. The reviewer feedback highlights several improvement opportunities, such as correcting a potentially unsafe @nullable annotation on a builder constructor parameter to avoid NullPointerExceptions, and ensuring that the previousThrowable parameters in retry algorithms are explicitly marked as @nullable to handle successful attempts where no exception is thrown.


/** Create a builder from a StubSettings object. */
protected Builder(StubSettings.Builder stubSettings) {
protected Builder(StubSettings.@Nullable Builder stubSettings) {

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

Annotating stubSettings as @Nullable suggests that null is a valid argument. However, if stubSettings is null, calling methods like getQuotaProjectId() or getWatchdogProvider() on this builder will result in a NullPointerException because they dereference stubSettings without a null check. If stubSettings must not be null, it should not be annotated with @Nullable, and ideally, a null check like Preconditions.checkNotNull(stubSettings) should be added.

* thrown instead
* @param previousSettings previous attempt settings
* @return next attempt settings, can be {@code null}, if there should be no new attempt
*/

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

In a @NullMarked class, previousThrowable should be annotated with @Nullable because on successful attempts (e.g., during polling or when checking if retries should continue after a success), previousThrowable can be null.

Suggested change
*/
@Nullable Throwable previousThrowable,

public boolean shouldRetry(
RetryingContext context,
@Nullable RetryingContext context,
Throwable previousThrowable,

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

In a @NullMarked class, previousThrowable should be annotated with @Nullable because on successful attempts (e.g., during polling or when checking if retries should continue after a success), previousThrowable can be null.

Suggested change
Throwable previousThrowable,
@Nullable Throwable previousThrowable,

public TimedAttemptSettings createNextAttempt(
RetryingContext context,
@Nullable RetryingContext context,
Throwable previousThrowable,

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

In a @NullMarked class, previousThrowable should be annotated with @Nullable because on successful attempts (e.g., during polling or when checking if retries should continue after a success), previousThrowable can be null.

Suggested change
Throwable previousThrowable,
@Nullable Throwable previousThrowable,

public boolean shouldRetry(
RetryingContext context,
@Nullable RetryingContext context,
Throwable previousThrowable,

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

In a @NullMarked class, previousThrowable should be annotated with @Nullable because on successful attempts (e.g., during polling or when checking if retries should continue after a success), previousThrowable can be null.

Suggested change
Throwable previousThrowable,
@Nullable Throwable previousThrowable,

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
B Maintainability Rating on New Code (required ≥ A)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

💡 Need a hand with PR review? Try Gitar by Sonar!

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
B Maintainability Rating on New Code (required ≥ A)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

💡 Need a hand with PR review? Try Gitar by Sonar!

@nnicolee
nnicolee force-pushed the feat/jspecify-auth branch from 76c939a to afdc8c6 Compare July 21, 2026 04:39
@nnicolee
nnicolee force-pushed the feat/jspecify-auth branch from afdc8c6 to 432c5d0 Compare July 21, 2026 04:43
@nnicolee
nnicolee requested a review from lqiu96 July 21, 2026 15:12
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