Arc - improve injection point transformation API

[manovotn]

As part of #38841, we discovered that the API for injection point transformation is fairly hard to use when you need to alter IPs which are method parameters. As of now, you have to be aware that you operate on an AnnotationTarget with kind METHOD instead of METHOD_PARAMETER and you need to alter annotations in a tedious way by supplying a different ann. target. That being said, it isn't broken per se, just hard to use properly.

This seems to have been intentional as older Jandex API (pre 3.0) didn't allow easier handling - but that's no longer the case.

We should do something along the lines of:

• Adding MethodParameterInfo into InjectionPointInfo
  • Probably deprecate #getTarget() method as it would be confusing and breaking to silently change its behavior
  • Introduce something like #getInjectedTarget() which will now return ann. target of method param where applicable
• Revisit InjectionPointsTransformer API
  • I am not sure how to handle this in a non-breaking manner - it now also returns method ann. target with its #getTarget() and we would like to change that. We might need to do the same thing as suggested above although that will end up being the only impl of TransformationContext which doesn't have getTarget method 🤷

[quarkus-bot]

/cc @Ladicek (arc), @mkouba (arc)

[Ladicek]

I would call the method getInjectionTarget(), not getInjectedTarget().

When it comes to the transformer API, there are more options. We could introduce some way through which a transformer implementation would signal that they want getTarget() to return MethodParameterInfo instead of MethodInfo, such as:

• an annotation on the implementation class
• a boolean-returning method that the implementation could override
• interface ParameterAwareInjectionPointsTransformer extends InjectionPointsTransformer

But it generally feels like mirroring InjectionPointInfo is best.

[mkouba]

I would call the method getInjectionTarget(), not getInjectedTarget().

I wouldn't because then a caller could expect the jakarta.enterprise.inject.spi.InjectionTarget to be returned which is not the case. Hm, we could try to come with other names...

But it generally feels like mirroring InjectionPointInfo is best.

We would need to deprecate InjectionPointsTransformer.TransformationContext.getTarget() (and update the javadoc about the returned value) and also InjectionPointsTransformer.TransformationContext.getAllAnnotations() which currently returns all annotations of the target method. Then add 2 new methods. One for the target and one for annotations. And that's IMO reasonable and much better than all the things listed above (ParameterAwareInjectionPointsTransformer etc.).

[Ladicek]

I would call the method getInjectionTarget(), not getInjectedTarget().

I wouldn't because then a caller could expect the jakarta.enterprise.inject.spi.InjectionTarget to be returned which is not the case.

I would expect most Quarkus developers to not even know about CDI's InjectionTarget, and even if they do, any such possible expectation is quickly dissolved by your IDE or compiler :-)

But we could call it getProgramElement(), because it does return a representation of the program element corresponding to the injection point.

The only downside is that most of the ArC API uses the word "target" as a name for this concept.

But it generally feels like mirroring InjectionPointInfo is best.

We would need to deprecate InjectionPointsTransformer.TransformationContext.getTarget() (and update the javadoc about the returned value) and also InjectionPointsTransformer.TransformationContext.getAllAnnotations() which currently returns all annotations of the target method. Then add 2 new methods. One for the target and one for annotations. And that's IMO reasonable and much better than all the things listed above (ParameterAwareInjectionPointsTransformer etc.).

Agree.

[mkouba]

But we could call it getProgramElement(), because it does return a representation of the program element corresponding to the injection point.

How about just getAnnotationTarget()?

[Ladicek]

That's reasonable. (One might ask what have annotations to do with any of this, but the return type of the method has been AnnotationTarget anyway.)

(getActualTarget(), anyone? :-) )

[mkouba]

Well, we might also try getInjectionPointTarget() but from my point of view, all these names (getAnnotationTarget(), getActualTarget() and getInjectionPointTarget()) are ok so let Matej decide (and take on the naming challenge ;-).

[Ladicek]

Haha good idea! 😆