New check request: OutputStream subclasses that inherit the per-byte write(byte[], int, int)
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) encourages subclasses to override it, and FilterOutputStream.write(byte[], int, int) says "Subclasses of FilterOutputStream should provide a more efficient implementation".
Example
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.
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 warningExpected 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.
src/main/java/demo/Demo.java:10: warning: [OutputStreamSlowMultibyteWrite] <message>
static final class NoFlush extends FilterOutputStream {
^
src/main/java/demo/Demo.java:28: warning: [OutputStreamSlowMultibyteWrite] <message>
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 warningsA 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
InputStreamSlowMultibyteReadcoversByteSink: a subtype ofOutputStreamthat declareswrite(int)whilewrite(byte[], int, int)resolves toOutputStreamorFilterOutputStream. - It does not cover
NoFlush, which declares nowritemethod at all. The input side has no such case, becauseFilterInputStream.read(byte[], int, int)passes the range to the wrapped stream. - An automatic fix that forwards the range to
outwould be wrong for a subclass whosewrite(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.
Source: google/error-prone