review: fix: support type parameters on method references

[SirYwell]

Previously, generic parameters where neither present in the model nor printed. For example,

BiFunction<Integer[], Integer, Integer> field = Arrays::<Integer>binarySearch;

was printed as

BiFunction<Integer[], Integer, Integer> field = Integer::binarySearch;

[flacocobot]

flacoco flags 4 suspicious lines as the potential root cause of the test failure.

[SirYwell]

Wow... I think I accidentally stumbled upon a solution for #4291?

Guess I need to look into that to make the tests pass, but NewClassTest seems to make wrong assumptions regarding the types, e.g.

spoon/src/test/java/spoon/test/constructorcallnewclass/NewClassTest.java

Line 70 in 011be63

assertType(Object.class, newClass);

(it should check for a strict subtype of Object here I think)

Also the printer seems to print things wrong now, causing tests that depend on it to fail. I'll continue to look into it, but if you have any idea why that even happens now, please let me know (@slarse as you answered on the linked issue).

---

EDIT: I had to switch from CtTargetedExpression to a CtExecutableReferenceExpression in the instanceof check. Now, it does not assign the type to anonymous classes correctly anymore (as before), but it doesn't fail at all the different places.

[MartinWitt]

Looks pretty good, but I have added 2 style comments and can you add a testcase for something more complex like Foo::<Bar<T,U>>batz.

[slarse]

I'll continue to look into it, but if you have any idea why that even happens now, please let me know (@slarse as you answered on the linked issue).

@SirYwell Sorry, why what happens?

[SirYwell]

@SirYwell Sorry, why what happens?

In 578c269, I switched from CtTargetedExpression which I used first to the more concrete CtExecutableReferenceExpression.

With CtTargetedExpression there, the bug I wanted to fix was fixed, but it had unintended side effects. One of them was that CtNewClass returned the correct type (of the anonymous class). This was because the CtConstructorCall gets that type assigned.

My initial thought was that this could also help fixing #4291, but it also introduced unwelcomed side effects. I went the way switching to CtExecutableReferenceExpression, but maybe that's a good place to look for when figuring out a fix.

I was hoping someone else would more about how to track where that SingleTypeReference from JDT comes from to assign it to the correct target.

[dginelli]

Hello @SirYwell, I’m Davide Ginelli, a researcher in software engineering that is currently working with other researchers on a software bot for fault localization (Flacocobot).

As you may have seen, after you created the pull request #4343, Flacocobot automatically added a comment to the pull request proposing the most suspicious lines related to the failures that broke the CI.

We would like to ask you:

• Did you use the Flacocobot suggestions to understand the reasons for the failures?
• Did you find it useful?

Based on your availability and preferences, it would be great for us if you could write your opinion directly as response to this message or also have a short discussion (for example over Zoom or Meet) on Monday, 13th.

Thank you!

[MartinWitt]

Thanks @SirYwell

[slarse]

@SirYwell Sorry, why what happens?

I was hoping someone else would more about how to track where that SingleTypeReference from JDT comes from to assign it to the correct target.

I don't really have any good answers. As for SingeTypeReference, that's just the base type reference for anything that's not a union type (I think). So, basically the base type reference used everywhere but in certain catch clauses. As for where it comes from, it's created a little here and there inside of JDT I believe, although that's probably not much help to you :)

[SirYwell]

Hello @dginelli,

I can't really say that I used the Flacocobot here to understand the test failures. I just looked up the run but I didn't find the test case it mentioned as failed. I also checked out that commit again and ran the test locally and it didn't fail. Not sure if I'm doing something wrong, but I'm a little bit confused here.

I can give some more general feedback on it: The suspiciousness value seems to be pretty low in this case, but I don't know if that's only the case because the issue here was fairly complex as the tests were failing for somewhat indirect reasons. I think an explanation of that value would help to make use of it.

Another thing you might want to look into is the Github Checks API. I think it's suited well for reports like that, and it would help to keep the reviews clean.

I hope that's helpful for you :)