Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest compilation with -parameters when AspectJAdviceParameterNameDiscoverer fails against ambiguity #34609

Closed
xardael opened this issue Mar 17, 2025 · 5 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Milestone

Comments

@xardael
Copy link

xardael commented Mar 17, 2025

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.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 17, 2025
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Mar 17, 2025
@sbrannen
Copy link
Member

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?

@sbrannen sbrannen added the status: waiting-for-feedback We need additional information before we can continue label Mar 17, 2025
@xardael
Copy link
Author

xardael commented Mar 17, 2025

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.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 17, 2025
@jhoeller
Copy link
Contributor

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.

@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 17, 2025
@jhoeller jhoeller self-assigned this Mar 17, 2025
@jhoeller jhoeller added this to the 6.2.5 milestone Mar 17, 2025
@jhoeller jhoeller changed the title AspectJAdviceParameterNameDiscoverer - getParameterNames bug? Suggest compilation with -parameters when AspectJAdviceParameterNameDiscoverer fails against ambiguity Mar 17, 2025
@xardael
Copy link
Author

xardael commented Mar 18, 2025

Thank you @jhoeller, compiled with "-parameters" solved the issue.

One more questien, could it be somehow specified using
argNames="processor,returnValue"
or this is not related?

Thanks.

@jhoeller
Copy link
Contributor

This should work as well. I recommend compiling with -parameters though.

@jhoeller jhoeller added the for: backport-to-6.1.x Marks an issue as a candidate for backport to 6.1.x label Mar 19, 2025
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.1.x Marks an issue as a candidate for backport to 6.1.x labels Mar 19, 2025
jhoeller added a commit that referenced this issue Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants