Domains inject parameterized values incorrectly

[SimY4]

Testing Problem

The setup like this:

@Domain(MyDomain.class)
@Domain(DomainContext.Global.class)
class TestTest {
  @Property
  boolean test(@ForAll Predicate<Number> p1, @ForAll Predicate<Number> p2, @ForAll Number n) {
    return p1.or(p2).test(n) == (p1.test(n) || p2.test(n));
  }
}

class MyThing implements Predicate<CharSequence> {
  public boolean test(CharSequence cs) { return true; }
}

class MyDomain extends DomainContextBase {
  @Provide
  Arbitrary<MyThing> myThingArbitrary() {
    return Arbitraries.just(new MyThing());
  }
}

will produce:

  java.lang.ClassCastException:
    class java.lang.Long cannot be cast to class java.lang.CharSequence (java.lang.Long and java.lang.CharSequence are in module java.base of loader 'bootstrap')

but if you remove domain and provide MyThing right in test class - no problem.

[jlink]

@SimY4 Interesting catch. Will look into it.

[jlink]

OK. It's a bug, but sadly one I've tried to fix a couple of times already without seeing a straightforward solution.

See https://github.com/jqwik-team/jqwik/blob/5055eb3f432e30b416f22a05cebaf7c56a0e3a33/engine/src/main/java/net/jqwik/engine/support/types/TypeUsageImpl.java#L410C1-L417C1

Maybe I'll find the time to reinvestigate. The problem is that type matching is sometimes too loose. And it's very hard to fix this without making other cases fail.

In your case the following change would fix it, but you probably know that already:

class MyDomain extends DomainContextBase {
	@Provide
	Arbitrary<Predicate<CharSequence>> myThingArbitrary() {
		return Arbitraries.just(new MyThing());
	}
}

[SimY4]

@jlink yeah, I figured. Not sure why it works for provided in test though. Is it just by chance? I get that it's probably related to MyThing not having any explicit type arguments. Should it find and compare only matching interfaces between requirements and provided definitions? I.e. I need Predicate I have MyThing which has Predicate but Number is not assignable from CharSeq so no.

[jlink]

@jlink yeah, I figured. Not sure why it works for provided in test though. Is it just by chance?

I assume it's due to search order, which is in parts coincidental :-/

I get that it's probably related to MyThing not having any explicit type arguments. Should it find and compare only
matching interfaces between requirements and provided definitions? I.e. I need Predicate I have MyThing which has
Predicate but Number is not assignable from CharSeq so no.

I don't really understand what you mean, but if you'd use an explicity ArbitraryProvider implementation in your domain, which is possible by just having an inner class, with an explicit implementation of canProvideFor(TypeUsage targetType) MyThing should not be injected in your test property. Haven't tried, though; just dry-running the code in my head.

[vlsi]

The problem is that type matching is sometimes too loose. And it's very hard to fix this without making other cases fail.

I guess the reason is that you probably want the following to work:

  @Property
  boolean test(@ForAll Predicate<Number> p1, @ForAll Predicate<Number> p2, @ForAll Number n) {
    return p1.or(p2).test(n) == (p1.test(n) || p2.test(n));
  }

  @Provide
  Arbitrary<Predicate<Double>> myThingArbitrary() {
    return ...;
  }

Technically speaking Predicate<Double> is not a subtype of Predicate<Number>, so in theory, jqwik should not wire the two. It should rather fail with error saying "Predicate<Number> is not provided".

The proper Java code would be

  @Property
  boolean test(@ForAll Predicate<? super Number> p1, @ForAll Predicate<? super Number> p2, @ForAll Number n) {
    return p1.or(p2).test(n) == (p1.test(n) || p2.test(n));
  }

  @Provide
  Arbitrary<Predicate<Double>> myThingArbitrary() {
    return ...;
  }

In that case, Predicate<Double> is a subtype of Predicate<? super Number>, so jqwik should infer provide-consume link.

Of course, you might argue that it would break existing tests that relied on somewhat working jqwik's loose type comparison. However, I would argue, well, they have chosen Java language, so they should follow its verbose use-site-type-variance, and they must declare type variance on the use site according to the rules of Java language.

See jspecify/jspecify#72

---

Unfortunately, Java does not have declaration-site type variance, so the workarounds could be:
a) Use Kotlin, see #250 (comment). With Kotlin you declare variance when declaring an interface or class, and it automatically puts ? extends and ? super when you use types in the code
b) Hard-code "default variance for well-known classes like Predicate, Function, Consumer, Supplier, Arbitrary". For instance, it is known that java.util.function.Predicate<T> is de-facto a java.util.function.Predicate<in T>, so jqwik's type comparison could silently treat java.util.function.Predicate<Number> as if it was java.util.function.Predicate<? super Number> when comparing the types.
c) Ask users to follow Java rules, and spell out @ForAll Predicate<? super Number> when they want a predicate with Number as an input

WDYT?

[SimY4]

@vlsi But in this case I don't see how variance is relevant since Number and CharSequence has no intersection which should be detectable: Number and CharSequence has no relation.

I get an impression that the error happens because TypeRef can't examine generic parameters. For example this works fine:

class MyThing<T> implements Predicate<T> {
  @Override
  public boolean test(T cs) {
    return true;
  }
}

class MyDomain extends DomainContextBase {
  @Provide
  Arbitrary<MyThing<CharSequence>> myThingArbitrary() {
    return Arbitraries.just(new MyThing());
  }
}

So the solution here is to look for the matching generic interface in type hierarchy and compare their generic parameters. In this example:

Predicate[Number] =?= (MyThing <: Predicate[CharSequence])

Predicate[Number] =?= Predicate[CharSequence]

Number =!= CharSequence

So there should be a pre-step before comparing generic parameters that will look for the right generic interface in arbitrary type hierarchy.

[jlink]

@vlsi @SimY4 You‘re both right. Jqwik doesn’t handle variance perfectly, but the problem here is more straightforward and should be fixable as described by @SimY4 above.

[vlsi]

@jlink , I suggest the following:

if (targetType.getRawType().isAssignableFrom(rawType)) {
	// TODO: this is too loose, e.g. DefaultStringArbitrary can be assigned to Arbitrary<Integer>
	// In order to solve that nested type arguments of this and targetType must be considered
	if (allTypeArgumentsCanBeAssigned(this.getTypeArguments(), targetType.getTypeArguments())) {
		return true;
	} 

I suggest the following:

1. Resolve "target type's" parameters based on the required supertype. I think it can be implemented with https://github.com/harawata/typeparameterresolver/blob/ba7997dff74f454a09d31efc11cee7a02a7e6f8c/src/main/java/net/harawata/reflection/TypeParameterResolver.java#L82-L94

2. Then execute the comparison logic

I believe it would handle all the cases above, however, it would requite harawata/typeparameterresolver dependency

WDYT?

[vlsi]

#492 might be relevant as well

[vlsi]

Here's a sample test case for TypeUsageTests

	@Example
	void isAssignableParameterized() throws NoSuchFieldException, NoSuchMethodException {
		class LocalClass {
			public void test(
				Predicate<Double> predicateDouble,
				Predicate<Number> predicateNumber,
				Predicate<? super Number> predicateSuperNumber,
				Predicate<? extends Number> predicateExtendsNumber
			) {
			}
		}
		Method method = LocalClass.class.getMethod("test", Predicate.class, Predicate.class, Predicate.class, Predicate.class);
		List<MethodParameter> parameters = JqwikReflectionSupport.getMethodParameters(method, LocalClass.class);
		TypeUsage predicateDouble = TypeUsageImpl.forParameter(parameters.get(0));
		TypeUsage predicateNumber = TypeUsageImpl.forParameter(parameters.get(1));
		TypeUsage predicateSuperNumber = TypeUsageImpl.forParameter(parameters.get(2));
		TypeUsage predicateExtendsNumber = TypeUsageImpl.forParameter(parameters.get(3));
		assertThat(predicateDouble.canBeAssignedTo(predicateNumber)).isFalse();
		assertThat(predicateNumber.canBeAssignedTo(predicateDouble)).isFalse();
		assertThat(predicateDouble.canBeAssignedTo(predicateSuperNumber)).isTrue();
		assertThat(predicateNumber.canBeAssignedTo(predicateSuperNumber)).isTrue();
		assertThat(predicateNumber.canBeAssignedTo(predicateExtendsNumber)).isFalse();
		assertThat(predicateDouble.canBeAssignedTo(predicateExtendsNumber)).isFalse();
	}

[jlink]

Here's a sample test case for TypeUsageTests

IMO (and the opinion of the compiler) the assertions are wrong:

assertThat(predicateDouble.canBeAssignedTo(predicateSuperNumber)).isTrue();
assertThat(predicateNumber.canBeAssignedTo(predicateSuperNumber)).isTrue();
assertThat(predicateNumber.canBeAssignedTo(predicateExtendsNumber)).isFalse();
assertThat(predicateDouble.canBeAssignedTo(predicateExtendsNumber)).isFalse();

should be

assertThat(predicateDouble.canBeAssignedTo(predicateSuperNumber)).isFalse();
assertThat(predicateNumber.canBeAssignedTo(predicateSuperNumber)).isTrue();
assertThat(predicateNumber.canBeAssignedTo(predicateExtendsNumber)).isTrue();
assertThat(predicateDouble.canBeAssignedTo(predicateExtendsNumber)).isTrue();

[vlsi]

there should be predicateSuperNumber.canBeAssignedTo(predicateDouble).isTrue as well

[SimY4]

Just one other thought and another reason to find and compare matching generic interfaces is:

class MyThing<T> implements Predicate<CharSequence> { ... }

In here MyThing is assignable to Predicate but the generic param has nothing to do with the Predicate type argument.

[vlsi]

@SimY4 , please read "1." in #499 (comment)

It seems you missed it

[jlink]

predicateSuperNumber.canBeAssignedTo(predicateDouble).isTrue

Does not look like that was the case:

	void test(
		Predicate<Double> predicateDouble,
		Predicate<Number> predicateNumber,
		Predicate<? super Number> predicateSuperNumber,
		Predicate<? extends Number> predicateExtendsNumber
	) {
		predicateSuperNumber = predicateNumber;
		// predicateDouble = predicateSuperNumber; // does not compile
		predicateExtendsNumber = predicateNumber;
		predicateExtendsNumber = predicateDouble;
}

[jlink]

I'm working on it on branch issue-449

Variance and type parameter matching seems fine so far, but I introduced a problem with recursive types on the way :-/

[SimY4]

@jlink for the purposes of PBT framework do you need to solve this for such generic case? I feel like you can't possibly expect to summon instances of not fully resolved types. And if they are resolved then T should be concrete. Essentially you can give up exploring the hierarchy branch when you get the parameterised type. You can't compare T and U even if they have common supertype. Wildcards are ok though.

[jlink]

It's not about solving all cases, but the following property - and others like it - worked before:

@Property
<T extends Comparable<T>> boolean constrainedTypeVariable(@ForAll T aValue) {
	return aValue != null;
}

Now it fails with a SOF. That's not acceptable.

[jlink]

Provided a fix and released it as "1.7.5-SNAPSHOT".

@SimY4 Maybe you can try if your use case now works.

[SimY4]

@jlink I ran my test suite using snapshot release - all seem to be working fine here.

[jlink]

Since a change to the behaviour of canBeAssignedTo(..) is relevant for a lot of hooks and domain implementations, I'll move the current version to 1.8.0-SNAPSHOT.

[vlsi]

Here's one more example which fails with bf4577b

	@Example
	void canBeAssignedToParametereized() throws NoSuchFieldException, NoSuchMethodException {
		abstract class StrFunction<T extends Number> implements Function<CharSequence, T> {
		};
		class LocalClass {
			public void test(
				Function<? extends CharSequence, Integer> functionExtendsCsInteger,
				Function<? extends CharSequence, Number> functionExtendsCsNumber,
				StrFunction<Number> customNumber,
				StrFunction<Integer> customInteger
			) {
				// Compilation fails if uncomment
				// functionExtendsCsInteger = customNumber;
				functionExtendsCsNumber = customNumber;
				functionExtendsCsInteger = customInteger;
				// functionExtendsCsNumber = customInteger;
			}
		}
		Method method = LocalClass.class.getMethod("test", Function.class, Function.class, StrFunction.class, StrFunction.class);
		List<MethodParameter> parameters = JqwikReflectionSupport.getMethodParameters(method, LocalClass.class);
		TypeUsage functionExtendsCsInteger = TypeUsageImpl.forParameter(parameters.get(0));
		TypeUsage functionExtendsCsNumber = TypeUsageImpl.forParameter(parameters.get(1));
		TypeUsage customNumber = TypeUsageImpl.forParameter(parameters.get(2));
		TypeUsage customInteger = TypeUsageImpl.forParameter(parameters.get(3));

		assertThat(customNumber.canBeAssignedTo(functionExtendsCsInteger)).isFalse();
		// FAILS in bf4577bc356bcdb583a9adc7c953254e06b1878f
		assertThat(customNumber.canBeAssignedTo(functionExtendsCsNumber)).isTrue();
		// FAILS in bf4577bc356bcdb583a9adc7c953254e06b1878f
		assertThat(customInteger.canBeAssignedTo(functionExtendsCsInteger)).isTrue();
		assertThat(customInteger.canBeAssignedTo(functionExtendsCsNumber)).isFalse();
	}

[jlink]

Here's one more example which fails with bf4577b

I'll see if that can easily be remedied. I assume not. The fix is an improvement but not a cure :-/

[jlink]

Working on a solution in branch https://github.com/jqwik-team/jqwik/tree/issue499

[jlink]

pull request to solve most/all of the variance and assignment issues

Also available as 1.8.0-SNAPSHOT