Suggest compilation with `-parameters` when `AspectJAdviceParameterNameDiscoverer` fails against ambiguity

[xardael]

Hi, I had this annotation which worked with AspectJ, however stopped working after I migrated to spring-aspects + spring-aop version 6.2.4:

@AfterReturning(pointcut = "this(processor) && @annotation(x.y.z.ProcessorListenerHook)", returning = "returnValue")
public void executeListenersAfter(JoinPoint joinPoint, Processor processor, Object returnValue) {

Exception is:

AmbiguousBindingException Binding of returning parameter is ambiguous: there are 2 candidates.

The problem is in AspectJAdviceParameterNameDiscoverer.getParameterNames - in processing parameters according to this order

public class AspectJAdviceParameterNameDiscoverer implements ParameterNameDiscoverer {

  private static final String THIS_JOIN_POINT = "thisJoinPoint";
  private static final String THIS_JOIN_POINT_STATIC_PART = "thisJoinPointStaticPart";

  // Steps in the binding algorithm...
  private static final int STEP_JOIN_POINT_BINDING = 1;
  private static final int STEP_THROWING_BINDING = 2;
  private static final int STEP_ANNOTATION_BINDING = 3;
  private static final int STEP_RETURNING_BINDING = 4;
  private static final int STEP_PRIMITIVE_ARGS_BINDING = 5;
  private static final int STEP_THIS_TARGET_ARGS_BINDING = 6;
  private static final int STEP_REFERENCE_PCUT_BINDING = 7;
  private static final int STEP_FINISHED = 8;

"Returning binding" is processed BEFORE "this target binding" so during calling maybeBindReturningVariable() there are two unbound arguments left, so this method throws an exception.

  /**
   * If a returning variable was specified and there is only one choice remaining, bind it.
   */
  private void maybeBindReturningVariable() {
    if (this.numberOfRemainingUnboundArguments == 0) {
      throw new IllegalStateException(
          "Algorithm assumes that there must be at least one unbound parameter on entry to this method");
    }

    if (this.returningName != null) {
      if (this.numberOfRemainingUnboundArguments > 1) {
        throw new AmbiguousBindingException("Binding of returning parameter '" + this.returningName +
            "' is ambiguous: there are " + this.numberOfRemainingUnboundArguments + " candidates.");
      }

Is this the intended behavior?

How should I use both this and returning in one @AfterReturning annotation?

Thanks.

[sbrannen]

Hi @xardael,

Congratulations on opening your first issue for the Spring Framework! 👍

Hi, I had this annotation which worked with AspectJ, however stopped working after I migrated to spring-aspects + spring-aop version 6.2.4:

Just to clarify...

Are you saying that your use of @AfterReturning works when compiling an aspect using the AspectJ compiler (ajc)?

Or are you saying that your use of @AfterReturning worked with a previous version of the Spring Framework and stopped working with Spring Framework 6.2.4?

[xardael]

Hi,

Are you saying that your use of @AfterReturning works when compiling an aspect using the AspectJ compiler (ajc)

I used aspectjrt v. 1.9.22.1.

Edit: I just want to clarify that the exception
AmbiguousBindingException Binding of returning parameter is ambiguous: there are 2 candidates. is thrown during runtime.

[jhoeller]

I'm afraid this is a limitation of AspectJAdviceParameterNameDiscoverer indeed: It can only handle signatures that are unambiguous without taking the actual parameter names into account. In your scenario, the processor and returnValue arguments are ambiguous if the declared parameter names are not available since both of them could be a target or a return value in terms of type.

The recommended solution is to compile your code with -parameters so that our StandardReflectionParameterNameDiscoverer strategy applies in preference to AspectJAdviceParameterNameDiscoverer which is really a limited fallback (and quite old as well). I'll turn this into a corresponding enhancement request for improving the exception message along those lines.