4

Suppose I have to following set:

Set<String> fruits = new HashSet<String>()
fruits.add("Apple")
fruits.add("Grapes")
fruits.add("Orange")

Set<String> unmodifiableFruits = Collections.unmodifiableSet(new HashSet<String>(fruits))
unmodifiableFruits.add("Peach") // -- Throws UnsupportedOperationException

Set<String> fruitSet = Collections.unmodifiableCollection(fruits)
fruitSet.add("Peach")
println(fruitSet)

If I were to use Collections.unmodifiableSet() it throws an exception when I attempt to use the add() method, but that isn't the case for Collections.unmodifiableCollection(). Why?

According the the documentation it should throw an error:

Returns an unmodifiable view of the specified collection. This method allows modules to provide users with "read-only" access to internal collections. Query operations on the returned collection "read through" to the specified collection, and attempts to modify the returned collection, whether direct or via its iterator, result in an UnsupportedOperationException.

All the code is written using Groovy 2.5.2

sp00m
  • 47,968
  • 31
  • 142
  • 252
  • It _is_ the case. – Louis Wasserman Aug 30 '18 at 18:57
  • @sp00m - In `Groovy`, it does compile. –  Aug 30 '18 at 19:01
  • 1
    I suspect that Groovy is doing some implicit conversion from the type returned by `unmodifiableCollection` (`java.util.Collections$UnmodifiableCollection` as far as my test goes) to `Set`. It's likely that it's creating a new, modifiable set (`java.util.LinkedHashSet` was the type assigned to `fruitSet` in my test). – ernest_k Aug 30 '18 at 19:02
  • @ernest_k - It's kind of misleading. The documentation clearly states *Returns an unmodifiable view of the specified collection.* That doesn't seem to be the case. –  Aug 30 '18 at 19:03
  • Wait, you're right: https://ideone.com/qIzCCm! – sp00m Aug 30 '18 at 19:03
  • Well, it would only be misleading if the implicit conversions were not specified somewhere ;-) – GhostCat Aug 30 '18 at 19:03
  • @sp00m - ;D. I'm looking at it in the eclipse IDE right now. –  Aug 30 '18 at 19:04
  • @GhostCat - I couldn't find it, but maybe I'm looking in the wrong place, but does it say anywhere that `unmodifiableCollection` creates a modifiable `set` and returns it. –  Aug 30 '18 at 19:07
  • Well, not exactly, it says an "unmodifiable [...] collection". I'm wondering how Groovy deals with that weird implicit conversion to `Set`... – sp00m Aug 30 '18 at 19:08
  • @ernest_k - Quiet interesting, and unexpected when reading the documentation. –  Aug 30 '18 at 19:08
  • @Sveta I agree. But if I were you, I would try to avoid type conversions that I do not explicitly control (I personally consider that risky). – ernest_k Aug 30 '18 at 19:10
  • 1
    @ernest_k - In this case, the `unmodifiableSet` is the better choice? –  Aug 30 '18 at 19:13
  • @Sveta It's clearly the better option of the two in your case. In the case you're expected to use `unmodifiableCollection`, then assign it to a `Collection` variable as per the method's return type (unless you inspect the actually returned data type, which would be superfluous given the existence of `unmodifiableSet`) – ernest_k Aug 30 '18 at 19:19
  • @ernest_k - Agreed. Would it be a good idea to notify the writers of the docs? Maybe let them know that it's misleading? –  Aug 30 '18 at 19:21
  • The documentation you linked to is for `java.util.Collection`, and that is not what introduced the confusion. This behavior is Groovy's (the implicit conversion). It's possible that it's documented in Groovy documentation. Check this: http://docs.groovy-lang.org/latest/html/documentation/#_custom_type_coercion – ernest_k Aug 30 '18 at 19:28
  • @ernest_k - The `Set` documentation is from Groovy, but that links to the `Collections` documentation which is from Java. To any reader, it's confusing if you read the docs and attempt to write the code. –  Aug 31 '18 at 12:26

1 Answers1

2

Short answer: adding Peach to this collection is possible, because Groovy does dynamic cast from Collection to Set type, so fruitSet variable is not of type Collections$UnmodifiableCollection but LinkedHashSet.

Take a look at this simple exemplary class:

class DynamicGroovyCastExample {

  static void main(String[] args) {
    Set<String> fruits = new HashSet<String>()
    fruits.add("Apple")
    fruits.add("Grapes")
    fruits.add("Orange")

    Set<String> fruitSet = Collections.unmodifiableCollection(fruits)
    println(fruitSet)
    fruitSet.add("Peach")
    println(fruitSet)
  }
}

In statically compiled language like Java, following line would throw compilation error:

Set<String> fruitSet = Collections.unmodifiableCollection(fruits)

This is because Collection cannot be cast to Set (it works in opposite direction, because Set extends Collection). Now, because Groovy is a dynamic language by design, it tries to cast to the type on the left hand side if the type returned on the right hand side is not accessible for the type on the left side. If you compile this code do a .class file and you decompile it, you will see something like this:

//
// Source code recreated from a .class file by IntelliJ IDEA
// (powered by Fernflower decompiler)
//

import groovy.lang.GroovyObject;
import groovy.lang.MetaClass;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
import org.codehaus.groovy.runtime.callsite.CallSite;

public class DynamicGroovyCastExample implements GroovyObject {
    public DynamicGroovyCastExample() {
        CallSite[] var1 = $getCallSiteArray();
        MetaClass var2 = this.$getStaticMetaClass();
        this.metaClass = var2;
    }

    public static void main(String... args) {
        CallSite[] var1 = $getCallSiteArray();
        Set fruits = (Set)ScriptBytecodeAdapter.castToType(var1[0].callConstructor(HashSet.class), Set.class);
        var1[1].call(fruits, "Apple");
        var1[2].call(fruits, "Grapes");
        var1[3].call(fruits, "Orange");
        Set fruitSet = (Set)ScriptBytecodeAdapter.castToType(var1[4].call(Collections.class, fruits), Set.class);
        var1[5].callStatic(DynamicGroovyCastExample.class, fruitSet);
        var1[6].call(fruitSet, "Peach");
        var1[7].callStatic(DynamicGroovyCastExample.class, fruitSet);
    }
}

The interesting line is the following one:

Set fruitSet = (Set)ScriptBytecodeAdapter.castToType(var1[4].call(Collections.class, fruits), Set.class);

Groovy sees that you have specified a type of fruitSet as Set<String> and because right side expression returns a Collection, it tries to cast it to the desired type. Now, if we track what happens next we will find out that ScriptBytecodeAdapter.castToType() goes to:

private static Object continueCastOnCollection(Object object, Class type) {
    int modifiers = type.getModifiers();
    Collection answer;
    if (object instanceof Collection && type.isAssignableFrom(LinkedHashSet.class) &&
            (type == LinkedHashSet.class || Modifier.isAbstract(modifiers) || Modifier.isInterface(modifiers))) {
        return new LinkedHashSet((Collection)object);
    }

// .....
}

Source: src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java#L253

And this is why fruitSet is a LinkedHashSet and not Collections$UnmodifableCollection.

enter image description here

Of course it works just fine for Collections.unmodifiableSet(fruits), because in this case there is no cast needed - Collections$UnmodifiableSet implements Set so there is no dynamic casting involved.

How to prevent similar situations?

If you don't need any Groovy dynamic features, use static compilation to avoid problems with Groovy's dynamic nature. If we modify this example just by adding @CompileStatic annotation over the class, it would not compile and we would be early warned:

enter image description here

Secondly, always use valid types. If the method returns Collection, assign it to Collection. You can play around with dynamic casts in runtime, but you have to be aware of consequences it may have.

Hope it helps.

Szymon Stepniak
  • 40,216
  • 10
  • 104
  • 131
  • So, it's better to use `Collections.unmodifiableSet`? Given that I know my colllection is a set, it's the safest method to call, correct? –  Aug 31 '18 at 12:28
  • @Sveta Correct. Static compilation would not allow you assign `Collection` to `Set`, so it is always a good choice to be as explicit as possible. – Szymon Stepniak Aug 31 '18 at 13:57
  • Would it be a good idea to notify the writers of the docs that `Collections.unmodifiableCollection` page doesn't accurately describe what the method does? The `Set` page is from Groovy but it links to Java's `Collections.unmodifiableCollection`. To any reader, it might be confusing. –  Aug 31 '18 at 14:14
  • Documentation of `Collections.unmodifiableCollection()` is OK and it returns what it describes. The problem you faced is a Groovy "feature" to cast between incompatible types. Right side expression `Collections.unmodifiableCollection()` returned unmodifiable collection, then Groovy compared with the type on the left side and found `Set` and because `Collection` cannot be cast to `Set` it applied `ScriptBytecodeAdapter.castToType()` to create a `Set` from given `Collection`. This is just abnormal usage powered by dynamic cast from type `Collection` to `Set`. – Szymon Stepniak Aug 31 '18 at 15:12
  • The issue is Groovy's `Set` documentation links to Java's documentation about `Collections.unmodifiableCollection()` which might give readers the idea that `Collections.unmodifiableCollection()` in both Groovy and Java function the same, but as you demonstrated, it doesn't. In Java, `Collections.unmodifiableCollection()` throws an exception, when you attempt to call `add()` just like the docs explain. In Groovy, it doesn't do that, as you demonstrated, it created to `Set` from the given collection. –  Aug 31 '18 at 15:32
  • It would be nice to be notified of the behavior before using `Collections.unmodifiableCollection()`. –  Aug 31 '18 at 16:01