Introduce additional tests for support of generics

[sbrannen]

Overview

Issues #333, #976, and #978 addressed the topics of generics and overridden methods in various ways; however, as conjectured in #978, we may still have lingering bugs in this regard.

Dedicated test class

ReflectionUtilsWithGenericTypeHierarchiesTests contains a set of interesting and some unclear cases.

Deliverables

For each of the following, introduce additional tests across multiple-level class hierarchies using generic interfaces, generic abstract classes, etc.

In other words, try to break the search algorithms.

• [ ] ReflectionUtils.findMethod(*)
• [ ] ReflectionUtils.findMethods(*)

[sbrannen]

This issue is up for grabs in case anyone in the community wants to take a stab at breaking our reflection utilities. 😉

PRs with failing tests are extremely welcome!

[SeriyBg]

@sbrannen, I cannot build the project with gradle.

A problem occurred evaluating root project 'junit5'.
> Could not find method runJunit4() for arguments [true] on object of type org.junit.gen5.gradle.JUnitExtension.

Am I missing something or doing something wrong?

[sbrannen]

You're using the alpha version of JUnit 5.

We just released 5.0 M6, and 5.0 RC1 work is being performed in master.

[sbrannen]

If you perform a fresh git clone https://github.com/junit-team/junit5.git, everything should build just fine.

[cloud-princess]

I'd like to take a stab if that's okay!

[sbrannen]

@cldssty, sure.... go for it!

[jadeleeuw]

I found a situation that breaks the search algorithm, given that my expectation of its behaviour is correct, so forgive my ignorance if this works as intended.

Given the two interfaces below:

interface A {
    default void foo(Double number) {
        System.out.println("A");
    }
}

interface B<T extends Number> {
    default void foo(T number) {
        System.out.println("B");
    }
}

And a class:

class Bar implements B<Integer>, A {}

findMethod(Bar.class, "foo", Double.class) will return the foo method of interface B. This is because that method is the first method found by the search algorithm that satisfies the signature, even though a more "specific"/non-generic method satisfying that signature is also available.

Turning the interfaces around (e.g. class Bar implements A, B<Integer> {}) does result in the foo from interface A being returned by findMethod.

I can imagine that a user in this situation might expect the most "specific"/non-generic method to be returned instead of the first method satisfying the signature, especially because invoking the foo method with a double will of course execute the method of interface A.

In essence: I am unsure whether the current behaviour of the search algorithm is working as you intended, or if this can be considered a bug. I can push a PR with a test case demonstrating the situation if needed.

[sbrannen]

@jadeleeuw, thanks for investigating and reporting that behavior!

I wouldn't say it's necessarily intentional but rather a "previously unknown limitation" of the algorithm.

Please feel free to submit a PR that reproduces the behavior.

We will then decide if we want to document it as a "known limitation" or consider a "bug" that needs to be fixed.

[sbrannen]

I wouldn't say it's necessarily intentional but rather a "previously unknown limitation" of the algorithm.

And when I say "previously unknown limitation", I opened this GitHub issue exactly because I assumed there were such use cases. 😉

[sbrannen]

@jadeleeuw, out of curiosity, how did you notice this behavior?

[jadeleeuw]

I must admit that I personally do not have a specific use case, but I was looking through the issues to see if there was a way that I could contribute to the project when I came across this issue, and toyed around with it to see if I could find a case where the result of the search algorithm deviated from my expectations.

[sbrannen]

OK. Thanks for the explanation. And by the way, good catch!

[aepedraza]

@sbrannen I'd like to contribute the project and found this up-for-grabs issue. What remains to be done after @jadeleeuw 's contribution? In other words: in which direction should we point further research?

Thanks in advance for your time

[PayalSainathMehta]

I'd like to work on this, how do i get started?

[neilmce]

@sbrannen I've opened a PR for your consideration. I believe it fixes one of the disabled tests in this issue.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

[jbduncan]

This might still be a useful issue to keep open?

[Michael1993]

I had a question about two of the disabled tests:

Should findMethod(A.class, "foo", Long.class) find A::foo(Number)? The current implementation suggests that, no, it should not as it is looking for a specific method that is either <T extends Number> foo(T) or explicitly foo(Long). Is this the intended behaviour?

If yes, these tests:

• ReflectionUtilsWithGenericTypeHierarchiesTests::findMoreSpecificMethodFromOverriddenImplementationOfGenericInterfaceMethod
• ReflectionUtilsWithGenericTypeHierarchiesTests::findMoreSpecificMethodFromImplementationOverDefaultInterfaceMethodAndGenericClassExtension

cover cases that actually behave as expected - finding the generic that satisfies the predicate over the overriden method that does not (not generic or exactly Long).

I'd also think this might warrant a javadoc update on findMethod? But this is a rather specific edge case.