What causes this "unchecked cast" warning?

user_14837054 0
Viewed 160

In java, i have made (and am using without issues), this method:

private final HashMap<String, Object> data = new HashMap<>();

    public final @NotNull <clazz> Collection<clazz> getCollection(@NotNull String key, @NotNull Object clazz) {
        Object object = this.data.get(key);
        if (object instanceof Collection) {
            Collection<?> collection = (Collection<?>) object;
            for (Object o : collection) {
                if (o.getClass().equals(clazz)) {
                    return (Collection<clazz>) collection;
                }
                break;
            }
        }
        return Collections.emptyList();
    }

Specifically, the line return (Collection<clazz>) collection; is producing the warning:

  • Unchecked cast: 'java.util.Collection<capture<?>>' to 'java.util.Collection<clazz>'

Honesty where appropriate: i improvised this method out of neccessity about one year ago, shortly after i started learning to code. Unfortunately, even now, i still do not understand what exactly the <clazz> stuff i put there is doing, why it works and how it works.

In that sense: i'm looking to understand - not to be spoonfed the solution to remove the warning message, of course. (And i totally didn't use @SuppressWarnings("unchecked") prior to this post.)

/edit:

After reading the replies so far, i am in the ball park of understand why this warning is produced.

Now, for a theoretical workaround that should solve the problem:

    public final @NotNull <T> Collection<T> getCollection(@NotNull String key, @NotNull Class<T> dataType) {
        Object object = this.data.get(key);
        Collection<T> r = new ArrayList<>();
        if (object instanceof Collection) {
            Collection<?> collection = (Collection<?>) object;
            for (Object o : collection) {
                if (o.getClass().equals(dataType)) {
                    r.add((T) o);
                }
            }
        }
        return r;
    }

But now the line r.add((T) o); is producing the same warning. There must be a better condition other than o.getClass().equals(dataType) that will make the IDE understand. (IntelliJ Community Edition, by the way)

java
3 Answers

There are two clazz variables here: the first is the second parameter to the method, and the other is the type parameter <clazz> of the method. I strongly advise to rename the second to avoid this ambiguity (normally type parameters are named with a single uppercase letter)`.

The reason the compiler is showing this warning is that it cannot guarantee (i.e. check at compile time) that the cast is safe. Nothing guarantees that the type of the parameter which you are doing the check against the element type is the same as the type parameter. So you can possible have an Integer passed as the argument, while you are returning a Collection<String> by calling the method with a String type argument:

data.put("a", List.of(1, 2));
    
Collection<String> collection = getCollection("a", Integer.class);
String first = collection.stream().findAny().get();  // ClassCastException at runtime

You can slightly improve that by having the class use the same type parameter:

public final @NotNull <T> Collection<T> getCollection(@NotNull String key, @NotNull Class<T> clazz) {
    Collection<?> object = data.get(key);
    if (object instanceof Collection) {
        Collection<?> collection = (Collection<?>) object;
        for (Object o : collection) {
            if (o.getClass().equals(clazz)) {
                return (Collection<T>) collection;
            }
            break;
        }
    }
    return Collections.emptyList();
}

But still this warning will stay because it's up to you to guarantee that all the collection elements are of the type T at runtime:

data.put("a", List.of("x", 2));
    
Collection<String> collection = SOTest.getCollection("a", String.class);
for(String s : collection) {
     System.out.println(s);  // second element will still cause ClassCastException
}
user_10642450

In addition to the (correct) observations in M Anouti's answer:

You are only checking whether the type of the first member matches your expectations. This is not enough to determine the type of the collection in general. For example:

public static void main(String[] args) {

    Main m = new Main();        
    List<Number> numbers = List.of(1, 0.0, BigInteger.valueOf(4));
    m.data.put("nums", numbers);
            
    Collection<Integer> coll = m.getCollection("nums", Integer.class);
    List<Integer> output = new ArrayList<>(coll);
    System.out.println(output);
    output.sort(Integer::compare);
}

Output:

[1, 0.0, 4]
Exception in thread "main" java.lang.ClassCastException: class java.lang.Double cannot be cast to class java.lang.Integer (java.lang.Double and java.lang.Integer are in module java.base of loader 'bootstrap')
    at java.base/java.util.TimSort.countRunAndMakeAscending(TimSort.java:355)
    at java.base/java.util.TimSort.sort(TimSort.java:220)
    at java.base/java.util.Arrays.sort(Arrays.java:1306)
    at java.base/java.util.ArrayList.sort(ArrayList.java:1721)
    at world.Main.main(Main.java:23)

As you can see, the unsafe cast leads to an incorrect generic type in the result, and this is only detected at runtime when it is actually attempted to use the members as instances of the declared type. In this example, the println accepts the incorrect type without exception, because that method does not care - it just invokes toString, and every Object has that. Only when we try to invoke Integer.compare, this fails.

Even if you check all elements, just because a collection currently does not contain an element of a given type does not mean it never will - if the collection is actually a Collection<Number>, but currently only contains Doubles, this does not mean that it can be safely used as a Collection<Double>.

user_25132000

Testing o.getClass().equals(clazz) doesn't mean that it's a Collection<clazz>.

Collection<clazz> can be thought of as the intersection of two types:

  • Collection<? extends clazz> is a Collection in which all elements are instances of clazz, or are null.
  • Collection<? super clazz> is a Collection to which it is acceptable to add elements of clazz, or null.

All you are determining in testing that the first element is an instance of clazz is that it is acceptable to have added an instance of clazz to that Collection: that is, you can only determine that it's an instance of Collection<? super clazz>, because the presence of an instance of clazz indicates that it would be safe to add another instance of clazz.

And the fact you're only checking the first element is irrelevant: you can check all of the elements of the list, and still you don't know any more information: all you can determine is that at the time you tested the elements, there were no instances of another class in the collection.

There are, in general, no guarantees that will remain true over time. If some other part of your code has access to the same Collection instance, but this has additional type information which permit instances of a more general type to be added to the Collection, this can break the type-safety of the value returned by getCollection.

To give a practical example:

List<Object> list = new ArrayList<>();
list.add(0);
list.add(1);

// This is the class in the question. I don't know the API for
// adding stuff to it, so I'm just making it up.
YourClass yourClass = new YourClass();
yourClass.put("key", list);

// Invoke the method in the question to get an unsafe collection.
Collection<Integer> unsafeCollection = yourClass.getCollection("key", Integer.class);

// This affects the contents of unsafeCollection.
list.clear();
list.add("hello");

// Now you will get a ClassCastException, because list (and collection,
// which is the same instance as list) only contains Strings.
for (Integer i : unsafeCollection) {}

However, if you had declared your method to return a Collection<? super Integer>:

Collection<? super Integer> safeCollection = yourClass.getCollectionSafe("key", Integer.class);

You wouldn't be able to iterate this assuming an Integer element type:

for (Integer i : safeCollection) {}  // Compiler error.

But you could for Object element type:

for (Object o : safeCollection) {}  // Fine.

So it wouldn't matter if list.add("hello") had been invoked.

But now the line r.add((T) o); is producing the same warning

It is important to understand what unchecked warnings are actually telling you: the compiler is unable to insert any instruction to determine if that the cast on that line is correct or not (as opposed to a checked cast, which would result in a checkcast bytecode instruction). A checkcast needs to know the type it's checking against, statically; it doesn't know what T is here because it's a type variable.

You are given the ability to tell the compiler, via @SuppressWarnings: "trust me, I know this is safe"; but in doing so, you are assuming responsibility for ensuring that it is. In that case, you can be sure it is safe, because of the immediately preceding check, so suppression is appropriate.

You might consider rewriting with a local variable, in order to be able to apply the suppression to only that cast, rather than having to apply it to the whole method:

if (o.getClass().equals(dataType)) {
  @SuppressWarnings("unchecked")  // Safe, because of preceding check.
  T castO = (T) o;
  r.add(castO);
}
user_37881760
Related