Skip to content

Conversation

@heruan
Copy link
Member

@heruan heruan commented Jan 5, 2026

This is a fork of #22659 that limits the scope of the ErrorProne plugin to the signals module, including all the JSpecify annotations needed for a clean build. Notes:

  • NullAway assumes @NotNull for non-annotated fields/params
  • some null-checks are required even if certain statements are implicitly null-safe, e.g. getting a value from a Map<X, @Nullable Y>, even if the logic never puts null in that context (but it might do in a different context, hence the @Nullable Y)
  • tests are excluded from the enforcement

@heruan heruan changed the base branch from main to error-prone January 5, 2026 16:49
@github-actions
Copy link

github-actions bot commented Jan 5, 2026

Test Results

1 277 files  1 277 suites   1h 12m 37s ⏱️
8 770 tests 8 666 ✅ 65 💤  2 ❌ 37 🔥
9 182 runs  9 014 ✅ 74 💤 16 ❌ 78 🔥

For more details on these failures and errors, see this check.

Results for commit 2a36b2c.

@Legioth
Copy link
Member

Legioth commented Jan 7, 2026

tests are excluded from the enforcement

Would it make sense to have enforcement also in the tests so that we can see that typical application code with enforcement enabled would behave as expected?

@heruan
Copy link
Member Author

heruan commented Jan 7, 2026

Would it make sense to have enforcement also in the tests so that we can see that typical application code with enforcement enabled would behave as expected?

It would, and I first started with that but in tests we tend to ignore nullability when we know the tests won't fail for that (e.g. with mocks); also, adding a lot of if (foo == null) in tests to make the compiler happy might reduce readability.

On the other hand, I think it would be an encouragement to start making our API less @Nullable so that we don't need null-checks everywhere.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 7, 2026

@heruan
Copy link
Member Author

heruan commented Jan 7, 2026

In 9d49f64 I've enforced nullability checks in tests, mainly by asserting nullable values not being null before using them with assertNotNull. This keeps good readability avoiding adding conditional blocks, but still breaks method chains when they're not null-safe. For example, if we declare an AtomicReference<Foo> we cannot reset it with ref.set(null) and if we declare as AtomicReference<@Nullable Foo> we cannot call ref.get().bar().

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.

4 participants