Support void class lookups in ReflectionUtils

[bjmi]

Running following parametrized test in JUnit 5.11

@ValueSource(strings = {
    "void", "boolean", "byte", "char", "short", "int", "long", "float", "double" })
@ParameterizedTest
void isPrimitive(Class<?> candidate) {
    assertTrue(candidate.isPrimitive());
}

only makes the "void" case fail.

org.junit.jupiter.api.extension.ParameterResolutionException:
    Error converting parameter at index 0: Failed to convert String "void" to type java.lang.Class

It looks like conversion of "void" to Class isn't supported.
Both classNameToTypeMap and primitiveToWrapperMap of org.junit.platform.commons.util.ReflectionUtils are lacking void.class and Void.class, but maybe this is by intention.

[marcphilipp]

Did you deliberately not use the classes attribute?

@ValueSource(classes = { void.class, ... })
@ParameterizedTest
void isPrimitive(Class<?> candidate) {
    assertTrue(candidate.isPrimitive());
}

That being said, I see no reason not to add support for "void"

[sbrannen]

Both classNameToTypeMap and primitiveToWrapperMap of org.junit.platform.commons.util.ReflectionUtils are lacking void.class and Void.class, but maybe this is by intention.

If my memory serves me correctly, it was indeed intentional since void and Void are not real types but rather pseudo-types.

And since we never had a concrete use case for supporting conversions for those in JUnit itself, we didn't introduce those mappings.

Though, I agree with @marcphilipp that it should be fine to add lookups for those in ReflectionUtils.

[bjmi]

Did you deliberately not use the classes attribute?

I quickly switched to classes when strings didn't work.

[sbrannen]

Technically speaking, "java.lang.Void" should have been converted to Class<Void> without the changes in #4049.

So, it looks like we actually made two changes.

1. Supporting class lookups by name for "void".
2. Supporting Void as a "wrapper" type.

Admittedly, that's a bit nuanced, but perhaps we should revise the release notes and documentation accordingly.

[sbrannen]

we should revise the release notes and documentation accordingly.

Reopening to address those concerns.

[sbrannen]

As a side note, I'm not so sure that the following should actually pass.

assertTrue(ReflectionUtils.isAssignableTo(Void.class, void.class));

Rationale: ReflectionUtils.isAssignableTo() is only supposed to be used for reflective method invocations, where the targetType (the second argument) represents a formally declared parameter type for a method or constructor. However, void is not valid as a method or constructor parameter type. Attempting to do that results in a compilation error.

In light of that, I think we should either:

1. remove the Void.class wrapper registration, or...
2. Modify the ReflectionUtils.isAssignableTo() methods so that false is always returned for a void targetType.

@marcphilipp, thoughts?

[sbrannen]

I updated the documentation in 57e20cd, but I'm leaving this open to address the following.

In light of that, I think we should either:

1. remove the `Void.class` wrapper registration, or...
2. Modify the `ReflectionUtils.isAssignableTo()` methods so that `false` is always returned for a `void` `targetType`.

[marcphilipp]

@sbrannen Good catch!

1. remove the Void.class wrapper registration

I'm in favor of removing the wrapper registration because, as you pointed out, the void type cannot be used for any kind of assignment.