Recognize com.google.errorprone.annotations.Immutable

This annotation is @inherited and should be recognized as marking
a type hierarchy as immutable.

The implementation adds a bit of infrastructure to efficiently look
through the type hierarchy, which is immediately useful for speeding
up looksLikeASetter().

Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

CHANGELOG.md

@@ -13,6 +13,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 - Agree verb with plural subject in the description of
 `SW_SWING_METHODS_INVOKED_IN_SWING_THREAD` ([#1664](https://github.com/spotbugs/spotbugs/pull/1664))
 - Wrong description of the `SE_TRANSIENT_FIELD_OF_NONSERIALIZABLE_CLASS` ([#1664](https://github.com/spotbugs/spotbugs/pull/1664))
+- Treat types with `@com.google.errorprone.annotations.Immutable` as immutable ([#1705](https://github.com/spotbugs/spotbugs/pull/1705))
 
 ## 4.4.1 - 2021-09-07
 ### Changed

spotbugs/build.gradle

@@ -97,6 +97,7 @@ dependencies {
   testImplementation 'junit:junit:4.13.1'
   testImplementation 'org.apache.ant:ant:1.10.11'
   testImplementation 'org.apache.logging.log4j:log4j-slf4j18-impl:2.14.0'
+  testImplementation 'com.google.errorprone:error_prone_annotations:2.9.0'
 
   guiImplementation sourceSets.main.runtimeClasspath
   guiCompileOnly 'com.apple:AppleJavaExtensions:1.4'

spotbugs/src/main/java/edu/umd/cs/findbugs/util/MutableClasses.java

@@ -2,11 +2,12 @@
 
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.stream.Stream;
-import java.util.List;
 
 import org.apache.bcel.Repository;
+import org.apache.bcel.classfile.AnnotationEntry;
 import org.apache.bcel.classfile.JavaClass;
 import org.apache.bcel.classfile.Method;
 
@@ -42,10 +43,6 @@ public class MutableClasses {
     private static final Set<String> KNOWN_IMMUTABLE_PACKAGES = new HashSet<>(Arrays.asList(
             "java.math", "java.time"));
 
-    private static final List<String> SETTER_LIKE_NAMES = Arrays.asList(
-            "set", "put", "add", "insert", "delete", "remove", "erase", "clear", "push", "pop",
-            "enqueue", "dequeue", "write", "append", "replace");
-
     public static boolean mutableSignature(String sig) {
         if (sig.charAt(0) == '[') {
             return true;
@@ -72,47 +69,139 @@ public static boolean mutableSignature(String sig) {
 
         try {
             JavaClass cls = Repository.lookupClass(dottedClassName);
-            if (Stream.of(cls.getAnnotationEntries()).anyMatch(s -> (s.toString().endsWith("/Immutable;"))
+            if (Stream.of(cls.getAnnotationEntries()).anyMatch(s -> s.toString().endsWith("/Immutable;")
                     || s.getAnnotationType().equals("jdk.internal.ValueBased"))) {
                 return false;
             }
-            return isMutable(cls);
+            return ClassAnalysis.load(cls, sig).isMutable();
         } catch (ClassNotFoundException e) {
             AnalysisContext.reportMissingClass(e);
             return false;
         }
     }
 
-    private static boolean isMutable(JavaClass cls) {
-        for (Method method : cls.getMethods()) {
-            if (looksLikeASetter(method, cls)) {
-                return true;
+    /**
+     * Analytic information about a {@link JavaClass} relevant to determining its mutability properties.
+     */
+    private static final class ClassAnalysis {
+        private static final List<String> SETTER_LIKE_PREFIXES = Arrays.asList(
+                "set", "put", "add", "insert", "delete", "remove", "erase", "clear", "push", "pop",
+                "enqueue", "dequeue", "write", "append", "replace");
+
+        /**
+         * Class under analysis.
+         */
+        private final JavaClass cls;
+        /**
+         * Superclass {@link ClassAnalysis}, lazily instantiated if present, otherwise {@code null}.
+         */
+        private ClassAnalysis superAnalysis;
+
+        // Various lazily-determined properties of this class. null indicates the property has not been determined yet.
+        private String sig;
+        private Boolean mutable;
+        private Boolean immutableByContract;
+
+        private ClassAnalysis(JavaClass cls, String sig) {
+            this.cls = cls;
+            this.sig = sig;
+        }
+
+        static ClassAnalysis load(JavaClass cls, String sig) {
+            // TODO: is there a place where we can maintain a cache of these for the duration of analysis?
+            return new ClassAnalysis(cls, sig);
+        }
+
+        boolean isMutable() {
+            Boolean local = mutable;
+            if (local == null) {
+                mutable = local = computeMutable();
             }
+            return local;
         }
-        try {
-            JavaClass sup = cls.getSuperClass();
-            if (sup != null) {
-                return isMutable(sup);
+
+        private boolean computeMutable() {
+            if (isImmutableByContract()) {
+                return false;
             }
-        } catch (ClassNotFoundException e) {
-            AnalysisContext.reportMissingClass(e);
+
+            for (Method method : cls.getMethods()) {
+                if (!method.isStatic() && looksLikeASetter(method)) {
+                    return true;
+                }
+            }
+
+            final ClassAnalysis maybeSuper = getSuperAnalysis();
+            return maybeSuper != null && maybeSuper.isMutable();
         }
-        return false;
-    }
 
-    public static boolean looksLikeASetter(Method method, JavaClass cls) {
-        if (method.isStatic()) {
+        private boolean looksLikeASetter(Method method) {
+            final String methodName = method.getName();
+            for (String name : SETTER_LIKE_PREFIXES) {
+                if (methodName.startsWith(name)) {
+                    // If setter-like methods returns an object of the same type then we suppose that it
+                    // is not a setter but creates a new instance instead.
+                    return !getSig().equals(method.getReturnType().getSignature());
+                }
+            }
             return false;
         }
 
-        for (String name : SETTER_LIKE_NAMES) {
-            if (method.getName().startsWith(name)) {
-                String retSig = method.getReturnType().getSignature();
-                // If setter-like methods returns an object of the same type then we suppose that it
-                // is not a setter but creates a new instance instead.
-                return !("L" + cls.getClassName().replace('.', '/') + ";").equals(retSig);
+        private String getSig() {
+            String local = sig;
+            if (local == null) {
+                sig = local = "L" + cls.getClassName().replace('.', '/') + ";";
             }
+            return local;
+        }
+
+        private boolean isImmutableByContract() {
+            Boolean local = immutableByContract;
+            if (local == null) {
+                immutableByContract = local = computeByImmutableContract();
+            }
+            return local;
+        }
+
+        private boolean computeByImmutableContract() {
+            for (AnnotationEntry entry : cls.getAnnotationEntries()) {
+                // Error-Prone's @Immutable annotation is @Inherited, hence it applies to subclasses as well
+                if (entry.getAnnotationType().equals("Lcom/google/errorprone/annotations/Immutable;")) {
+                    return true;
+                }
+            }
+
+            final ClassAnalysis maybeSuper = getSuperAnalysis();
+            return maybeSuper != null && maybeSuper.isImmutableByContract();
+        }
+
+        private ClassAnalysis getSuperAnalysis() {
+            ClassAnalysis local = superAnalysis;
+            if (local == null) {
+                superAnalysis = local = loadSuperAnalysis();
+            }
+            return local;
+        }
+
+        private ClassAnalysis loadSuperAnalysis() {
+            // Quick check if there is a superclass of importance
+            final String superName = cls.getSuperclassName();
+            if (superName == null || superName.equals("java.lang.Object")) {
+                return null;
+            }
+
+            final JavaClass superClass;
+            try {
+                superClass = cls.getSuperClass();
+            } catch (ClassNotFoundException e) {
+                AnalysisContext.reportMissingClass(e);
+                return null;
+            }
+            if (superClass == null) {
+                return null;
+            }
+
+            return load(superClass, null);
         }
-        return false;
     }
 }

spotbugs/src/test/java/edu/umd/cs/findbugs/util/MutableClassesTest.java

@@ -95,4 +95,25 @@ public static void setImmutable(Immutable imm) {
     public void TestImmutable() {
         Assert.assertFalse(MutableClasses.mutableSignature("Ledu/umd/cs/findbugs/util/MutableClassesTest$Immutable;"));
     }
+
+    @com.google.errorprone.annotations.Immutable
+    public static class ErrorProneImmutable {
+        public void write() {
+            // Does not matter
+        }
+    }
+
+    public static class ErrorProneImmutableSubclass extends ErrorProneImmutable {
+        public void writeOther() {
+            // Does not matter
+        }
+    }
+
+    @Test
+    public void TestErrorProneImmutable() {
+        Assert.assertFalse(MutableClasses.mutableSignature(
+                "Ledu/umd/cs/findbugs/util/MutableClassesTest$ErrorProneImmutable;"));
+        Assert.assertFalse(MutableClasses.mutableSignature(
+                "Ledu/umd/cs/findbugs/util/MutableClassesTest$ErrorProneImmutableSubclass;"));
+    }
 }