ClassCastExceptions with JDK9 javac

[cushon]

JDK 9 fixes a javac bug (JDK-8058199) that was causing checkcast intructions to be skipped. Previously javac used the parameter types of a method symbol's erased type as targets when translating the arguments. In JDK 9, javac has been fixed to use the inferred types as targets. The fix causes additional checkcasts to be generated if the inferred types do not have the same erasure.

The fix breaks Mockito answer strategies that pick types based on the erased method signature's return type, and causes tests to fail with ClassCastExceptions when compiled with the JDK 9 javac.

---

Example 1

class Super<T> {
  T g() {
    return null;
  }
}

class Sub extends Super<Boolean> {}

@Mock Sub s;
when(s.g()).thenReturn(false);

compiled with javac 8

INVOKEVIRTUAL Sub.g ()Ljava/lang/Object;
INVOKESTATIC org/mockito/Mockito.when (Ljava/lang/Object;)Lorg/mockito/stubbing/OngoingStubbing;

compiled with javac 9

INVOKEVIRTUAL Sub.g ()Ljava/lang/Object;
CHECKCAST java/lang/Boolean
INVOKESTATIC org/mockito/Mockito.when (Ljava/lang/Object;)Lorg/mockito/stubbing/OngoingStubbing;

The erased return type of Super.g is Object, but the expected return type of Sub.g is Boolean. If the answer strategy returns Object the checkcast fails.

---

Example 2

class Foo {
  <T> T getFirst(Iterable<T> xs) { return xs.iterator().next(); }
}

@Mock Foo f;
Iterable<Boolean> it = Arrays.asList(false);
when(f.getFirst(it)).thenReturn(false)

compiled with javac 8

INVOKEVIRTUAL Foo.getFirst (Ljava/lang/Iterable;)Ljava/lang/Object;
INVOKESTATIC org/mockito/Mockito.when (Ljava/lang/Object;)Lorg/mockito/stubbing/OngoingStubbing;

compiled with javac 9

INVOKEVIRTUAL Foo.getFirst (Ljava/lang/Iterable;)Ljava/lang/Object;
CHECKCAST java/lang/Boolean
INVOKESTATIC org/mockito/Mockito.when (Ljava/lang/Object;)Lorg/mockito/stubbing/OngoingStubbing;

The erased return type of Foo.getFirst is Object, but the inferred return type of getFirst(Iterable<Boolean>) is Boolean. If the answer strategy returns Object the checkcast fails.

---

The first example could be fixed by using GenericMetadataSupport in all of the answer implementations instead of invocation.getMethod().getReturnType().

It gets more difficult if the mock's type is an instantiation of a generic type (e.g. @Mock Foo<Bar> x;), since the field's type arguments get dropped. I think fixing that would require adding support for mocking types, not just classes.

For the second example, returning the right answer requires considering the generic signature of the invoked method, and performing type inference using the argument types. Unfortunately the runtime type of the argument is going to be a raw Iterable and the inference depends on knowing it's Iterable<Boolean>, so I'm not sure what to do there.

[raphw]

This problem is also the cause for #304

[PascalSchumacher]

The test from #304 passes on current master d648014 so maybe this issue is also solved?

[raphw]

Closing this issue as it seems to be resolved.

[cushon]

I don't think this is fixed, both of the examples in the original report still fail with v2.0.78-beta and jdk 9-ea+116.

Full repro: https://gist.github.com/cushon/b5c5e3995e9ad68ed3df8fd8626daf09

java.lang.ClassCastException: codegen.java.lang.Object$MockitoMock$1914109206 (in module: Unnamed Module) cannot be cast to java.lang.Boolean (in module: java.base)

java.lang.ClassCastException: codegen.java.lang.Object$MockitoMock$1914109206 (in module: Unnamed Module) cannot be cast to java.lang.Boolean (in module: java.base)

[raphw]

Thanks, I check this out prior to release.

[cushon]

Sorry - do you mean that you verified it's fixed at head, or that you will take another look before the next release? If it's the second one, do you mind re-opening the issue to make sure it doesn't get lost? I'm seeing the same behaviour at head.

[bric3]

@cushon I think it's more that we'll look at it ;)

Typically using GenericMetadataSupport for @Mock fields has been on my mind for some time already. However note that using mock()style would not work unless we use some TypeReference trick.

It gets more difficult if the mock's type is an instantiation of a generic type (e.g. @mock Foo x;), since the field's type arguments get dropped. I think fixing that would require adding support for mocking types, not just classes.

@raphw I think we should not delay 2.0 for that one, JDK9 is not yet out. We can still release fixes later.

[raphw]

@cushon I have just completed testing Mockito against Java 9 and everything works as expected. The examples that you provide work without any problems.

Thinking about how this would work, also makes sense. If not configured to return a different value, Mockito returns null during the configuration what passes the checkcast. Only if a user specifies an illegal value to the generic signature (heap pollution), a problem occurs but this is not really Mockito's responsibility.

Did you test a recent version?

[cushon]

If not configured to return a different value, Mockito returns null during the configuration what passes the checkcast.

What about RETURNS_SMART_NULLS and RETURNS_DEEP_STUBS?

[raphw]

Ah, of course. Sorry for that, I did another mistake in my test. This problem is actually already reproduceable on recent versions of Java 8.

[raphw]

Ok, I have now finally looked into this in detail. There are three scenarios:

1. For generic return types, it suffices to resolve those return types properly by using GenericMetadataSupport. This can simply be added, I already tried this.
2. For locally inferred generic return types, there is not much we can do. For example, if we mocked some method public <T> T foo() { ... } there is no information preserved within the method that would allow us to know the expected type for T. We could only parse any method from where foo was called to contextually determine the return type. This would add a whole new layer of complexity to Mockito and I do not think we should support this case. We should probably add a warning to the docs.
3. For generic fields, we could find ways to preserve this information. We should probably refactor the RETURN_SMART_NULLS option to allow its binding to a generic base type rather than using the mock instance's (raw) type.

[TimvdLippe]

#639 builds on Java 9 which still has 3 test failures (but they seem to be related to AssertJ rather than our code). @cushon could you check out that branch and test if your usecases have been fixed with JDK9?

[cushon]

could you check out that branch and test if your usecases have been fixed with JDK9?

@TimvdLippe it has not been fixed, I turned the repro into a unit test in #664.

Those tests fail in the jdk9 branch:

org.mockito.internal.stubbing.defaultanswers.ReturnsSmartNullsTest > inferred_generic_type FAILED
    java.lang.ClassCastException: codegen.java.lang.Object$MockitoMock$1081761404 (in module: Unnamed Module) cannot be cast to java.lang.Boolean (in module: java.base)
        at org.mockito.internal.stubbing.defaultanswers.ReturnsSmartNullsTest.inferred_generic_type(ReturnsSmartNullsTest.java:122)

org.mockito.internal.stubbing.defaultanswers.ReturnsSmartNullsTest > generic_return_type_erasure FAILED
    java.lang.ClassCastException: codegen.java.lang.Object$MockitoMock$1081761404 (in module: Unnamed Module) cannot be cast to java.lang.Boolean (in module: java.base)
        at org.mockito.internal.stubbing.defaultanswers.ReturnsSmartNullsTest.generic_return_type_erasure(ReturnsSmartNullsTest.java:105)

[cushon]

For generic return types, it suffices to resolve those return types properly by using GenericMetadataSupport. This can simply be added, I already tried this.

That sounds good to me. I skipped the original class Sub extends Super<Boolean> {} example in #664 because it's no longer broken by the JDK9 javac change; it hasn't worked since 7fabd84. Is it worth splitting this into a separate bug?

    @Test
    public void generic_return_type_checkcast() throws Throwable {
        class Super<T> {
            T get() {
                return null;
            }
        }

        class Sub extends Super<Boolean> {}

        Sub sub = Mockito.mock(Sub.class, Answers.RETURNS_SMART_NULLS);
        Mockito.when(sub.get()).thenReturn(true);
        assertTrue(sub.get());
    }

generic_return_type_checkcast FAILED
    java.lang.ClassCastException: codegen.java.lang.Object$MockitoMock$1200321394 cannot be cast to java.lang.Boolean
        at org.mockito.internal.stubbing.defaultanswers.ReturnsSmartNullsTest$1Sub$MockitoMock$412017289.get(Unknown Source)
        at org.mockito.internal.stubbing.defaultanswers.ReturnsSmartNullsTest$1Sub$MockitoMock$412017289.get(Unknown Source)
        at org.mockito.internal.stubbing.defaultanswers.ReturnsSmartNullsTest.generic_return_type_checkcast(ReturnsSmartNullsTest.java:94)

[bric3]

Careful as GenericMetadataSupport requires more CPU and memory.

[TimvdLippe]

We might be able to get around these issues with a solution like #643

[ePaul]

Note that this also fails with Eclipse's Java compiler (running under a Java 8 JRE) in Eclipse Neon (4.5), with the same ClassCastException.

[asolntsev]

@cushon @raphw @ePaul What is the status of this issue?
Java 9 has been released today!

I am getting the following error in all my tests that use Mockito:

java.lang.ClassCastException: codegen.java.lang.Object$MockitoMock$1574589819 cannot be cast to org.hibernate.Session

[raphw]

As said, there is not much we can do. The information of the required type lies outside of the class we instrument. We simply do not have this information available. I am afraid that this chained mocking is something we cannot fully support and you will have to mock your method chanes manually to supply this information.

[asolntsev]

@raphw Thank you!

Well, does it mean that mockito will never work with Java 9? Omg, it's an epic disaster... :(

[raphw]

Only when returning automatic mocks from mocks with generic return types, maybe we should add a warning for this to make the error more intuitive.

[asolntsev]

@raphw Yes, the warning would be very helpful.
So, do I correctly understand that people will need to rewrite their tests in order to upgrade to Java 9? Probably a lot of tests?

[cushon]

There's an Error Prone check that detects this problem at compile-time: http://errorprone.info/bugpattern/MockitoCast. It can also be run as a refactoring to add work-arounds to affected code.