How do I address unchecked cast warnings?

The obvious answer, of course, is not to do the unchecked cast.

If it's absolutely necessary, then at least try to limit the scope of the @SuppressWarnings annotation. According to its Javadocs, it can go on local variables; this way, it doesn't even affect the entire method.

Example:

@SuppressWarnings("unchecked")
Map<String, String> myMap = (Map<String, String>) deserializeMap();

There is no way to determine whether the Map really should have the generic parameters <String, String>. You must know beforehand what the parameters should be (or you'll find out when you get a ClassCastException). This is why the code generates a warning, because the compiler can't possibly know whether is safe.

Unfortunately, there are no great options here. Remember, the goal of all of this is to preserve type safety. "Java Generics" offers a solution for dealing with non-genericized legacy libraries, and there is one in particular called the "empty loop technique" in section 8.2. Basically, make the unsafe cast, and suppress the warning. Then loop through the map like this:

@SuppressWarnings("unchecked")
Map<String, Number> map = getMap();
for (String s : map.keySet());
for (Number n : map.values());

If an unexpected type is encountered, you will get a runtime ClassCastException, but at least it will happen close to the source of the problem.

Wow; I think I figured out the answer to my own question. I'm just not sure it's worth it! :)

The problem is the cast isn't checked. So, you have to check it yourself. You can't just check a parameterized type with instanceof, because the parameterized type information is unavailable at runtime, having been erased at compile time.

But, you can perform a check on each and every item in the hash, with instanceof, and in doing so, you can construct a new hash that is type-safe. And you won't provoke any warnings.

Thanks to mmyers and Esko Luontola, I've parameterized the code I originally wrote here, so it can be wrapped up in a utility class somewhere and used for any parameterized HashMap. If you want to understand it better and aren't very familiar with generics, I encourage viewing the edit history of this answer.

public static <K, V> HashMap<K, V> castHash(HashMap input,
                                            Class<K> keyClass,
                                            Class<V> valueClass) {
  HashMap<K, V> output = new HashMap<K, V>();
  if (input == null)
      return output;
  for (Object key: input.keySet().toArray()) {
    if ((key == null) || (keyClass.isAssignableFrom(key.getClass()))) {
        Object value = input.get(key);
        if ((value == null) || (valueClass.isAssignableFrom(value.getClass()))) {
            K k = keyClass.cast(key);
            V v = valueClass.cast(value);
            output.put(k, v);
        } else {
            throw new AssertionError(
                "Cannot cast to HashMap<"+ keyClass.getSimpleName()
                +", "+ valueClass.getSimpleName() +">"
                +", value "+ value +" is not a "+ valueClass.getSimpleName()
            );
        }
    } else {
        throw new AssertionError(
            "Cannot cast to HashMap<"+ keyClass.getSimpleName()
            +", "+ valueClass.getSimpleName() +">"
            +", key "+ key +" is not a " + keyClass.getSimpleName()
        );
    }
  }
  return output;
}

That's a lot of work, possibly for very little reward... I'm not sure if I'll use it or not. I'd appreciate any comments as to whether people think it's worth it or not. Also, I'd appreciate improvement suggestions: is there something better I can do besides throw AssertionErrors? Is there something better I could throw? Should I make it a checked Exception?

You can create a utility class like the following, and use it to suppress the unchecked warning.

public class Objects {

    /**
     * Helps to avoid using {@code @SuppressWarnings({"unchecked"})} when casting to a generic type.
     */
    @SuppressWarnings({"unchecked"})
    public static <T> T uncheckedCast(Object obj) {
        return (T) obj;
    }
}

You can use it as follows:

import static Objects.uncheckedCast;
...

HashMap<String, String> getItems(javax.servlet.http.HttpSession session) {
      return uncheckedCast(session.getAttribute("attributeKey"));
}

Some more discussion about this is here: http://cleveralias.blogs.com/thought_spearmints/2006/01/suppresswarning.html

In the HTTP Session world you can't really avoid the cast, since the API is written that way (takes and returns only Object).

With a little bit of work you can easily avoid the unchecked cast, 'though. This means that it will turn into a traditional cast giving a ClassCastException right there in the event of an error). An unchecked exception could turn into a CCE at any point later on instead of the point of the cast (that's the reason why it's a separate warning).

Replace the HashMap with a dedicated class:

import java.util.AbstractMap;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

public class Attributes extends AbstractMap<String, String> {
    final Map<String, String> content = new HashMap<String, String>();

    @Override
    public Set<Map.Entry<String, String>> entrySet() {
        return content.entrySet();
    }

    @Override
    public Set<String> keySet() {
        return content.keySet();
    }

    @Override
    public Collection<String> values() {
        return content.values();
    }

    @Override
    public String put(final String key, final String value) {
        return content.put(key, value);
    }
}

Then cast to that class instead of Map<String,String> and everything will be checked at the exact place where you write your code. No unexpected ClassCastExceptions later on.

In this particular case, I would not store Maps into the HttpSession directly, but instead an instance of my own class, which in turn contains a Map (an implementation detail of the class). Then you can be sure that the elements in the map are of the right type.

But if you anyways want to check that the contents of the Map are of right type, you could use a code like this:

public static void main(String[] args) {
    Map<String, Integer> map = new HashMap<String, Integer>();
    map.put("a", 1);
    map.put("b", 2);
    Object obj = map;

    Map<String, Integer> ok = safeCastMap(obj, String.class, Integer.class);
    Map<String, String> error = safeCastMap(obj, String.class, String.class);
}

@SuppressWarnings({"unchecked"})
public static <K, V> Map<K, V> safeCastMap(Object map, Class<K> keyType, Class<V> valueType) {
    checkMap(map);
    checkMapContents(keyType, valueType, (Map<?, ?>) map);
    return (Map<K, V>) map;
}

private static void checkMap(Object map) {
    checkType(Map.class, map);
}

private static <K, V> void checkMapContents(Class<K> keyType, Class<V> valueType, Map<?, ?> map) {
    for (Map.Entry<?, ?> entry : map.entrySet()) {
        checkType(keyType, entry.getKey());
        checkType(valueType, entry.getValue());
    }
}

private static <K> void checkType(Class<K> expectedType, Object obj) {
    if (!expectedType.isInstance(obj)) {
        throw new IllegalArgumentException("Expected " + expectedType + " but was " + obj.getClass() + ": " + obj);
    }
}

I may have misunderstood the question(an example and a couple of surrounding lines would be nice), but why don't you always use an appropriate interface (and Java5+)? I see no reason why you would ever want to cast to a HashMap instead of a Map<KeyType,ValueType>. In fact, I can't imagine any reason to set the type of a variable to HashMap instead of Map.

And why is the source an Object? Is it a parameter type of a legacy collection? If so, use generics and specify the type you want.

If I have to use an API that doesn't support Generics.. I try and isolate those calls in wrapper routines with as few lines as possible. I then use the SuppressWarnings annotation and also add the type-safety casts at the same time.

This is just a personal preference to keep things as neat as possible.

A quick guess if you post your code can say for sure but you might have done something along the lines of

HashMap<String, Object> test = new HashMap();

which will produce the warning when you need to do

HashMap<String, Object> test = new HashMap<String, Object>();

it might be worth looking at

Generics in the Java Programming Language

if your unfamiliar with what needs to be done.

Almost every problem in Computer Science can be solved by adding a level of indirection*, or something.

So introduce a non-generic object that is of a higher-level that a Map. With no context it isn't going to look very convincing, but anyway:

public final class Items implements java.io.Serializable {
    private static final long serialVersionUID = 1L;
    private Map<String,String> map;
    public Items(Map<String,String> map) {
        this.map = New.immutableMap(map);
    }
    public Map<String,String> getMap() {
        return map;
    }
    @Override public String toString() {
        return map.toString();
    }
}

public final class New {
    public static <K,V> Map<K,V> immutableMap(
        Map<? extends K, ? extends V> original
    ) {
        // ... optimise as you wish...
        return Collections.unmodifiableMap(
            new HashMap<String,String>(original)
        );
    }
}

static Map<String, String> getItems(HttpSession session) {
    Items items = (Items)
        session.getAttribute("attributeKey");
    return items.getMap();
}

*Except too many levels of indirection.

Just typecheck it before you cast it.

Object someObject = session.getAttribute("attributeKey");
if(someObject instanceof HashMap)
HashMap<String, String> theHash = (HashMap<String, String>)someObject;  

And for anyone asking, it's quite common to receive objects where you aren't sure of the type. Plenty of legacy "SOA" implementations pass around various objects that you shouldn't always trust. (The horrors!)

EDIT Changed the example code once to match the poster's updates, and following some comments I see that instanceof doesn't play nicely with generics. However changing the check to validate the outer object seems to play well with the commandline compiler. Revised example now posted.