Narrowing reference conversions should always check for type-compatibility

Description of the Rule

Narrowing reference conversions should always check for type-compatibility.

According to the spec “such conversions require a test at run time to find out whether the actual reference value is a legitimate value of the new type. If not, then a ClassCastException is thrown.”.

And consequently the compiler cannot warn about these cases.

In our particular example (see the link in the references below) we had an object holding a raw reference to a generic class Link<T>. A method of that referenced object gets called, returning an instance of the generic type parameter, hence Object at runtime. This object was down casted to something it was not compatible causing a ClassCastException at runtime. While this is the expected behaviour, it would have been good to have it flagged earlier.

Noncompliant Code

public class Test {
    public Object returnAnObject() {
        return new Object();
    }
    public Number returnAnObjectAsNumber() {
        return (Number) returnAnObject();
    }
}

Compilant Code

public class Test {
    public Object returnAnObject() {
        return new Object();
    }
    public Number returnAnObjectAsNumber() {
        Object anObject = returnAnObject();
        return anObject instanceof Number ? anObject : null;
    }
}

Exceptions

Maybe, if the method is declared to throw a ClassCastException that should not raise an issue, hence the following would be compliant as well

public class Test {
    public Object returnAnObject() {
        return new Object();
    }
    public Number returnAnObjectAsNumber() throws ClassCastException {
        Object anObject = returnAnObject();
        return (Number) anObject;
    }
}

External references and/or language specifications

type

Bug

Hello @Buuhuu,

I’m not sure I completely understood your suggestion. In the description I noticed, you’re talking about generic raw type. As far as I know, it’s absolutely discouraged to use raw types, as this way compiler won’t actually help you with proper typing and you will get exceptions in the runtime. And we have a rule for this : Java static code analysis: Raw types should not be used

About your general suggestion, while you are right we should check the type before casting, I am afraid this might cause too much noise. As Java doesn’t have Smart casts yet, we need to do much more casts than it is actually referenced.

However, this could be a nice idea for an advanced rule (Symbolic Execution) At the moment this is not possible due to the limitations of our engine, but if we’re able to track the type during the execution path, we can report an issue only on the lines that can produce ClassCastExceptions.

Regards,
Margarita