Skip to content

Annotate toProvider(..) methods to specify that providers can return null.#1872

Merged
copybara-service[bot] merged 1 commit intomasterfrom
test_734534964
Mar 8, 2025
Merged

Annotate toProvider(..) methods to specify that providers can return null.#1872
copybara-service[bot] merged 1 commit intomasterfrom
test_734534964

Conversation

@copybara-service
Copy link

Annotate toProvider(..) methods to specify that providers can return null.

This doesn't change any Guice behavior; it just lets nullness checkers (like Kotlin and the Checker Framework) allow you to bind to such providers more easily.

Guice's handling of null is not entirely principled, and this CL doesn't attempt to "solve" that, nor does it attempt to thoroughly annotate for nullness, let alone enable a nullness checker for the Guice codebase. Rather, it aims only to remove a restriction that I don't think is serving any particular purpose and that no on "intended"; I think it just kind of happened because the Checker Framework treats unannotated code leniently and strictly at different times, and this happens to be one of the strict cases. (We might also have once had the hope of cracking down on injection of null?)

The immediate motivation for this CL is google/inject-common#26: Soon, nullness checkers will know that Providers.of(null) returns gasp a Provider that can return null. That makes bind(Foo.class).to(Providers.of(null)) an error. This CL lets those callers continue to compile.

@copybara-service copybara-service bot force-pushed the test_734534964 branch 4 times, most recently from 2b6a574 to 19eab7b Compare March 8, 2025 02:31
…n `null`.

This doesn't change any Guice behavior; it just lets nullness checkers (like Kotlin and the Checker Framework) allow you to bind to such providers more easily.

Guice's handling of `null` is not entirely principled, and this CL doesn't attempt to "solve" that, nor does it attempt to thoroughly annotate for nullness, let alone enable a nullness checker for the Guice codebase. Rather, it aims only to remove a restriction that I don't think is serving any particular purpose and that no on "intended"; I think it just kind of happened because the Checker Framework treats unannotated code leniently and strictly at different times, and this happens to be one of the strict cases. (We might also have once had the hope of cracking down on injection of `null`?)

The immediate motivation for this CL is google/inject-common#26: Soon, nullness checkers will know that `Providers.of(null)` returns *gasp* a `Provider` that can return `null`. That makes `bind(Foo.class).to(Providers.of(null))` an error. This CL lets those callers continue to compile.

PiperOrigin-RevId: 734755975
@copybara-service copybara-service bot merged commit cdf076c into master Mar 8, 2025
@copybara-service copybara-service bot deleted the test_734534964 branch March 8, 2025 03:00
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