search

eclipse-aspectj / aspectj

Other
300 stars 86 forks source link

Support AspectJ annotations as meta-annotations #81

Open sbrannen opened 3 years ago

sbrannen commented 3 years ago

Many projects in the Java ecosystem support the use of their annotations as meta-annotations (Spring Framework, JUnit 5, etc.); however, AspectJ currently does not support the use of annotations such as @Aspect as a meta-annotation.

AspectJ internals such as org.aspectj.internal.lang.reflect.AjTypeImpl.isAspect() use standard JDK algorithms like java.lang.Class.getAnnotation() to find annotations; however, it would be relatively simple to modify AspectJ's annotation lookups to also find AspectJ annotations when used as meta-annotations.

See also: https://github.com/spring-projects/spring-framework/issues/27221

aclement commented 3 years ago

I think we should do that. Might be slightly wider scope though because I think AjTypeImpl and the reflective type infrastructure is for the load-time weaving, we would want consistency with build time weaving too. So I'm just saying other avenues through the code recognizing aspects may need to recognize the meta usage.

quaff commented 3 years ago

why not using isAnnotationPresent here https://github.com/eclipse/org.aspectj/blob/67b1c353a02c335074a736ebf76a49bf24eefb19/runtime/src/main/java/org/aspectj/internal/lang/reflect/AjTypeImpl.java#L1059-L1061 like https://github.com/eclipse/org.aspectj/blob/67b1c353a02c335074a736ebf76a49bf24eefb19/runtime/src/main/java/org/aspectj/internal/lang/reflect/AjTypeImpl.java#L1067-L1069

sbrannen commented 3 years ago

@quaff, clazz.getAnnotation(annotationType) != null is logically equivalent to clazz.isAnnotationPresent(annotationType).

They both check if the annotation type is present (i.e., directly present or @Inherited).

So switching to isAnnotationPresent() would simplify the current implementation, but it does not add meta-annotation support.

In order to support finding AspectJ annotations when used as meta-annotations, custom annotation search algorithms have to be implemented, similar to those in the Spring Framework and JUnit 5. However, AspectJ does not need algorithms as complicated as those in Spring and JUnit.

quaff commented 3 years ago

@quaff, clazz.getAnnotation(annotationType) != null is logically equivalent to clazz.isAnnotationPresent(annotationType).

They both check if the annotation type is present (i.e., directly present or @Inherited).

So switching to isAnnotationPresent() would simplify the current implementation, but it does not add meta-annotation support.

In order to support finding AspectJ annotations when used as meta-annotations, custom annotation search algorithms have to be implemented, similar to those in the Spring Framework and JUnit 5. However, AspectJ does not need algorithms as complicated as those in Spring and JUnit.

@sbrannen , my point is the inconsistent code style in the same source file, not address this issue, thanks for your attention.