Add FilterOutputStreamSlowMultibyteWrite (#2024)

If a subclass of FilterOutputStream implements `void write(int)`, they
should also override `void write(byte[], int, int)`, otherwise the
performance of the stream is likely to be slow.

See https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/FilterOutputStream.html#write(byte%5B%5D,int,int)
> Subclasses of FilterOutputStream should provide a more efficient implementation of this method.

Similar in concept to https://errorprone.info/bugpattern/InputStreamSlowMultibyteRead

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/FilterOutputStreamSlowMultibyteWrite.java

@@ -0,0 +1,102 @@
+/*
+ * (c) Copyright 2022 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Portions adapted from
+// https://github.com/google/error-prone/blob/e4769fd/core/src/main/java/com/google/errorprone/bugpatterns/InputStreamSlowMultibyteRead.java
+// Copyright 2016 The Error Prone Authors.
+
+package com.palantir.baseline.errorprone;
+
+import com.google.common.collect.ImmutableList;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.BugPattern.SeverityLevel;
+import com.google.errorprone.BugPattern.StandardTags;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.Matchers;
+import com.google.errorprone.suppliers.Supplier;
+import com.google.errorprone.suppliers.Suppliers;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import com.sun.tools.javac.code.Symbol.TypeSymbol;
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.util.Name;
+import java.io.FilterOutputStream;
+import javax.lang.model.element.ElementKind;
+
+@BugPattern(
+        name = "FilterOutputStreamSlowMultibyteWrite",
+        summary = "Please also override `void write(byte[], int, int)`, "
+                + "otherwise multi-byte writes to this output stream are likely to be slow.",
+        severity = SeverityLevel.WARNING,
+        tags = StandardTags.PERFORMANCE)
+public final class FilterOutputStreamSlowMultibyteWrite extends BugChecker implements ClassTreeMatcher {
+
+    private static final Matcher<ClassTree> IS_FILTER_OUTPUT_STREAM = Matchers.isSubtypeOf(FilterOutputStream.class);
+
+    private static final Matcher<MethodTree> WRITE_INT_METHOD = Matchers.allOf(
+            Matchers.methodIsNamed("write"),
+            Matchers.methodReturns(Suppliers.VOID_TYPE),
+            Matchers.methodHasParameters(Matchers.isSameType(Suppliers.INT_TYPE)));
+
+    private static final Supplier<Name> WRITE = VisitorState.memoize(state -> state.getName("write"));
+
+    @Override
+    public Description matchClass(ClassTree classTree, VisitorState state) {
+        if (!IS_FILTER_OUTPUT_STREAM.matches(classTree, state)) {
+            return Description.NO_MATCH;
+        }
+
+        TypeSymbol thisClassSymbol = ASTHelpers.getSymbol(classTree);
+        if (thisClassSymbol.getKind() != ElementKind.CLASS) {
+            return Description.NO_MATCH;
+        }
+
+        Type intType = state.getSymtab().intType;
+        MethodSymbol singleByteWriteMethod = ASTHelpers.resolveExistingMethod(
+                state, thisClassSymbol, WRITE.get(state), ImmutableList.of(intType), ImmutableList.of());
+        if (singleByteWriteMethod == null) {
+            return Description.NO_MATCH;
+        }
+
+        Type byteArrayType = state.arrayTypeForType(state.getSymtab().byteType);
+        MethodSymbol multiByteWriteMethod = ASTHelpers.resolveExistingMethod(
+                state,
+                thisClassSymbol,
+                WRITE.get(state),
+                ImmutableList.of(byteArrayType, intType, intType),
+                ImmutableList.of());
+
+        if (multiByteWriteMethod != null && multiByteWriteMethod.owner.equals(thisClassSymbol)) {
+            return Description.NO_MATCH;
+        }
+
+        // Find method that overrides the single-byte write. It should also override the multibyte write.
+        MethodTree writeByteMethod = classTree.getMembers().stream()
+                .filter(MethodTree.class::isInstance)
+                .map(MethodTree.class::cast)
+                .filter(m -> WRITE_INT_METHOD.matches(m, state))
+                .findFirst()
+                .orElse(null);
+
+        return writeByteMethod == null ? describeMatch(classTree) : describeMatch(writeByteMethod);
+    }
+}

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/FilterOutputStreamSlowMultibyteWriteTest.java

@@ -0,0 +1,130 @@
+/*
+ * (c) Copyright 2022 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Portions adapted from
+// https://github.com/google/error-prone/blob/e4769fd/core/src/test/java/com/google/errorprone/bugpatterns/InputStreamSlowMultibyteReadTest.java
+// Copyright 2014 The Error Prone Authors.
+
+package com.palantir.baseline.errorprone;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+
+class FilterOutputStreamSlowMultibyteWriteTest {
+
+    private final CompilationTestHelper compilationHelper =
+            CompilationTestHelper.newInstance(FilterOutputStreamSlowMultibyteWrite.class, getClass());
+
+    @Test
+    public void doingItRight() {
+        compilationHelper
+                .addSourceLines(
+                        "TestClass.java",
+                        "class TestClass extends java.io.FilterOutputStream {",
+                        "  TestClass() { super(null); }",
+                        "  public void write(byte[] b, int a, int c) {}",
+                        "  public void write(int b) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void empty() {
+        compilationHelper
+                .addSourceLines(
+                        "TestClass.java",
+                        "  // BUG: Diagnostic contains:",
+                        "class TestClass extends java.io.FilterOutputStream {",
+                        "  TestClass() { super(null); }",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void basic() {
+        compilationHelper
+                .addSourceLines(
+                        "TestClass.java",
+                        "class TestClass extends java.io.FilterOutputStream {",
+                        "  TestClass() { super(null); }",
+                        "  // BUG: Diagnostic contains:",
+                        "  public void write(int b) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void abstractOverride() {
+        compilationHelper
+                .addSourceLines(
+                        "TestClass.java",
+                        "abstract class TestClass extends java.io.FilterOutputStream {",
+                        "  TestClass() { super(null); }",
+                        "  // BUG: Diagnostic contains:",
+                        "  public abstract void write(int b);",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void nativeOverride() {
+        compilationHelper
+                .addSourceLines(
+                        "TestClass.java",
+                        "abstract class TestClass extends java.io.FilterOutputStream {",
+                        "  TestClass() { super(null); }",
+                        "  // BUG: Diagnostic contains:",
+                        "  public native void write(int b);",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void inheritedMultiByteWrite() {
+        compilationHelper
+                .addSourceLines(
+                        "Super.java",
+                        "abstract class Super extends java.io.FilterOutputStream {",
+                        "  Super() { super(null); }",
+                        "  public void write(byte[] b, int a, int c) {}",
+                        "}")
+                .addSourceLines(
+                        "TestClass.java",
+                        "class TestClass extends Super {",
+                        "  // BUG: Diagnostic contains:",
+                        "  public void write(int b) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void inheritedSingleByteWrite() {
+        compilationHelper
+                .addSourceLines(
+                        "Super.java",
+                        "abstract class Super extends java.io.FilterOutputStream {",
+                        "  Super() { super(null); }",
+                        "  // BUG: Diagnostic contains:",
+                        "  public void write(int b) {}",
+                        "}")
+                .addSourceLines(
+                        "TestClass.java",
+                        "class TestClass extends Super {",
+                        "  public void write(byte[] b, int a, int c) {}",
+                        "}")
+                .doTest();
+    }
+}

changelog/@unreleased/pr-2024.v2.yml

@@ -0,0 +1,13 @@
+type: improvement
+improvement:
+  description: |-
+    If a subclass of FilterOutputStream implements `void write(int)`, they
+    should also override `void write(byte[], int, int)`, otherwise the
+    performance of the stream is likely to be slow.
+
+    See https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/FilterOutputStream.html#write(byte%5B%5D,int,int)
+    > Subclasses of FilterOutputStream should provide a more efficient implementation of this method.
+
+    Similar in concept to https://errorprone.info/bugpattern/InputStreamSlowMultibyteRead
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2024