False positive EI_EXPOSE_REP when java.util.Map.copyOf is used

[jochenberger]

The following code triggers EI_EXPOSE_REP

package spotbogs.test;

import java.util.Map;

public class Foo {

    public Foo(Map<String, String> data) {
        this.data = Map.copyOf(data);
    }

    private final Map<String, String> data;

    public Map<String, String> getData() {
        return data;
    }
}

Since Map.copyOf returns an unmodifiable map (java.util.ImmutableCollections.AbstractImmutableMap), this is a false positive.
The same is true for List.copyOf and Set.copyOf.

Sample project:
copyof-issue.zip

[welcome]

Thanks for opening your first issue here! 😃
Please check our contributing guideline. Especially when you report a problem, make sure you share a Minimal, Complete, and Verifiable example to reproduce it in this issue.

[jochenberger]

The same is true for unmodifiable collections created by from streams using for example Collectors.toUnmodifiableList.
Updated Java code:

package spotbugs.test;

import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class Foo {

    public Foo(Map<String, String> map, List<String> list, Set<String> set) {
        this.map = Map.copyOf(map);
        this.list = List.copyOf(list);
        this.set = Set.copyOf(set);
    }

    private final Map<String, String> map;
    private final List<String> list;
    private final Set<String> set;

    private final List list2 = Stream.of().collect(Collectors.toUnmodifiableList());

    public Map<String, String> getMap() {
        return map;
    }

    public List<String> getList() {
        return list;
    }

    public Set<String> getSet() {
        return set;
    }

    public List getList2() {
        return list2;
    }
}

Updated sample project: sample-project.zip

Findings:

M V EI: spotbugs.test.Foo.getList() may expose internal representation by returning Foo.list  At Foo.java:[line 28]
M V EI: spotbugs.test.Foo.getList2() may expose internal representation by returning Foo.list2  At Foo.java:[line 36]
M V EI: spotbugs.test.Foo.getMap() may expose internal representation by returning Foo.map  At Foo.java:[line 24]
M V EI: spotbugs.test.Foo.getSet() may expose internal representation by returning Foo.set  At Foo.java:[line 32]

[baloghadamsoftware]

Thank you for reporting this! I think that I understand the problem and I have an idea how to solve it. However, it is not easy because we need to find all possible assignments to the field and check whether all of them assigns an immutable concrete type. StoreAnalysis might help, but I am not yet sure whether I need to use it directly or create another analysis on top of it for this particular problem.

[baloghadamsoftware]

I think that I found the solution. Fix is in progress. Fortunately, we have field summaries thus it is not so difficult at all.

[baloghadamsoftware]

#2141 fixes the original issue. Collectors.toUnmodifiableList is a bit more complicated, it will be fixed in a future PR.