Skip to content

Commit 62d7396

Browse files
jhoellerjhoeller
committed
Suggest compilation with -parameters in case of ambiguity
Closes gh-34609 (cherry picked from commit 86b2617)
1 parent 252058a commit 62d7396

File tree

2 files changed

+41
-41
lines changed

2 files changed

+41
-41
lines changed

‎spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJAdviceParameterNameDiscoverer.java

+11-8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -241,7 +241,7 @@ public String[] getParameterNames(Method method) {
241241

242242
try {
243243
int algorithmicStep = STEP_JOIN_POINT_BINDING;
244-
while ((this.numberOfRemainingUnboundArguments > 0) && algorithmicStep < STEP_FINISHED) {
244+
while (this.numberOfRemainingUnboundArguments > 0 && algorithmicStep < STEP_FINISHED) {
245245
switch (algorithmicStep++) {
246246
case STEP_JOIN_POINT_BINDING -> {
247247
if (!maybeBindThisJoinPoint()) {
@@ -373,7 +373,8 @@ private void maybeBindReturningVariable() {
373373
if (this.returningName != null) {
374374
if (this.numberOfRemainingUnboundArguments > 1) {
375375
throw new AmbiguousBindingException("Binding of returning parameter '" + this.returningName +
376-
"' is ambiguous: there are " + this.numberOfRemainingUnboundArguments + " candidates.");
376+
"' is ambiguous: there are " + this.numberOfRemainingUnboundArguments + " candidates. " +
377+
"Consider compiling with -parameters in order to make declared parameter names available.");
377378
}
378379

379380
// We're all set... find the unbound parameter, and bind it.
@@ -485,8 +486,8 @@ private void maybeExtractVariableNamesFromArgs(@Nullable String argsSpec, List<S
485486
*/
486487
private void maybeBindThisOrTargetOrArgsFromPointcutExpression() {
487488
if (this.numberOfRemainingUnboundArguments > 1) {
488-
throw new AmbiguousBindingException("Still " + this.numberOfRemainingUnboundArguments
489-
+ " unbound args at this()/target()/args() binding stage, with no way to determine between them");
489+
throw new AmbiguousBindingException("Still " + this.numberOfRemainingUnboundArguments +
490+
" unbound args at this()/target()/args() binding stage, with no way to determine between them");
490491
}
491492

492493
List<String> varNames = new ArrayList<>();
@@ -535,8 +536,8 @@ else if (varNames.size() == 1) {
535536

536537
private void maybeBindReferencePointcutParameter() {
537538
if (this.numberOfRemainingUnboundArguments > 1) {
538-
throw new AmbiguousBindingException("Still " + this.numberOfRemainingUnboundArguments
539-
+ " unbound args at reference pointcut binding stage, with no way to determine between them");
539+
throw new AmbiguousBindingException("Still " + this.numberOfRemainingUnboundArguments +
540+
" unbound args at reference pointcut binding stage, with no way to determine between them");
540541
}
541542

542543
List<String> varNames = new ArrayList<>();
@@ -741,7 +742,9 @@ private void findAndBind(Class<?> argumentType, String varName) {
741742
* Simple record to hold the extracted text from a pointcut body, together
742743
* with the number of tokens consumed in extracting it.
743744
*/
744-
private record PointcutBody(int numTokensConsumed, @Nullable String text) {}
745+
private record PointcutBody(int numTokensConsumed, @Nullable String text) {
746+
}
747+
745748

746749
/**
747750
* Thrown in response to an ambiguous binding being detected when

‎spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java

+30-33
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -203,7 +203,6 @@ void perThisAspect() throws Exception {
203203
itb.getSpouse();
204204

205205
assertThat(maaif.isMaterialized()).isTrue();
206-
207206
assertThat(imapa.getDeclaredPointcut().getMethodMatcher().matches(TestBean.class.getMethod("getAge"), null)).isTrue();
208207

209208
assertThat(itb.getAge()).as("Around advice must apply").isEqualTo(0);
@@ -301,7 +300,7 @@ void bindingWithSingleArg() {
301300
void bindingWithMultipleArgsDifferentlyOrdered() {
302301
ManyValuedArgs target = new ManyValuedArgs();
303302
ManyValuedArgs mva = createProxy(target, ManyValuedArgs.class,
304-
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new ManyValuedArgs(), "someBean")));
303+
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new ManyValuedArgs(), "someBean")));
305304

306305
String a = "a";
307306
int b = 12;
@@ -320,7 +319,7 @@ void introductionOnTargetNotImplementingInterface() {
320319
NotLockable notLockableTarget = new NotLockable();
321320
assertThat(notLockableTarget).isNotInstanceOf(Lockable.class);
322321
NotLockable notLockable1 = createProxy(notLockableTarget, NotLockable.class,
323-
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new MakeLockable(), "someBean")));
322+
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new MakeLockable(), "someBean")));
324323
assertThat(notLockable1).isInstanceOf(Lockable.class);
325324
Lockable lockable = (Lockable) notLockable1;
326325
assertThat(lockable.locked()).isFalse();
@@ -329,7 +328,7 @@ void introductionOnTargetNotImplementingInterface() {
329328

330329
NotLockable notLockable2Target = new NotLockable();
331330
NotLockable notLockable2 = createProxy(notLockable2Target, NotLockable.class,
332-
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new MakeLockable(), "someBean")));
331+
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new MakeLockable(), "someBean")));
333332
assertThat(notLockable2).isInstanceOf(Lockable.class);
334333
Lockable lockable2 = (Lockable) notLockable2;
335334
assertThat(lockable2.locked()).isFalse();
@@ -343,20 +342,19 @@ void introductionOnTargetNotImplementingInterface() {
343342
void introductionAdvisorExcludedFromTargetImplementingInterface() {
344343
assertThat(AopUtils.findAdvisorsThatCanApply(
345344
getAdvisorFactory().getAdvisors(
346-
aspectInstanceFactory(new MakeLockable(), "someBean")),
345+
aspectInstanceFactory(new MakeLockable(), "someBean")),
347346
CannotBeUnlocked.class)).isEmpty();
348347
assertThat(AopUtils.findAdvisorsThatCanApply(getAdvisorFactory().getAdvisors(
349-
aspectInstanceFactory(new MakeLockable(),"someBean")), NotLockable.class)).hasSize(2);
348+
aspectInstanceFactory(new MakeLockable(),"someBean")), NotLockable.class)).hasSize(2);
350349
}
351350

352351
@Test
353352
void introductionOnTargetImplementingInterface() {
354353
CannotBeUnlocked target = new CannotBeUnlocked();
355354
Lockable proxy = createProxy(target, CannotBeUnlocked.class,
356-
// Ensure that we exclude
357355
AopUtils.findAdvisorsThatCanApply(
358-
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new MakeLockable(), "someBean")),
359-
CannotBeUnlocked.class));
356+
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new MakeLockable(), "someBean")),
357+
CannotBeUnlocked.class));
360358
assertThat(proxy).isInstanceOf(Lockable.class);
361359
Lockable lockable = proxy;
362360
assertThat(lockable.locked()).as("Already locked").isTrue();
@@ -370,16 +368,16 @@ void introductionOnTargetExcludedByTypePattern() {
370368
ArrayList<Object> target = new ArrayList<>();
371369
List<?> proxy = createProxy(target, List.class,
372370
AopUtils.findAdvisorsThatCanApply(
373-
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new MakeLockable(), "someBean")),
374-
List.class));
371+
getAdvisorFactory().getAdvisors(aspectInstanceFactory(new MakeLockable(), "someBean")),
372+
List.class));
375373
assertThat(proxy).as("Type pattern must have excluded mixin").isNotInstanceOf(Lockable.class);
376374
}
377375

378376
@Test
379377
void introductionBasedOnAnnotationMatch() { // gh-9980
380378
AnnotatedTarget target = new AnnotatedTargetImpl();
381379
List<Advisor> advisors = getAdvisorFactory().getAdvisors(
382-
aspectInstanceFactory(new MakeAnnotatedTypeModifiable(), "someBean"));
380+
aspectInstanceFactory(new MakeAnnotatedTypeModifiable(), "someBean"));
383381
Object proxy = createProxy(target, AnnotatedTarget.class, advisors);
384382
assertThat(proxy).isInstanceOf(Lockable.class);
385383
Lockable lockable = (Lockable) proxy;
@@ -393,9 +391,9 @@ void introductionWithArgumentBinding() {
393391
TestBean target = new TestBean();
394392

395393
List<Advisor> advisors = getAdvisorFactory().getAdvisors(
396-
aspectInstanceFactory(new MakeITestBeanModifiable(), "someBean"));
394+
aspectInstanceFactory(new MakeITestBeanModifiable(), "someBean"));
397395
advisors.addAll(getAdvisorFactory().getAdvisors(
398-
aspectInstanceFactory(new MakeLockable(), "someBean")));
396+
aspectInstanceFactory(new MakeLockable(), "someBean")));
399397

400398
Modifiable modifiable = (Modifiable) createProxy(target, ITestBean.class, advisors);
401399
assertThat(modifiable).isInstanceOf(Modifiable.class);
@@ -426,7 +424,7 @@ void aspectMethodThrowsExceptionLegalOnSignature() {
426424
TestBean target = new TestBean();
427425
UnsupportedOperationException expectedException = new UnsupportedOperationException();
428426
List<Advisor> advisors = getAdvisorFactory().getAdvisors(
429-
aspectInstanceFactory(new ExceptionThrowingAspect(expectedException), "someBean"));
427+
aspectInstanceFactory(new ExceptionThrowingAspect(expectedException), "someBean"));
430428
assertThat(advisors).as("One advice method was found").hasSize(1);
431429
ITestBean itb = createProxy(target, ITestBean.class, advisors);
432430
assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(itb::getAge);
@@ -439,20 +437,20 @@ void aspectMethodThrowsExceptionIllegalOnSignature() {
439437
TestBean target = new TestBean();
440438
RemoteException expectedException = new RemoteException();
441439
List<Advisor> advisors = getAdvisorFactory().getAdvisors(
442-
aspectInstanceFactory(new ExceptionThrowingAspect(expectedException), "someBean"));
440+
aspectInstanceFactory(new ExceptionThrowingAspect(expectedException), "someBean"));
443441
assertThat(advisors).as("One advice method was found").hasSize(1);
444442
ITestBean itb = createProxy(target, ITestBean.class, advisors);
445443
assertThatExceptionOfType(UndeclaredThrowableException.class)
446-
.isThrownBy(itb::getAge)
447-
.withCause(expectedException);
444+
.isThrownBy(itb::getAge)
445+
.withCause(expectedException);
448446
}
449447

450448
@Test
451449
void twoAdvicesOnOneAspect() {
452450
TestBean target = new TestBean();
453451
TwoAdviceAspect twoAdviceAspect = new TwoAdviceAspect();
454452
List<Advisor> advisors = getAdvisorFactory().getAdvisors(
455-
aspectInstanceFactory(twoAdviceAspect, "someBean"));
453+
aspectInstanceFactory(twoAdviceAspect, "someBean"));
456454
assertThat(advisors).as("Two advice methods found").hasSize(2);
457455
ITestBean itb = createProxy(target, ITestBean.class, advisors);
458456
itb.setName("");
@@ -466,7 +464,7 @@ void twoAdvicesOnOneAspect() {
466464
void afterAdviceTypes() throws Exception {
467465
InvocationTrackingAspect aspect = new InvocationTrackingAspect();
468466
List<Advisor> advisors = getAdvisorFactory().getAdvisors(
469-
aspectInstanceFactory(aspect, "exceptionHandlingAspect"));
467+
aspectInstanceFactory(aspect, "exceptionHandlingAspect"));
470468
Echo echo = createProxy(new Echo(), Echo.class, advisors);
471469

472470
assertThat(aspect.invocations).isEmpty();
@@ -475,7 +473,7 @@ void afterAdviceTypes() throws Exception {
475473

476474
aspect.invocations.clear();
477475
assertThatExceptionOfType(FileNotFoundException.class)
478-
.isThrownBy(() -> echo.echo(new FileNotFoundException()));
476+
.isThrownBy(() -> echo.echo(new FileNotFoundException()));
479477
assertThat(aspect.invocations).containsExactly("around - start", "before", "after throwing", "after", "around - end");
480478
}
481479

@@ -487,7 +485,6 @@ void nonAbstractParentAspect() {
487485
assertThat(Modifier.isAbstract(aspect.getClass().getSuperclass().getModifiers())).isFalse();
488486

489487
List<Advisor> advisors = getAdvisorFactory().getAdvisors(aspectInstanceFactory(aspect, "incrementingAspect"));
490-
491488
ITestBean proxy = createProxy(new TestBean("Jane", 42), ITestBean.class, advisors);
492489
assertThat(proxy.getAge()).isEqualTo(86); // (42 + 1) * 2
493490
}
@@ -812,19 +809,19 @@ void before() {
812809
invocations.add("before");
813810
}
814811

815-
@AfterReturning("echo()")
816-
void afterReturning() {
817-
invocations.add("after returning");
812+
@After("echo()")
813+
void after() {
814+
invocations.add("after");
818815
}
819816

820-
@AfterThrowing("echo()")
821-
void afterThrowing() {
822-
invocations.add("after throwing");
817+
@AfterReturning(pointcut = "this(target) && execution(* echo(*))", returning = "returnValue")
818+
void afterReturning(JoinPoint joinPoint, Echo target, Object returnValue) {
819+
invocations.add("after returning");
823820
}
824821

825-
@After("echo()")
826-
void after() {
827-
invocations.add("after");
822+
@AfterThrowing(pointcut = "this(target) && execution(* echo(*))", throwing = "exception")
823+
void afterThrowing(JoinPoint joinPoint, Echo target, Throwable exception) {
824+
invocations.add("after throwing");
828825
}
829826
}
830827

@@ -967,7 +964,7 @@ private Method getGetterFromSetter(Method setter) {
967964
class MakeITestBeanModifiable extends AbstractMakeModifiable {
968965

969966
@DeclareParents(value = "org.springframework.beans.testfixture.beans.ITestBean+",
970-
defaultImpl=ModifiableImpl.class)
967+
defaultImpl = ModifiableImpl.class)
971968
static MutableModifiable mixin;
972969

973970
}

0 commit comments

Comments
 (0)