Fix for false positives EI_EXPOSE_REP in case of unmodifiable collections

CHANGELOG.md

@@ -11,6 +11,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 - Bump up logback to `1.4.0`
 - Bump up log4j2 binding to `2.18.0`
 - Fixed InvalidInputException in Eclipse while bug reporting ([#2134](https://github.com/spotbugs/spotbugs/issues/2134))
+- Fixed false positives `EI_EXPOSE_REP` thrown in case of fields initialized by the `of` or `copyOf` method of a `List`, `Map` or `Set` ([#1771](https://github.com/spotbugs/spotbugs/issues/1771))
 - Fixed CFGBuilderException thrown when `dup_x2` is used to swap the reference and wide-value (double, long) in the stack ([#2146](https://github.com/spotbugs/spotbugs/pull/2146))
 
 ## 4.7.1 - 2022-06-26

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue1771Test.java

@@ -0,0 +1,32 @@
+package edu.umd.cs.findbugs.detect;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeThat;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.number.OrderingComparison.greaterThanOrEqualTo;
+
+public class Issue1771Test extends AbstractIntegrationTest {
+    @Before
+    public void verifyJavaVersion() {
+        assumeFalse(System.getProperty("java.specification.version").startsWith("1."));
+        int javaVersion = Integer.parseInt(System.getProperty("java.specification.version"));
+        assumeThat(javaVersion, is(greaterThanOrEqualTo(11)));
+    }
+
+    @Test
+    public void test() {
+        performAnalysis("../java11/ghIssues/Issue1771.class");
+        BugInstanceMatcher matcher = new BugInstanceMatcherBuilder()
+                .bugType("EI_EXPOSE_REP").build();
+        assertThat(getBugCollection(), containsExactly(0, matcher));
+    }
+}

spotbugs/src/main/java/edu/umd/cs/findbugs/OpcodeStack.java

@@ -60,6 +60,7 @@
 import org.apache.bcel.classfile.Method;
 import org.apache.bcel.generic.BasicType;
 import org.apache.bcel.generic.Type;
+import org.apache.commons.lang3.tuple.Pair;
 
 import edu.umd.cs.findbugs.OpcodeStack.Item.SpecialKind;
 import edu.umd.cs.findbugs.StackMapAnalyzer.JumpInfoFromStackMap;
@@ -129,6 +130,38 @@ public class OpcodeStack {
 
     private static final boolean DEBUG2 = DEBUG;
 
+    private static final Map<Pair<String, String>, String> IMMUTABLE_RETURNER_MAP = new HashMap<>();
+    static {
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "emptyList"), "Ljava/util/Collections$EmptyList;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "emptyMap"), "Ljava/util/Collections$EmptyMap;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "emptyNavigableMap"), "Ljava/util/Collections$EmptyNavigableMap;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "emptySortedMap"), "Ljava/util/Collections$EmptyNavigableMap;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "emptySet"), "Ljava/util/Collections$EmptySet;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "emptyNavigableSet"), "Ljava/util/Collections$EmptyNavigableSet;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "emptySortedSet"), "Ljava/util/Collections$EmptyNavigableSet;");
+
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "singletonList"), "Ljava/util/Collections$SingletonList;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "singletonMap"), "Ljava/util/Collections$SingletonMap;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "singleton"), "Ljava/util/Collections$SingletonSet;");
+
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "unmodifiableList"), "Ljava/util/Collections$UnmodifiableList;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "unmodifiableMap"), "Ljava/util/Collections$UnmodifiableMap;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "unmodifiableNavigableMap"), "Ljava/util/Collections$UnmodifiableNavigableMap;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "unmodifiableSortedMap"), "Ljava/util/Collections$UnmodifiableSortedMap;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "unmodifiableSet"), "Ljava/util/Collections$UnmodifiableSet;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "unmodifiableNavigableSet"), "Ljava/util/Collections$UnmodifiableNavigableSet;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Collections", "unmodifiableSortedSet"), "Ljava/util/Collections$UnmodifiableSortedSet;");
+
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/List", "of"), "Ljava/util/ImmutableCollections$AbstractImmutableList;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/List", "copyOf"), "Ljava/util/ImmutableCollections$AbstractImmutableList;");
+
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Map", "of"), "Ljava/util/ImmutableCollections$AbstractImmutableMap;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Map", "copyOf"), "Ljava/util/ImmutableCollections$AbstractImmutableMap;");
+
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Set", "of"), "Ljava/util/ImmutableCollections$AbstractImmutableSet;");
+        IMMUTABLE_RETURNER_MAP.put(Pair.of("java/util/Set", "copyOf"), "Ljava/util/ImmutableCollections$AbstractImmutableSet;");
+    }
+
     @StaticConstant
     static final HashMap<String, String> boxedTypes = new HashMap<>();
 
@@ -2624,15 +2657,35 @@ private void processMethodCall(DismantleBytecode dbc, int seen) {
             Item result = new Item(JAVA_UTIL_ARRAYS_ARRAY_LIST);
             push(result);
             return;
-        } else if (seen == Const.INVOKESTATIC && "(Ljava/util/List;)Ljava/util/List;".equals(signature)
-                && "java/util/Collections".equals(clsName)) {
-            Item requestParameter = pop();
-            if (JAVA_UTIL_ARRAYS_ARRAY_LIST.equals(requestParameter.getSignature())) {
-                Item result = new Item(JAVA_UTIL_ARRAYS_ARRAY_LIST);
+        } else if (seen == Const.INVOKESTATIC) {
+            Item requestParameter = null;
+            if ("(Ljava/util/List;)Ljava/util/List;".equals(signature)
+                    && "java/util/Collections".equals(clsName)) {
+                requestParameter = top();
+            }
+            String returnTypeName = IMMUTABLE_RETURNER_MAP.get(Pair.of(clsName, methodName));
+            if (returnTypeName != null) {
+                SignatureParser sp = new SignatureParser(signature);
+                for (int i = 0; i < sp.getNumParameters(); ++i) {
+                    pop();
+                }
+                Item result;
+                if (requestParameter != null && JAVA_UTIL_ARRAYS_ARRAY_LIST.equals(requestParameter.getSignature())) {
+                    result = new Item("Ljava/util/Collections$UnmodifiableRandomAccessList");
+                } else {
+                    result = new Item(returnTypeName);
+                }
                 push(result);
                 return;
+            } else if (requestParameter != null) {
+                pop();
+                if (JAVA_UTIL_ARRAYS_ARRAY_LIST.equals(requestParameter.getSignature())) {
+                    Item result = new Item(JAVA_UTIL_ARRAYS_ARRAY_LIST);
+                    push(result);
+                    return;
+                }
+                push(requestParameter); // fall back to standard logic
             }
-            push(requestParameter); // fall back to standard logic
         }
 
         pushByInvoke(dbc, seen != Const.INVOKESTATIC);
@@ -3172,6 +3225,10 @@ private Item pop() {
         return stack.remove(stack.size() - 1);
     }
 
+    private Item top() {
+        return stack.get(stack.size() - 1);
+    }
+
     public void replace(int stackOffset, Item value) {
         if (stackOffset < 0 || stackOffset >= stack.size()) {
             AnalysisContext.logError("Can't get replace stack offset " + stackOffset + " from " + stack.toString() + " @ " + v.getPC()

spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindReturnRef.java

@@ -36,6 +36,7 @@
 import edu.umd.cs.findbugs.OpcodeStack;
 import edu.umd.cs.findbugs.ba.AnalysisContext;
 import edu.umd.cs.findbugs.ba.XField;
+import edu.umd.cs.findbugs.ba.type.TypeFrameModelingVisitor;
 import edu.umd.cs.findbugs.classfile.CheckedAnalysisException;
 import edu.umd.cs.findbugs.classfile.ClassDescriptor;
 import edu.umd.cs.findbugs.bcel.OpcodeStackDetector;
@@ -196,7 +197,7 @@ public void sawOpcode(int seen) {
                     field.isPublic() ||
                     AnalysisContext.currentXFactory().isEmptyArrayField(field) ||
                     field.getName().indexOf("EMPTY") != -1 ||
-                    !MutableClasses.mutableSignature(field.getSignature())) {
+                    !MutableClasses.mutableSignature(TypeFrameModelingVisitor.getType(field).getSignature())) {
                 return;
             }
             bugAccumulator.accumulateBug(new BugInstance(this, (staticMethod ? "MS" : "EI") + "_EXPOSE_"

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

@@ -39,7 +39,25 @@ public class MutableClasses {
             "com.google.common.collect.ImmutableSortedMap",
             "com.google.common.collect.ImmutableSortedMultiset",
             "com.google.common.collect.ImmutableSortedSet",
-            "com.google.common.collect.ImmutableTable"));
+            "com.google.common.collect.ImmutableTable",
+            "java.util.Collections$EmptyList",
+            "java.util.Collections$EmptyMap",
+            "java.util.Collections$EmptyNavigableMap",
+            "java.util.Collections$EmptySet",
+            "java.util.Collections$EmptyNavigableSet",
+            "java.util.Collections$SingletonList",
+            "java.util.Collections$SingletonMap",
+            "java.util.Collections$SingletonSet",
+            "java.util.Collections$UnmodifiableList",
+            "java.util.Collections$UnmodifiableMap",
+            "java.util.Collections$UnmodifiableNavigableMap",
+            "java.util.Collections$UnmodifiableSortedMap",
+            "java.util.Collections$UnmodifiableSet",
+            "java.util.Collections$UnmodifiableNavigableSet",
+            "java.util.Collections$UnmodifiableSortedSet",
+            "java.util.ImmutableCollections$AbstractImmutableList",
+            "java.util.ImmutableCollections$AbstractImmutableMap",
+            "java.util.ImmutableCollections$AbstractImmutableSet"));
 
     private static final Set<String> KNOWN_IMMUTABLE_PACKAGES = new HashSet<>(Arrays.asList(
             "java.math", "java.time"));

spotbugsTestCases/src/java11/Issue1771.java

@@ -0,0 +1,195 @@
+package ghIssues;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.NavigableSet;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.TreeSet;
+
+public class Issue1771 {
+    public Issue1771(List<String> list, Map<String, String> map, Set<String> set) {
+        this.list = List.copyOf(list);
+        this.map = Map.copyOf(map);
+        this.set = Set.copyOf(set);
+
+        list2 = List.of("foo", "bar");
+        map2 = Map.of("FOO", "foo", "BAR", "bar");
+        set2 = Set.of("foo", "bar");
+
+        List<String> l = new ArrayList<>();
+        l.add("foo");
+        l.add("bar");
+
+        Map<String, String> m = new HashMap();
+        m.put("FOO", "foo");
+        m.put("BAR", "bar");
+
+        NavigableMap<String, String> nm = new TreeMap();
+        nm.put("FOO", "foo");
+        nm.put("BAR", "bar");
+
+        SortedMap<String, String> sm = new TreeMap();
+        sm.put("FOO", "foo");
+        sm.put("BAR", "bar");
+
+        Set<String> s = new HashSet();
+        s.add("foo");
+        s.add("bar");
+
+        NavigableSet<String> ns = new TreeSet();
+        ns.add("foo");
+        ns.add("bar");
+
+        SortedSet<String> ss = new TreeSet();
+        ss.add("foo");
+        ss.add("bar");
+
+        list3 = Collections.unmodifiableList(l);
+        map3 = Collections.unmodifiableMap(m);
+        navigableMap3 = Collections.unmodifiableNavigableMap(nm);
+        sortedMap3 = Collections.unmodifiableSortedMap(sm);
+        set3 = Collections.unmodifiableSet(s);
+        navigableSet3 = Collections.unmodifiableNavigableSet(ns);
+        sortedSet3 = Collections.unmodifiableSortedSet(ss);
+
+        list4 = Collections.singletonList("foo");
+        map4 = Collections.singletonMap("FOO", "foo");
+        set4 = Collections.singleton("foo");
+
+        list5 = Collections.emptyList();
+        map5 = Collections.emptyMap();
+        navigableMap5 = Collections.emptyNavigableMap();
+        sortedMap5 = Collections.emptySortedMap();
+        set5 = Collections.emptySet();
+        navigableSet5 = Collections.emptyNavigableSet();
+        sortedSet5 = Collections.emptySortedSet();
+    }
+
+    private final List<String> list;
+    private final Map<String, String> map;
+    private final Set<String> set;
+
+    private final List<String> list2;
+    private final Map<String, String> map2;
+    private final Set<String> set2;
+
+    private final List<String> list3;
+    private final Map<String, String> map3;
+    private final NavigableMap<String, String> navigableMap3;
+    private final SortedMap<String, String> sortedMap3;
+    private final Set<String> set3;
+    private final NavigableSet<String> navigableSet3;
+    private final SortedSet<String> sortedSet3;
+
+    private final List<String> list4;
+    private final Map<String, String> map4;
+    private final Set<String> set4;
+
+    private final List<String> list5;
+    private final Map<String, String> map5;
+    private final NavigableMap<String, String> navigableMap5;
+    private final SortedMap<String, String> sortedMap5;
+    private final Set<String> set5;
+    private final NavigableSet<String> navigableSet5;
+    private final SortedSet<String> sortedSet5;
+
+    public List<String> getList() {
+        return list;
+    }
+
+    public Map<String, String> getMap() {
+        return map;
+    }
+
+    public Set<String> getSet() {
+        return set;
+    }
+
+    public List<String> getList2() {
+        return list2;
+    }
+
+    public Map<String, String> getMap2() {
+        return map2;
+    }
+
+    public Set<String> getSet2() {
+        return set2;
+    }
+
+    public List<String> getList3() {
+        return list3;
+    }
+
+    public Map<String, String> getMap3() {
+        return map3;
+    }
+
+    public NavigableMap<String, String> getNavigableMap3() {
+        return navigableMap3;
+    }
+
+    public SortedMap<String, String> getSortedMap3() {
+        return sortedMap3;
+    }
+
+    public Set<String> getSet3() {
+        return set3;
+    }
+
+    public NavigableSet<String> getNavigableSet3() {
+        return navigableSet3;
+    }
+
+    public SortedSet<String> getSortedSet3() {
+        return sortedSet3;
+    }
+
+    public List<String> getList4() {
+        return list4;
+    }
+
+    public Map<String, String> getMap4() {
+        return map4;
+    }
+
+    public Set<String> getSet4() {
+        return set4;
+    }
+
+    public List<String> getList5() {
+        return list5;
+    }
+
+    public Map<String, String> getMap5() {
+        return map5;
+    }
+
+    public NavigableMap<String, String> getNavigableMap5() {
+        return navigableMap5;
+    }
+
+    public SortedMap<String, String> getSortedMap5() {
+        return sortedMap5;
+    }
+
+    public Set<String> getSet5() {
+        return set5;
+    }
+
+    public NavigableSet<String> getNavigableSet5() {
+        return navigableSet5;
+    }
+
+    public SortedSet<String> getSortedSet5() {
+        return sortedSet5;
+    }
+}