Handling of receiver parameters

[SirYwell]

This issue covers the general handling of receiver parameters in spoon.

Receiver parameters were introduced with Java 8. They are specified in JLS 8.4 with example code in JLS 8.4.1.

Spoon got support for this feature with this Pull request (related issue).
Currently, spoon handles receiver parameters in the same way other parameters are handled too. This brings us to the first issue:
When creating a CtExecutableReference (using CtExecutable#getReference() or ExecutableFactory#createReference(CtExecutable<?>)), the receiver parameter is included. For me, this is unexpected or even wrong, and a call to CtExecutableReference#getActualMethod() will always return null.

Having the receiver parameter in the parameter list also makes the creation of invocations harder, as you manually need to ignore receiver parameters on your own.

A second issue becomes visible when looking at the example code in the JLS: For now, spoon only handles methods. A receiver parameter in a constructor of an inner class (referencing the enclosing class) will simply be ignored and does not appear in the model created by spoon.

Adding support for constructors in the same way as with methods should be pretty simple, but I'd like to discuss alternatives of how to represent receiver parameters in the metamodel altogether.

I thought about several solutions with different pros and cons:

1) Add a isReceiver(): boolean method to CtParameter

Pros:

• Backwards compatible
• Allows to check if a parameter is a receiver parameter without the need to check CtParameter#getSimpleName().equals("this")
• Can be implemented by either setting a boolean or just checking CtParameter#getSimpleName().equals("this") internally
• Works for both constructors and methods directly

Cons:

• Only one parameter (the first) can be a receiver parameter - we have very specific code in a less specific class
• Static methods and non-inner constructors do not allow receiver parameters - this is not modeled with this solution
• CtExecutableReference#getParameters() still returns a list of CtTypeReferences including the receiver parameters
  • Duplicating the isReceiver() method in CtTypeReference would increase code that does not really belong there
  • In best case, you should not need to check the CtParameters of the CtExecutable to see if a CtTypeReference is a receiver
• The receiver parameter is not part of the method signature but in the list of method parameters - this might be unexpected for any work with the API

2) Add a hasReceiver(): boolean method to CtExecutable

a) but keep the parameter in the parameter list

Pros:

• Backwards compatible
• Easy to implement, basically the same way as mentioned in 1) by checking the first parameter (if present) or a boolean
• Works for both constructors and methods directly
• The receiver parameter is unique in its name (besides qualified names like Outer.Inner.this) and position (0) and is represented in the model this way

Cons:

• Should be reflected in CtExecutableReference too
• The receiver parameter is still in the list of parameters

b) and remove it from the parameter list

Pros:

• Easy to implement
• Works for both constructors and methods directly
• Does not infer with the parameter list for invocations and references

Cons:

• Not backwards compatible if the current way is considered to be correct, although I'm not sure what to expect when reading the current documentation of CtExecutable#getParameters() or CtParameter

3) Add a CtReceiver

Do not add it to the parameter list but add get/setReceiver() methods. Add the respective properties to CtExecutable. I'm not sure if it should be part of CtExecutableReference with a CtReceiverReference or similar.

(super types of CtReceiver might be discussed)

Pros:

• Makes it easy to deal with other parameters
• Makes dealing with receiver parameters easy as well
• Allows to keep track of additional information (as e.g. qualified names)
• Does not infer with the parameter list for invocations and references
• Works for both constructors and methods directly

Cons:

• Not backwards compatible in the way described in 2b)

4) Only add support for constructors the same way as for methods

Pros:

• Easy to implement

Cons:

• None of the points I considered above are applied
• Handling receiver parameters is more difficult
• Handling constructors without thinking about receivers will be error prone (could even break existing code)

---

Some general questions:

• How to deal with a CtExecutable#addParameterAt(int, CtParameter) with 0 as position? (e.g. refactoring a method and its invocations by adding a parameter at 0 would fail currently)
• Would anyone expect receiver parameters in the parameter list? Would it makes sense to clarify the docs in such places?

Side note: When looking at the code, I was super confused because the JDT Receiver is never traversed by JDT itself - in contrast to the other arguments, annotations, type parameters, etc. I don't know if this is a bug in JDT, but calling the traverse method ourselves is fine I guess.

---

I'm happy to implement any of this, or modified variants depending on your opinions and ideas. Feedback is very welcome.

[andrewbwogi]

I think the ideal solutions here are not backward compatible. If we value compatibility the most, we can save the former in some organized way. When we redesign the general API, there may then be ideas that have matured over time to minimize hasty design decisions.

[slarse]

Sorry about the late response on this one, it slipped past during vacations.

the receiver parameter is included. For me, this is unexpected or even wrong

I agree that his is wrong, and would label it a bug that should be fixed.

As for a solution, I personally would like the receiver parameter to remain in the list of formal parameters. While Spoon originally was all about semantics, several projects I'm involved in (merging and automatic program repair) rely heavily on the Spoon AST remaining somewhat close to the actual syntactical structure of the source file. This is especially helpful when pretty printing, and even more so when sniper printing.

I think options 1) and 2a) (perhaps both, they aren't mutually exclusive) are the most promising. I dislike option 3) because every method and constructor has a receiver, it's just implicit in the vast majority of cases.

An additional option would be to implement option 1/2 with CtReceiver/CtReceiverReference that subtype CtParameter/CtParameterReference.

Ping @monperrus, thoughts?

[monperrus]

Thanks for analyzing this dark corner.

This brings us to the first issue: When creating a CtExecutableReference (using CtExecutable#getReference() or ExecutableFactory#createReference(CtExecutable<?>)), the receiver parameter is included.

That can be fixed in a PR. I don't expect many/any test case to fail.

A receiver parameter in a constructor of an inner class (referencing the enclosing class) will simply be ignored and does not appear in the model created by spoon.

That can be fixed in a separate PR.

1. Add a isReceiver(): boolean method to CtParameter

That's reasonable and easy to understand for clients. Definitely a good improvement.

[SirYwell]

I was looking into #4145 for a bit and came across

spoon/src/main/java/spoon/support/visitor/ClassTypingContext.java

Lines 660 to 663 in 9a45639

	if (thisExecutable.getParameters().size() != thatExecutable.getParameters().size()) {
	//the executables have different count of parameters they cannot have same signature
	return false;
	}

which is wrong if receiver parameters are in that list.

I'm not sure if there are more spots like that, but this is definitely an edge case that can cause major headache I think.

[SirYwell]

Closing this as we implemented #5674 in Spoon v11.