Nested annotations no longer supported in ASM-based annotation processing

[AndreasKl]

tl;dr

Build https://github.com/AndreasKl/spring-issue-in-TypeMappedAnnotation the test fails with the following exception:

org.springframework.beans.factory.BeanDefinitionStoreException: Failed to read candidate component class: file [.../demo/DemoApplicationTests.class]; nested exception is java.lang.ClassCastException: class org.springframework.core.annotation.TypeMappedAnnotation cannot be cast to class java.util.Map (org.springframework.core.annotation.TypeMappedAnnotation is in unnamed module of loader 'app'; java.util.Map is in module java.base of loader 'bootstrap')

This is caused by the @AliasFor("value") and @AliasFor("hallo") on public @interface Bad.

We do not see this behaviour with Spring Boot 2.1.11 (Spring Core 5.1.12.RELEASE) but can reproduce the issue on (Spring Core 5.2.2.RELEASE and 5.2.4.BUILD-SNAPSHOT).

Failing test case:

public class Gh24375Tests {

  @Target({ElementType.METHOD, ElementType.TYPE})
  @Retention(RetentionPolicy.RUNTIME)
  public @interface A {

    @AliasFor("value")
    B versus() default @B;

    @AliasFor("versus")
    B value() default @B;
  }

  @Target(ElementType.ANNOTATION_TYPE)
  @Retention(RetentionPolicy.RUNTIME)
  public @interface B {

    String name() default "";
  }

  @Test
  @A(versus = @B)
  public void gh24375() {
    new ClassPathScanningCandidateComponentProvider(true)
        .findCandidateComponents(Integer.class.getPackage().getName());
  }
}

The root cause could be in:

org.springframework.core.annotation.AnnotationTypeMapping#areEquivalent(java.lang.Object, java.lang.Object, java.util.function.BiFunction<java.lang.reflect.Method,java.lang.Object,java.lang.Object>)

and could be mitigated by adding:

if (value instanceof Annotation && extractedValue instanceof TypeMappedAnnotation) {
  return areEquivalent((Annotation) value, (TypeMappedAnnotation<?>)extractedValue, valueExtractor);
}

private static boolean areEquivalent(Annotation annotation, @Nullable TypeMappedAnnotation<?> extractedValue, BiFunction<Method, Object, Object> valueExtractor) {

    AttributeMethods attributes = AttributeMethods.forAnnotationType(annotation.annotationType());
    for (int i = 0; i < attributes.size(); i++) {
        Method attribute = attributes.get(i);
        String name = attributes.get(i).getName();
        if (!areEquivalent(ReflectionUtils.invokeMethod(attribute, annotation),
            extractedValue.getValue(name).orElse(null), valueExtractor)) {
            return false;
        }
    }
    return true;
}

As this is core framework code I'm not really feeling confident to provide a PR without breaking something else.

[sbrannen]

Thanks for raising the issue!

I have confirmed that the supplied test case executes without errors on 5.1.x and fails with the stated exception on master.

[sbrannen]

Update: I've submitted a passing regression test for 5.1.x in 6699833 and a failing (though currently @Disabled) regression test for master/5.2.x in 2a2efbe.

[sbrannen]

As this is core framework code I'm not really feeling confident to provide a PR without breaking something else.

I came to a similar conclusion about what might need to be done to provide a fix. I also looked at your code and tried it out against master, and it seems to do the job.

But let's wait on feedback from @philwebb before taking the next step.

Cheers

[philwebb]

I think the change looks right. I probably need to see the actual commit to be sure. The only thing that concerns me is how similar two areEquivalent method appear. @sbrannen If you have a commit lined up I'll take a look in a debugger when I get a sec.

[sbrannen]

I think the change looks right. I probably need to see the actual commit to be sure. The only thing that concerns me is how similar two areEquivalent method appear. @sbrannen If you have a commit lined up I'll take a look in a debugger when I get a sec.

Indeed, I implemented the fix slightly differently by adding logic to the existing areEquivalent(Annotation, Object, BiFunction<Method, Object, Object>) method.

See this commit for details: sbrannen@625dc7b

See this commit for details: sbrannen@b55a564

@philwebb, as soon as you give a 👍 for that, I'll merge it into master.

Cheers

[sbrannen]

This has been fixed in 974caca and revised in 5d4f1d9.

@AndreasKl, feel free to try it out in the next 5.2.4 snapshot build and let us know if it works for you.