Bug: Matching of generic target types no longer works for provider methods in interface default methods
Closed this issue · 14 comments
Testing Problem
In jqwik 1.7 it was possible to have a structure like this:
public interface SimpleTest<T> {
@Property
default void simpleTest(@ForAll("datapoints") T datapoint) {
}
@Provide("datapoints")
Arbitrary<T> datapoints();
}Then have an extension for this, in which it is more natural to not refer to the provider any more as 'datapoints'.
interface ATest<X extends A> extends SimpleTest<X> {
@Provide("as")
Arbitrary<X> as();
@Override
@Provide("datapoints")
default Arbitrary<X> datapoints() {
return as();
}
@Property
default void aProperty(@ForAll("as") X a) {
}
}And that have an actual test class
public class BTest implements ATest<B> {
@Provide("as")
public Arbitrary<B> as() {
return Combinators.combine(
Arbitraries.integers().between(1, 100),
Arbitraries.strings().alpha().ofLength(5))
.as(B::new);
}
}This test then runned fine. It runs both 'simpleTest' as 'aPropertyTest' for all b's.
Since 'datapoints' was just implemented in ATest with a default implementation, in all extensions it was the same as 'as'.
In versions of jqwik > 1.7 it doesn't work any more and sais:
Cannot find an Arbitrary [datapoints] for Parameter of type [@net.jqwik.api.ForAll(value="datapoints", supplier=net.jqwik.api.ArbitrarySupplier$NONE.class) B]Suggested Solution
I think the original behaviour was sensible, and this regression was probably not intentional.
In general this should still be possible. What has changed is that the constraints applied to type variables are more strict now since they (try to) follow the Java compiler's rules for type substitutabiliy.
@mihxil I'll check your concrete example to find out what's happening here.
@mihxil Can you add a simple implementations for A and B (and probably X) so that your example compiles? I could do it myself, but I might get their relation to type X wrong, which could change the picture. My guess is that there's an unexpected effect from these type relations.
I figured out one possible problem: The variable datapoint in simpleTest is of generic type T which has no constraints, i.e. effectively it could be anything, but the compiler won't allow any concrete type to be assigned to datapoint.
Workaround: If you change the declaration to
@Property
default void simpleTest(@ForAll("datapoints") Object datapoint) {
}
it works for me.
Since T is unconstrained you can only use methods of Object anyway.
My case was of course a bit more complicated then this. I'm not convinced yet that it will do :-) But thanks for looking into it. I'll come back to you.
OK. I'll leave the issue open until you can come up with more information.
I looked a bit further into it. And it is clear that the issue has only to do but with the title already sais. Using 'default' Provides breaks things. Every property depending on a 'default' implementation needs to accept a more generic type then needed.
The most simple thing I could come up
public interface A {
}public interface ATheory<T extends A> {
@Property
default void toStringNoErrors(@ForAll("datapointsOrNull") Object object) {// Must be Object!
Assertions.assertThatNoException().isThrownBy(() -> {
System.out.println("" + object);
});
}
@Provide
Arbitrary<T> datapoints();
@Provide
default Arbitrary<@Nullable T> datapointsOrNull() {
return datapoints()
.injectNull(0.1);
}
}public class AImplTest implements ATheory<AImpl> {
@Override
public Arbitrary<AImpl> datapoints() {
return
Arbitraries.integers().between(1, 100).map((i) -> new AImpl());
}
}public interface B extends A {
int getJ();
default int plus(B o) {
return o == null ? getJ() : o.getJ() + getJ();
}
}And here it gets ugly:
public interface BTheory<T extends B> extends ATheory<T> {
@Property
default void jNotNegative(@ForAll("datapoints") T b) {
assertThat(b.getJ()).isNotNegative();
}
@Property
default void plusTest(@ForAll("datapoints") T b1, @ForAll("datapointsOrNull") Object o2) {
T b2 = (T) o2; // ugly
assertThatNoException().isThrownBy(() -> {
b1.plus(b2);
});
}
}with
public class BImpl extends AImpl implements B {
private final int j;
public BImpl(int j) {
this.j = j;
}
@Override
public int getJ() {
return j;
}
}
public class BImplTest implements BTheory<B> {
@Override
@Provide("datapoints")
public Arbitrary<B> datapoints() {
return
Arbitraries.integers().between(1, 100).map(BImpl::new);
}
}
So, I used default method to have also a provider that has nulls sometimes. But if I make something up for that, in this case just test if the 'plus' methods at least doesn't throw, and also accept an occasional null, it gets ugly. It was not needed in 1.7.2, I could just have then
default void plusTest(@ForAll("datapoints") T b1, @ForAll("datapointsOrNull") T o2) {No need for casting.
Yes, I can work around it by casting, and in such a simple example it is not so bad or course, but in actual classes the whole thing may get littered with casts.
It may be that the whole issue is that I don't understand generics well enough, it's always a bit hard. It may also be that what I'm trying here is a bit silly anyway for some reason.
But I really don't see why it should be like this. The 'datapointsOrNull' method is returning the correct type. And it did work earlier :-)
My personal check for the question is it a generics problem or a bug: Can a variable of type Arbitrary<TargetType> be assigned the return value of the provider method. So in your case: Does the following code compile
interface BTheory<T extends B> extends ATheory<T> ...
default void plusTestNotUgly(@ForAll("datapoints") T b1, @ForAll("datapointsOrNull") T b2) {
Arbitrary<T> assigned = datapointsOrNull(); // Does this compile?
}The answer is yes, which makes this a bug as far as I currently understand it. The fact that it works for non-default methods but not default methods suggests the same.
@mihxil Thank you for catching and reporting this. I hope you can live with the workaround for the time being. I put the bug as first thing for 1.9.2 but cannot make any promises about when I'm going to work on it.
I've dived a bit deeper into the generic type matching logic. Short story: It's very complicated :-/
That means that I cannot fix it easily.
One thing you could do - besides the cast - is to override the provider method just calling super:
interface BTheory<T extends B> extends ATheory<T> ...
@Property
default void plusTestNotUgly(@ForAll("datapoints") T b1, @ForAll("datapointsOrNull") T b2) {
assertThatNoException().isThrownBy(() -> {
b1.plus(b2);
});
}
@Override
default Arbitrary<@Nullable T> datapointsOrNull() {
return ATheory.super.datapointsOrNull();
}It's more code but less ugly. YMMV.
Another option is to completely release the type constraints for provider methods and only require them to return some kind of arbitrary. This would lead to generation time errors if providers create non fitting instances - instead of property resolution errors.
Another option is to completely release the type constraints for provider methods and only require them to return some kind of arbitrary. This would lead to generation time errors if providers create non fitting instances - instead of property resolution errors.
The more I think about that it seems to be the most pragmatic choice. And we had that already in earlier versions...
Another option is to completely release the type constraints for provider methods and only require them to return some kind of arbitrary. This would lead to generation time errors if providers create non fitting instances - instead of property resolution errors.
The more I think about that it seems to be the most pragmatic choice. And we had that already in earlier versions...
I'm going forward with this idea.
Released in 1.9.2
thanks, sorry for not reacting sooner. I'll let you know if needed.