Eamonn McManus's Blog

Getting rid of unchecked warnings for casts

If you've ever made a serious effort to get rid of "unchecked" warnings from the Java compiler (the ones it gives you with -Xlint:unchecked) then you'll probably have found some cases where you know a cast is correct but you can't convince the compiler of it. Is there anything better than adding @SuppressWarnings("unchecked") around the whole method?

The problem with the @SuppressWarnings solution is that it's much too broad. It suppresses warnings for the whole method, even though it's just one little cast that you want to allow. What if there's another place in the method where the compiler would have given a warning, telling you about a real problem? There's also a readability problem. If it's a big method, it can be hard to work out which particular line is the reason for the @SuppressWarnings annotation; you have to add a comment saying "yoo-hoo! I'm the reason!"

Fortunately, there's a good solution. Suppose your code does this:

    List<String> list = (List<String>) x;

You can change it to this:

    List<String> list = cast(x);

where the cast method is defined like this:

@SuppressWarnings("unchecked")
private static <T> T cast(Object x) {
    return (T) x;
}

If there several different classes where you need this method, you could make it public and put it in a utility class, say Util.java. Then every class that needs it can use import static com.example.mypackage.Util.cast so you can still write just plain cast as above.

The magic here is that the compiler can use type inference to figure out what T should be in the method call. In the example, since we're assigning the result of cast to a List<String>, the compiler can infer that T must be List<String>.

The compiler can do this if you're assigning the result of cast to a variable or returning it from a method or using it as a method parameter. In more complicated expressions you might need to break the cast call out into a separate assignment.

In some cases even this won't work. For example, if you want to use the cast method to cast to a type variable you are out of luck:

    <E> Set<E> result(Object x) {
        E e = cast(x);  // does not compile
        return Collections.singleton(e);
    }

You can work around this using the little-known syntax for "explicit type parameters":

    <E> Set<E> result(Object x) {
        E e = Util.<E>cast(x);
        return Collections.singleton(e);
    }

This is ugly, notably because you can't write just <E>cast(x); the explicit type parameter has to appear after a dot. But it shouldn't happen very often.

This cast technique is basically a way to move all of the @SuppressWarnings annotations from your code to single place. You should still follow the same guidelines as you would have without cast. It should only be used as a last resort if you can't restructure your code to eliminate the warnings. And you must be really sure that the value being cast really is of the type you are casting it to. Otherwise you will get a ClassCastException at some completely different part of your code, where there might not even be an explicit cast.

(By the way I'm sure this cast method has been independently invented many times, for example here.)

Comments

• I'm not sure it's a great idea to have such a general carpet sweeping method. Keeping duplicated cast methods as close to its usage as possible would be a good start.

  There are some cases where unchecked casts are inevitable, but you can make the cast method smarter. For instance reading a generic object through serialisation (without defaultReadObject) will always be a problem. However you can move the readObject (or readUnshared) call into the case method.

  @SuppressWarnings("unchecked")
  static <T> T readObject(
  java.io.ObjectInputStream in
  ) throws java.io.IOException, java.lang.ClassNotFoundException {
  return (T)in.readObject();
  }

  There's a usenet thread here.

  Posted by: tackline on March 30, 2007 at 03:56 AM

• Tom, agreed that caution and restraint are appropriate (as I mentioned in my last paragraph). Also it is a good idea as you suggest to move the inference magic to a higher level to group several similar calls to cast.

  I'm not sure I see what is to be gained by "keeping duplicated cast methods as close to its usage as possible"; I interpret this as meaning having a private cast method in every class that needs one. The disadvantages I see are: you have to delete the cast method explicitly when it is no longer used in its class (maybe your IDE can help, but still); you can't use your IDEs "find usages" method to find everywhere in your code that you are using this unsafe trick; and if you need to pass a type parameter then you have to write MyRandomClassName.<T>cast(x).

  Posted by: emcmanus on March 30, 2007 at 04:06 AM

• For your singleton example it's not necessary to create own utility method, functionality of Class class is enough:

  
  <E> Set<E> result(Object x, Class<E> clazz) {
  E e = clazz.cast(x);
  return Collections.singleton(e);
  }

  ...
  Set<String> keys = result(myKey, String.class);

  VS

  Posted by: vsilaev on March 30, 2007 at 05:39 AM

• Valery,

  Passing an explicit Class parameter is certainly one good way of restructuring the code to eliminate warnings. But it does require callers to change. I don't think the particular example I gave is very good (how can we be sure that x really is of type E? if we know it is, couldn't we declare it as an E?).

  As a data point, the JMX sources in JDK 7 contain 29 uses of Util.cast. That's quite a lot, but at least partly reflects the dynamically-typed nature of the JMX API. Of the 29, 8 have an explicit type parameter. In three cases, that parameter is a type variable, and in the other five it is Class<Object> or Class<T>.

  The example of
  com.sun.jmx.remote.util.CacheMap is perhaps more realistic. Here's the get(Object) method, specified by Map<K, V>:

  public V get(Object key) {
  cache(Util.<K>cast(key));
  return super.get(key);
  }

  We're relying on the caller of this internal class not to pass us anything other than a K, though in fact the use of K and V throughout is just for readability. We don't want to have to specify the real class of K via a Class parameter on the constructor, and we don't want to use Object everywhere either. Hence Util.cast.

  Posted by: emcmanus on March 30, 2007 at 06:40 AM

• In the serialisation case, it's trivial to put the generic reference inside a non-generic class, and serialise and deserialise that. Then there's no casting issue.

  Posted by: ricky_clarkson on March 30, 2007 at 07:42 AM

• The @SuppressWarnings annotation also applies to local variables. (It's one of the Target values for the annotation.) So you can simply do

  
  @SuppressWarnings("unchecked")
  List list = (List) x;

  
  within your method. This makes it perfectly clear what cast it applies to.

  Disclaimer: I haven't tested this with javac, but it works fine in Eclipse.

  Posted by: basdebakker on March 30, 2007 at 08:14 AM

• Sorry, I forgot to quote the HTML characters, that should have been

  @SuppressWarnings("unchecked")
  List<String> list = (List<String>) x;
  

  Posted by: basdebakker on March 30, 2007 at 08:17 AM

• Bas,

  Well, blow me down! I didn't know you could do that. It does make the cast method logically unnecessary, though still considerably more concise.

  Your suggestion does work with javac, by the way.

  Posted by: emcmanus on March 30, 2007 at 08:49 AM

• Thanks for that: straight in my GenUtils toolbox!

  Rgds

  Damon

  Posted by: damonhd on March 30, 2007 at 09:40 AM

• Peronally, I think its pretty clear that unchecked warnings in Java can't keep things straight. They should disable them by default (not even ask you to run with -Xlint:unchecked). I'm not kidding. They can keep errors for "List<String> blah = getListOfIntegers();", but I just turn off the unchecked warnings in Eclipse. I don't find them helpful at all.

  Posted by: tompalmer on March 30, 2007 at 09:48 AM

• ricky_clarkson: Do you mean using a reference of the erased type? Say copying a List<String> to a List? You still end up having to get back from List to List<String>, which should give you an unchecked.

  Eamonn: I think it's more obvious (and easier to find in the general case) to see @SuppressWarnings than happen to notice the significance of 'cast'. Heed your compiler's warnings, and if you don't, make it obvious that you are not. At least use an ugly name - REIFICATION_CAST or something. If I was scanning through the code, I wouldn't notice the problem. I would have to be looking at it quite intently to see.

  So I had a search for cast() in com.sun.jmx, JDK7 b07 (really don't like the static import, btw).

  
  Introspector - two uses of <Class<Object>>cast(). There can only be one Class<Object>. You could use Object.class. Or perhaps the compiler was right to warn.
  CacheMap - bad cast. There is no reason to assume that key is of type K.
  Service - I believe this should read service.cast(...) (i.e. use Class.cast).
  SnmpOidDatabaseSupport - okay(ish). This is 'necessary' because the interface used is not generified. Actually, it returns Vector<?> (generally a bad practice) so it isn't actually necessary.

  Posted by: tackline on March 30, 2007 at 12:48 PM

• Tom,

  Thanks for the free code review! :-)

  I agree that the Introspector lie is ugly. The problem is that at this point we have determined that c is a Class representing an interface that the object mbean implements. So it meets the constraint expressed by the StandardMBeanSupport constructor:

  public <T> StandardMBeanSupport(T resource, Class<T> mbeanInterface)

  However there's no clean way to convince the compiler of that fact. But Util.<Class>cast() would be less unclean than Util.<Class<Object>>cast().

  Concerning CacheMap, I judged that the readability advantage of being able to use K in the definition of the LinkedList<SoftReference<K>> field outweighed the potential for incorrect usage of this non-public class, especially since such usage would not in fact break anything. Alas, non-reified generics imply a lot of compromises of this sort.

  Concerning Service, you are right; it is always better to use Class.cast if there is an appropriate Class to hand. In fact I noticed this when looking for an example of a cast to a type variable in my reply to Valery.

  Concerning SnmpOidDatabaseSupport, these ancient SNMP packages were by far my biggest headache when de-linting the JMX sources. I had to use Vector<?> because the inheritance hierarchy includes methods that return Vector<SnmpOidRecord> that are overridden by methods that return Vector<SnmpOidTable>. Ugh. This is legacy code that we're not motivated to modify more than is strictly necessary for fear of breaking it, so a bigger rewrite was not on the cards.

  Posted by: emcmanus on April 02, 2007 at 02:43 AM

• Hmmm, following up to myself here about the Introspector code.

  In fact using <Class> instead of Class<Object> does not work - I was fooled by a stray @SuppressWarnings on the method in question. However, I did find another solution which is better than Class<Object>. In outline it is:

  
  public static <T> DynamicMBean makeDynamicMBean(T mbean)
  throws NotCompliantMBeanException {
  final Class<?> mbeanClass = mbean.getClass();
  Class<? super T> c = Util.cast(getStandardMBeanInterface(mbeanClass));
  if (c != null)
  return new StandardMBeanSupport(mbean, c);
  ...

  
  We are no longer lying to the compiler - getStandardMBeanInterface really does return a Class that is "super T".

  Adding a type parameter to the method just so we can make things work inside it is a bit bizarre, and if this method were part of the public API I would forward to a private method with the type parameter.

  Posted by: emcmanus on April 03, 2007 at 02:04 AM

• Tackline (Tom Hawtin),

  No, I mean an instance of a generic type referred to by an instance of a non-generic type. E.g.:

  
  class StuffImGoingToSerialise implements Serializable
  {
  List<String> stringList;
  }

  You can serialise and deserialise instances of that without the warnings.

  Posted by: ricky_clarkson on April 04, 2007 at 10:13 AM

• I explained all kind of casting operations on my blog.. If you want to know anything else feel free to contact me by mail.
  

  Posted by: angelas on May 25, 2007 at 01:45 PM

• Your picture is quite lovely...I like it...

  Posted by: bookworm02 on September 18, 2007 at 01:46 AM

This work is licensed under a Creative Commons License.