New check request: OutputStream subclasses that inherit the per-byte write(byte[], int, int)
- Dominant language
- Java
- Stars
- 7.2k
- Forks
- 820
- Avg merge
- 5h 9m
- Merged PRs (30d)
- 50
Description
I wrapped an `OutputStream` in a `FilterOutputStream` subclass that overrides only `flush()`. Every `write(byte[], off, len)` through it reached the wrapped stream as one `write(int)` call per byte, and Error Prone reported nothing. A direct `OutputStream` subclass that implements `write(int)` only is not reported either. The `InputStream` counterpart, a class that implements `read()` without `read(byte[], int, int)`, is reported by `InputStreamSlowMultibyteRead`.
The JDK documents both inherited implementations as ones to replace. [`OutputStream.write(byte[], int, int)`](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/io/OutputStream.html#write(byte%5B%5D,int,int)) encourages subclasses to override it, and [`FilterOutputStream.write(byte[], int, int)`](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/io/FilterOutputStream.html#write(byte%5B%5D,int,int)) says "Subclasses of FilterOutputStream should provide a more efficient implementation".
### Example
```java
package demo;
import java.io.FilterOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
public class Demo {
/** A decorator that only suppresses flush(). Not reported. */
static final class NoFlush extends FilterOutputStream {
NoFlush(OutputStream out) {
super(out);
}
@Override
public void flush() {}
}
/** An OutputStream that implements write(int) only. Not reported. */
static final class ByteSink extends OutputStream {
private final OutputStream out;
ByteSink(OutputStream out) {
this.out = out;
}
@Override
public void write(int b) throws IOException {
out.write(b);
}
}
/** The InputStream counterpart. Reported by InputStreamSlowMultibyteRead. */
static final class ByteSource extends InputStream {
private final InputStream in;
ByteSource(InputStream in) {
this.in = in;
}
@Override
public int read() throws IOException {
return in.read();
}
}
}
```
### Actual output, Error Prone 2.50.0
Compiled with Error Prone 2.50.0 (Gradle 9.6.1, `net.ltgt.errorprone` 5.1.1, JDK 21.0.9). The path is shortened, and this is the whole compiler output. `NoFlush` (line 10) and `ByteSink` (line 28) are not reported; the warning on `ByteSource` is the control that shows the check runs, not the problem.
```text
src/main/java/demo/Demo.java:42: warning: [InputStreamSlowMultibyteRead] Please also override int read(byte[], int, int), otherwise multi-byte reads from this input stream are likely to be slow.
public int read() throws IOException {
^
(see https://errorprone.info/bugpattern/InputStreamSlowMultibyteRead)
1 warning
```
### Expected output
The check name and the message below are placeholders; what I am asking for is a warning on line 10 and on line 28, next to the existing one on line 42.
```text
src/main/java/demo/Demo.java:10: warning: [OutputStreamSlowMultibyteWrite]
static final class NoFlush extends FilterOutputStream {
^
src/main/java/demo/Demo.java:28: warning: [OutputStreamSlowMultibyteWrite]
public void write(int b) throws IOException {
^
src/main/java/demo/Demo.java:42: warning: [InputStreamSlowMultibyteRead] Please also override int read(byte[], int, int), otherwise multi-byte reads from this input stream are likely to be slow.
public int read() throws IOException {
^
(see https://errorprone.info/bugpattern/InputStreamSlowMultibyteRead)
3 warnings
```
A `FilterOutputStream` or `OutputStream` subclass that declares `write(byte[], int, int)` stays unreported.
### Where it came up
pgjdbc had the `NoFlush` shape: an anonymous `FilterOutputStream` in `PGStream.getEncodingWriter()` that overrides only `flush()`. An ArchUnit rule found it, and pgjdbc/pgjdbc#4391 fixes it. That is the only case I know of; I have not measured how often the pattern occurs in other code bases.
### One possible shape
The requirement is the two new warnings in the expected output; the rest is yours to choose.
- A mirror of `InputStreamSlowMultibyteRead` covers `ByteSink`: a subtype of `OutputStream` that declares `write(int)` while `write(byte[], int, int)` resolves to `OutputStream` or `FilterOutputStream`.
- It does not cover `NoFlush`, which declares no `write` method at all. The input side has no such case, because `FilterInputStream.read(byte[], int, int)` passes the range to the wrapped stream.
- An automatic fix that forwards the range to `out` would be wrong for a subclass whose `write(int)` transforms each byte, so a warning without a fix seems safer.
### Searched before filing
Issues and pull requests in google/error-prone, open and closed, for `FilterOutputStream`, `OutputStream multibyte`, `write(byte[], int, int)`, `slow write OutputStream`, `OutputStream single byte`, and `InputStreamSlowMultibyteRead`: nothing related. On `master` at 80c91b3fe1, the only checks under `core/src/main/java` that mention `FilterOutputStream` or `OutputStream.class` are `CloseableDecoratorTypes` and `DefaultCharset`.
### Alternative in use
pgjdbc checks this with an ArchUnit rule over compiled classes at test time. It works for one project, but every project has to write and maintain its own rule, and the rule runs in tests rather than at compile time.
If the check sounds useful, I am happy to draft a PR.
Contributor guide
Research direction
Start by reading the existing InputStreamSlowMultibyteRead check and the checks under core/src/main/java, including the references to OutputStream.class. Define the new warning around OutputStream and FilterOutputStream subclasses that inherit the range-writing implementation, while leaving subclasses that declare it unreported; validate the two requested warnings against the Java example.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- devtools, tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 58/100