Skip to content

Annotate PushbackInputStream.buf as @Nullable.#141

Merged
cpovirk merged 1 commit intomainfrom
pushback
Mar 23, 2026
Merged

Annotate PushbackInputStream.buf as @Nullable.#141
cpovirk merged 1 commit intomainfrom
pushback

Conversation

@cpovirk
Copy link
Copy Markdown
Collaborator

@cpovirk cpovirk commented Mar 10, 2026

This follows google/xplat@f7a56b3

That commit does additionally add @Nullable to the InputStream in
constructor parameters. It looks like a null in leads to a stream that
doesn't work unless you later modify FilterInputStream.in directly, as
the xplat copy
documents
.
On the one hand, this is no different from its superclass
FilterInputStream, where we included
@Nullable
.
On the other hand, FilterInputStream specifically documents that
passing null is an intended use case, and we've seen people pass
null in practice with the intent of modifying the in field later. We
haven't seen that with PushbackInputStream, even in the rare instances
in which the class is subclassed.

This follows google/xplat@f7a56b3

That commit does additionally add `@Nullable` to the `InputStream in`
constructor parameters. It looks like a null `in` leads to a stream that
doesn't work unless you later modify `FilterInputStream.in` directly, as
[the xplat copy
documents](google/xplat@f7a56b3#diff-f21cbb1ece57585a6b018af66249cc68cf16f32fbfc25fdecd2c47dc567abcc3R43-R44).
On the one hand, this is no different from its superclass
`FilterInputStream`, where we [included
`@Nullable`](https://github.com/jspecify/jdk/blob/cf198c06e3bd59572304f866b5cbf70781112f55/src/java.base/share/classes/java/io/FilterInputStream.java#L57-L60).
On the other hand, `FilterInputStream` specifically documents that
passing `null` is an intended use case, and we've seen people pass
`null` in practice with the intent of modifying the `in` field later. We
haven't seen that with `PushbackInputStream`, even in the rare instances
in which the class is subclassed.
@cpovirk cpovirk requested a review from wmdietl March 10, 2026 19:36
Copy link
Copy Markdown
Collaborator

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

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

In the xplat version, I see lots of null checks for buf.
In this version, most places assume that buf is non-null and directly dereference it.
Only close sets it back to null, so it does make sense to make the field Nullable.

@wmdietl wmdietl assigned cpovirk and unassigned wmdietl Mar 23, 2026
@cpovirk cpovirk merged commit fd65be6 into main Mar 23, 2026
21 checks passed
@cpovirk cpovirk deleted the pushback branch March 23, 2026 20:18
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.

3 participants