Conversion of type parameters when creating executable references

[dotSp0T]

Describe the bug

Updating an older project from spoon 5.5.x to latest version. Getting exception I have issues understanding the problem with...

Stacktrace:

Exception in thread "main" spoon.SpoonException: CtExecutableReference cannot use CtTypeParameterReference. Use boundingType of CtTypeParameterReference instead.
	at spoon.support.reflect.reference.CtExecutableReferenceImpl.checkMethodParameterTypeRef(CtExecutableReferenceImpl.java:184)
	at spoon.support.reflect.reference.CtExecutableReferenceImpl.addParameter(CtExecutableReferenceImpl.java:176)
	at spoon.support.reflect.reference.CtExecutableReferenceImpl.setParameters(CtExecutableReferenceImpl.java:167)
	at spoon.reflect.factory.CodeFactory.createConstructorCall(CodeFactory.java:192)
	at spoon.reflect.factory.FactoryImpl.createConstructorCall(FactoryImpl.java:479)
	at org.crumbleworks.forge.spoon_openglparser.LwjglOpenGLAPIProcessor.createCustomPointerBufferAndWrapper(LwjglOpenGLAPIProcessor.java:1312)
	at org.crumbleworks.forge.spoon_openglparser.LwjglOpenGLAPIProcessor.process(LwjglOpenGLAPIProcessor.java:772)
	at spoon.support.QueueProcessingManager.process(QueueProcessingManager.java:115)
	at spoon.support.QueueProcessingManager.process(QueueProcessingManager.java:132)
	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.process(JDTBasedSpoonCompiler.java:194)
	at spoon.Launcher.process(Launcher.java:791)
	at org.crumbleworks.forge.spoon_openglparser.LwjglOpenGLAPIProcessor.main(LwjglOpenGLAPIProcessor.java:1478)

Code:

CtClass<?> lwjglPB = getFactory().Class().get(PointerBuffer.class);
        
List<CtMethod<?>> pbMethods = new ArrayList<>();
for(CtMethod<?> method : lwjglPB.getAllMethods()) {
    //some code filtering out methods we want/don't want

    pbMethods.add(method);
    }
}


//populate interface & wrapper
for(CtMethod<?> ctMethod : pbMethods) {
    CtMethod<Object> intfMethod = (CtMethod<Object>)ctMethod.clone();

    //creating a wrapper method and doing some stuff for the wrapper
    .
    .
    .
    
    //add to interface
    cwAPIPointerBuffer.addMethod(intfMethod);
    
    //add to wrapper
    CtMethod<Object> wrapMethod = (CtMethod<Object>)intfMethod.clone();
    wrapMethod.setComments(null);
    wrapMethod.addAnnotation(getFactory().Code().createAnnotation(getFactory().Annotation().createReference(Override.class)));
    
    List<CtExpression<?>> arguments = new LinkedList<CtExpression<?>>();
    for(CtParameter<?> param : ctMethod.getParameters()) {

    .
    .
    .

    @SuppressWarnings("unchecked")
    CtInvocation<Object> methodCall = (CtInvocation<Object>)getFactory().Code().createInvocation(
            getFactory().createVariableRead(internalPointerBuffer.getReference(), false),
            ctMethod.getReference(),
            arguments);
    
    //some more logic happening here, but the following is the only part that interacts with the methodCall object
    //AND is the part that throws, the line that throws is marked with '->'
    wrapMethod.setBody(getFactory().Core().createReturn().setReturnedExpression(
      getFactory().createConstructorCall(
              cwEnginePointerBufferWrapper.getReference(),
->          methodCall)
      ));
    
    cwEnginePointerBufferWrapper.addMethod(wrapMethod);
}

I cannot provide the full sourcecode as of this moment, as there's a bunch of proprietory stuff in there. I am trying to narrow it down to a self-contained executable sample

Source code you are trying to analyze/transform

No response

Source code for your Spoon processing

No response

Actual output

No response

Expected output

No response

Spoon Version

10.1.1

JVM Version

18

What operating system are you using?

Windows 11

[dotSp0T]

The bug seems to have been triggered by some workspace issues after the upgrade from 5.5.x

Still having issues but these seem to be part of my own programlogic so far :)

[dotSp0T]

After resolving the other issues i had (which had to do with how spoon builds its model) the bug is back :)

[I-Al-Istannen]

Have you tried following what the error message tells you to do (though it is a tad cryptic :P)? I.e. probably passing in

((CtTypeParameterReference) cwEnginePointerBufferWrapper.getReference()).getBoundingType()

in your case?

Some justification for this change was apparently provided in #2401 (comment).

[dotSp0T]

That looks interesting, but the error is thrown from the ctMethod.getReference() further down -> which is passed into the new methodCall as a parameter/argument.

But, I am currently following up on that lead - together with your helpful translation of the exception ;)

[dotSp0T]

Updated the codesample with the, presumably, affected method-wrapping code.

The codebase is designed to wrap a couple of classes/interfaces of another library into a single class that delegates calls to the individual library classes/interfaces/methods. The produced class is then used as an interface/proxy for the library.

[I-Al-Istannen]

Yea, the new example code in the offpost is sadly a bit too broken for me to repair and have a look at.

Sounds like you are building a Facade. Interesting. I also see that you are cleaning javadoc, maybe I can interest you in #4748 :P

[dotSp0T]

Broke it down quite a bit again, maybe you're able to see the issue on a glance now? The ctMethod object is retrieved as is from spoon here lwjglPB.getAllMethods() and further on never modified, only read...

Your plugin looks veeeery useful, I might dabble into using that somewhere down the line (whenever I get to further clean up this codebase and abstract it so I can make it opensource).

[I-Al-Istannen]

Okay, I was able to recreate the issue and it is a bit ugly. So prepare for a slight wall-of-text.

The problem is that the return type of your method call (the CtInvocation you build) is a type parameter (SELF). This is a consequence of how getAllMethods deals with inherited methods that are never overridden: It returns the executable declared in the respective supertype without adapting it to the base class. Therefore, you receive the public SELF put(SELF) method defined in the CustomBuffer superclass instead of the adapted public PointerBuffer put(PointerBuffer) method that actually is used here.

You can solve this by manually adapting the methods to the PointerBuffer class before building the invocations:

ctMethod = new TypeAdaptor(lwjglPB).adaptMethod(ctMethod);    // <<-- here
for (CtParameter<?> param : ctMethod.getParameters()) {
    CtInvocation<Object> methodCall = (CtInvocation<Object>) factory.Code().createInvocation

This will take care of this specific issue. What it does not solve, however, is methods returning a type parameter that is still symbolic at the time of use. This happens when your class does not concretize a type parameter (SELF here is assigned a concrete type, but E in lists for example is not) or your method defines its own type parameter (e.g. public <T> T void()). In these cases you still have a type parameter as return value.

The type-correct thing to do would be inferring the types and using them in the invocation, but I think that problem is impossible to solve at this point in time (literally impossible, as you are only building up the context needed for type inference). The next best thing to do is just taking the bound of the type variable (or its erasure). This should be fine, as it does not impact printing and you don't typically do much more with a transformed model.

To do this, I would probably just hack it in before you create the constructor call:

CtInvocation<Object> methodCall = (CtInvocation<Object>) factory.Code().createInvocation(
        factory.createVariableRead(ctField.getReference(), false),
        ctMethod.getReference(),
        arguments
);
if (methodCall.getType() instanceof CtTypeParameterReference) {
    methodCall.setType((CtTypeReference<Object>) ((CtTypeParameterReference) methodCall.getType()).getBoundingType());
}

You could also just indiscriminately set the return type to the erasure of the current return type, which is even less correct but likely still good enough.

This is a bit of an unfortunate consequence of the way executable references deal with type parameters. Based on the little time I spent on this problem it might be alright to omit the SpoonException and silently adjust the type to its bound when building the reference. This means the model will silently degrade, but I am not sure if letting the user perform type inference manually is a better solution.

[dotSp0T]

That is a beautiful wall of text. Thanks for that write-up, helps a lot to understand the issue here.

Should I close this or keep it open and transform it into a feature-request?

[I-Al-Istannen]

That's a good question actually ;) I am not quite sure. To comply with the spirit of the check (as I understand it), the parameter would need to be adapted to the used subclass and, if it is still a type parameter, its upper bound / erasure taken. The first part is context-specific and can not be done automatically.

I'd be interested in some more opinions on how spoon should handle this. My comment above is hopefully enough for context for anybody else weighing in. If we leave the current behavior, we should at least improve the error message to be a bit more actionable.

I think I will change this issue to a discussion as it is not really a bug — the feature works as intended but we aren't sure if it is the most friendly solution.

If things still don't work for you with the erasure/bounded-type workaround, that would be interesting as well.

EDIT: I can't edit the issue title, but maybe something like Conversion of type parameters when creating executable references might be more accurate in the current state?