rewrite-static-analysis icon indicating copy to clipboard operation
rewrite-static-analysis copied to clipboard

Change mutable public collections to immutable versions

Open yeikel opened this issue 3 years ago • 3 comments


public static final List<String> entries = Arrays.asList("A","B");

public static final List<String> entries = Collections.unmodifiableList(Arrays.asList("A","B"));

While List.of is also an option, it is important to note that List.of throws NPE for null access patterns (like contains) and that can be problematic.


var a = Collections.unmodifiableList(Arrays.asList("A","B"));
var b =Arrays.asList("A","B");
var c = List.of("A","B");

a.contains(null) // false
b.contains(null) // false
c.contains(null) // NPE

yeikel avatar Nov 11 '22 17:11 yeikel

Could you please assign it to me? I would attempt it in coming weeks...

coiouhkc avatar May 19 '23 11:05 coiouhkc

I have some doubts if this recipe could do harm in certain situations. Consider something like:

@Test
void doNotWrapIfSet() {
    rewriteRun(
      java("""
        import java.util.Arrays;
        import java.util.List;
        
        class A {
            public static void main(String[] args) {
                List<String> entries = Arrays.asList("A", "B");
                entries.set(0, "C");
                System.out.println(entries);
            }
        }
        """
      )
    );
}

This is allowed currently, as changing an element of the backing array does not change the size of the list.

But this recipe could convert that into Collections.unmodifiableList(Arrays.asList("A", "B")), which no longer allows entries.set(0, "C").

timtebeek avatar Jun 12 '23 07:06 timtebeek

You could use data flow to determine if entries is ever used with set

JLLeitschuh avatar Jun 15 '23 18:06 JLLeitschuh