From: Alexander Kriegisch Date: Sun, 4 Dec 2022 15:16:37 +0000 (+0100) Subject: Fix #366085 concerning declared annotations with source retention X-Git-Tag: V1_9_19~4 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=65f1ec72c2fc9446a162780dc3f6dee625704c02;p=aspectj.git Fix #366085 concerning declared annotations with source retention See https://bugs.eclipse.org/bugs/show_bug.cgi?id=366085. See https://stackoverflow.com/q/74618269/1082681. The issue described in the Bugzilla issue is about 'declare @type', but similar issues also existed for 'declare @field', 'declare @method', 'declare @constructor'. This fix is rather superficial and leaves things to be desired, because it is rather hacky and simply ignores errors source retention annotation declarations during weaving. A better fix would drop the corresponding declarations while parsing and also issue compiler warnings in each case. Signed-off-by: Alexander Kriegisch --- diff --git a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/DeclareAnnotation.java b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/DeclareAnnotation.java index 7a452ef05..00bbf5647 100644 --- a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/DeclareAnnotation.java +++ b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/patterns/DeclareAnnotation.java @@ -380,6 +380,8 @@ public class DeclareAnnotation extends Declare { int idx = 0; if (annos.length > 0 && annos[0].getType().getSignature().equals("Lorg/aspectj/internal/lang/annotation/ajcDeclareAnnotation;")) { + if (annos.length < 2) + continue; idx = 1; } annotation = annos[idx]; @@ -441,6 +443,8 @@ public class DeclareAnnotation extends Declare { int idx = 0; if (annoTypes[0].getSignature().equals("Lorg/aspectj/internal/lang/annotation/ajcDeclareAnnotation;")) { idx = 1; + if (annoTypes.length < 2) + continue; } annotationType = annoTypes[idx]; break; diff --git a/tests/bugs1919/366085/Application.java b/tests/bugs1919/366085/Application.java new file mode 100644 index 000000000..3ef2126fd --- /dev/null +++ b/tests/bugs1919/366085/Application.java @@ -0,0 +1,11 @@ +public class Application { + public int number; + + public Application(int number) { + this.number = number; + } + + public int getNumber() { + return number; + } +} diff --git a/tests/bugs1919/366085/DeclareAnnotationsAspect.aj b/tests/bugs1919/366085/DeclareAnnotationsAspect.aj new file mode 100644 index 000000000..8fdc9460c --- /dev/null +++ b/tests/bugs1919/366085/DeclareAnnotationsAspect.aj @@ -0,0 +1,13 @@ +public aspect DeclareAnnotationsAspect { + // These should be ignored, because @ToString has SOURCE retention + declare @type : Application : @ToString; + declare @method : * Application.*(..) : @ToString; + declare @constructor : Application.new(..) : @ToString; + declare @field : * Application.* : @ToString; + + // These should be applied, because @Marker has RUNTIME retention + declare @type : Application : @Marker; + declare @method : * Application.*(..) : @Marker; + declare @constructor : Application.new(..) : @Marker; + declare @field : * Application.* : @Marker; +} diff --git a/tests/bugs1919/366085/Marker.java b/tests/bugs1919/366085/Marker.java new file mode 100644 index 000000000..42ba9af15 --- /dev/null +++ b/tests/bugs1919/366085/Marker.java @@ -0,0 +1,4 @@ +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +@Retention(RetentionPolicy.RUNTIME) +public @interface Marker { } diff --git a/tests/bugs1919/366085/ToString.java b/tests/bugs1919/366085/ToString.java new file mode 100644 index 000000000..08e9399c4 --- /dev/null +++ b/tests/bugs1919/366085/ToString.java @@ -0,0 +1,4 @@ +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +@Retention(RetentionPolicy.SOURCE) +public @interface ToString { } diff --git a/tests/src/test/java/org/aspectj/systemtest/ajc1919/Bugs1919Tests.java b/tests/src/test/java/org/aspectj/systemtest/ajc1919/Bugs1919Tests.java index 38441267e..3c921f51a 100644 --- a/tests/src/test/java/org/aspectj/systemtest/ajc1919/Bugs1919Tests.java +++ b/tests/src/test/java/org/aspectj/systemtest/ajc1919/Bugs1919Tests.java @@ -15,8 +15,8 @@ import org.aspectj.testing.XMLBasedAjcTestCase; */ public class Bugs1919Tests extends XMLBasedAjcTestCase { - public void testDummyBug() { - //runTest("dummy Java 19 bug"); + public void testDeclareAnnotationWithSourceRetention() { + runTest("declare annotation with SOURCE retention"); } public static Test suite() { diff --git a/tests/src/test/resources/org/aspectj/systemtest/ajc1919/ajc1919.xml b/tests/src/test/resources/org/aspectj/systemtest/ajc1919/ajc1919.xml index 3741b338c..eaeca7143 100644 --- a/tests/src/test/resources/org/aspectj/systemtest/ajc1919/ajc1919.xml +++ b/tests/src/test/resources/org/aspectj/systemtest/ajc1919/ajc1919.xml @@ -162,8 +162,14 @@ - - - + + + + + + + + + diff --git a/weaver/src/main/java/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/main/java/org/aspectj/weaver/bcel/BcelClassWeaver.java index 6a71640ca..450a49189 100644 --- a/weaver/src/main/java/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/main/java/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -1023,7 +1023,12 @@ class BcelClassWeaver implements IClassWeaver { if (annotationsToAdd == null) { annotationsToAdd = new ArrayList<>(); } - AnnotationGen a = ((BcelAnnotation) decaM.getAnnotation()).getBcelAnnotation(); + BcelAnnotation decaMAnnotation = (BcelAnnotation) decaM.getAnnotation(); + if (decaMAnnotation == null) { + unusedDecams.remove(decaM); + continue; + } + AnnotationGen a = decaMAnnotation.getBcelAnnotation(); AnnotationGen ag = new AnnotationGen(a, clazz.getConstantPool(), true); annotationsToAdd.add(ag); mg.addAnnotation(decaM.getAnnotation()); @@ -1423,7 +1428,8 @@ class BcelClassWeaver implements IClassWeaver { // go through all the declare @field statements for (DeclareAnnotation decaf : decafs) { if (decaf.getAnnotation() == null) { - return false; + unusedDecafs.remove(decaf); + continue; } if (decaf.matches(field, world)) { if (decaf.isRemover()) {