From: Andy Clement Date: Thu, 13 Jun 2013 19:29:45 +0000 (-0700) Subject: Preserve ordering of declare annotation when removing and adding annotations X-Git-Tag: AS_BETA_JAVA8_CREATED~4 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5b0b6b07d2b581cddd1bac4a2a6b12cd4ac02b07;p=aspectj.git Preserve ordering of declare annotation when removing and adding annotations Issue: 407739 --- diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembers.java b/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembers.java index 87e32b273..8e41c0a82 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembers.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembers.java @@ -16,6 +16,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.Hashtable; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -58,9 +59,9 @@ public class CrosscuttingMembers { private List declareDominates = new ArrayList(4); // These are like declare parents type mungers - private Set declareAnnotationsOnType = new HashSet(); - private Set declareAnnotationsOnField = new HashSet(); - private Set declareAnnotationsOnMethods = new HashSet(); + private Set declareAnnotationsOnType = new LinkedHashSet(); + private Set declareAnnotationsOnField = new LinkedHashSet(); + private Set declareAnnotationsOnMethods = new LinkedHashSet(); // declareAnnotationsOnMethods includes constructors too private Set declareTypeEow = new HashSet(); diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembersSet.java b/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembersSet.java index 59bd2c097..3937830cd 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembersSet.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembersSet.java @@ -18,6 +18,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -286,10 +287,12 @@ public class CrosscuttingMembersSet { return declareParents; } - // DECAT Merge multiple together + /** + * @return an amalgamation of the declare @type statements. + */ public List getDeclareAnnotationOnTypes() { if (declareAnnotationOnTypes == null) { - Set ret = new HashSet(); + Set ret = new LinkedHashSet(); for (Iterator i = members.values().iterator(); i.hasNext();) { ret.addAll(i.next().getDeclareAnnotationOnTypes()); } @@ -299,9 +302,12 @@ public class CrosscuttingMembersSet { return declareAnnotationOnTypes; } + /** + * @return an amalgamation of the declare @field statements. + */ public List getDeclareAnnotationOnFields() { if (declareAnnotationOnFields == null) { - Set ret = new HashSet(); + Set ret = new LinkedHashSet(); for (Iterator i = members.values().iterator(); i.hasNext();) { ret.addAll(i.next().getDeclareAnnotationOnFields()); } @@ -312,11 +318,11 @@ public class CrosscuttingMembersSet { } /** - * Return an amalgamation of the declare @method/@constructor statements. + * @return an amalgamation of the declare @method/@constructor statements. */ public List getDeclareAnnotationOnMethods() { if (declareAnnotationOnMethods == null) { - Set ret = new HashSet(); + Set ret = new LinkedHashSet(); for (Iterator i = members.values().iterator(); i.hasNext();) { ret.addAll(i.next().getDeclareAnnotationOnMethods()); } diff --git a/tests/bugs173/pr407739/.AbstractAspectChangeAnnotation.java.swp b/tests/bugs173/pr407739/.AbstractAspectChangeAnnotation.java.swp new file mode 100644 index 000000000..f27d62670 Binary files /dev/null and b/tests/bugs173/pr407739/.AbstractAspectChangeAnnotation.java.swp differ diff --git a/tests/bugs173/pr407739/Aspect.java b/tests/bugs173/pr407739/Aspect.java new file mode 100644 index 000000000..439261bd9 --- /dev/null +++ b/tests/bugs173/pr407739/Aspect.java @@ -0,0 +1,6 @@ +public aspect Aspect { + + declare @field: * Hello.dummy : -@MyAnnotation; + declare @field: * Hello.dummy : @MyAnnotation(dummy2 = "korte"); + +} diff --git a/tests/bugs173/pr407739/Hello.java b/tests/bugs173/pr407739/Hello.java new file mode 100644 index 000000000..81c13f14d --- /dev/null +++ b/tests/bugs173/pr407739/Hello.java @@ -0,0 +1,9 @@ +public class Hello { + + @MyAnnotation(dummy1 = "alma") + private String dummy; + + public static void main(String []argv) throws Exception { + System.out.print(Hello.class.getDeclaredField("dummy").getDeclaredAnnotations()[0]); + } +} diff --git a/tests/bugs173/pr407739/MyAnnotation.java b/tests/bugs173/pr407739/MyAnnotation.java new file mode 100644 index 000000000..fbe3c25fb --- /dev/null +++ b/tests/bugs173/pr407739/MyAnnotation.java @@ -0,0 +1,13 @@ + +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.METHOD; +import java.lang.annotation.Retention; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import java.lang.annotation.Target; + +@Target({METHOD, FIELD}) +@Retention(RUNTIME) +public @interface MyAnnotation { + String dummy1() default ""; + String dummy2() default ""; +} diff --git a/tests/bugs173/pr407966/Aspect.aj b/tests/bugs173/pr407966/Aspect.aj new file mode 100644 index 000000000..7270f742f --- /dev/null +++ b/tests/bugs173/pr407966/Aspect.aj @@ -0,0 +1,5 @@ +public aspect Aspect { + void around(): call(* Def.def(..)) { + System.out.println("aspect"); + } +} diff --git a/tests/bugs173/pr407966/Def.java b/tests/bugs173/pr407966/Def.java new file mode 100644 index 000000000..49c6221fd --- /dev/null +++ b/tests/bugs173/pr407966/Def.java @@ -0,0 +1,8 @@ +class Clazz { +} + +public class Def { + public static void def(Clazz[] c) { + System.out.println("def"); + } +} diff --git a/tests/bugs173/pr407966/Use.java b/tests/bugs173/pr407966/Use.java new file mode 100644 index 000000000..2a21d396c --- /dev/null +++ b/tests/bugs173/pr407966/Use.java @@ -0,0 +1,5 @@ +public class Use { + public static void main(String[] argv) { + Def.def(null); + } +} diff --git a/tests/bugs173/pr408014/Code.java b/tests/bugs173/pr408014/Code.java new file mode 100644 index 000000000..ee219c4a0 --- /dev/null +++ b/tests/bugs173/pr408014/Code.java @@ -0,0 +1,6 @@ + +class Code implements Foo.Intface {} + +class Foo { + interface Intface {} +} diff --git a/tests/bugs173/pr408014/IIdentifiable.java2 b/tests/bugs173/pr408014/IIdentifiable.java2 new file mode 100644 index 000000000..b54cdeec4 --- /dev/null +++ b/tests/bugs173/pr408014/IIdentifiable.java2 @@ -0,0 +1,5 @@ +interface IIdentifiable { + UUID getPlatformId(); + void setPlatformId(UUID id); +} + diff --git a/tests/bugs173/pr408014/IdentifiableMixin.java b/tests/bugs173/pr408014/IdentifiableMixin.java new file mode 100644 index 000000000..fe9e45f81 --- /dev/null +++ b/tests/bugs173/pr408014/IdentifiableMixin.java @@ -0,0 +1,24 @@ +import org.aspectj.lang.annotation.*; + +public class IdentifiableMixin implements MyAspect.IIdentifiable { + + private String id; + + public String getPlatformId() { + return id; + } + +} + +@Aspect +class MyAspect { + + public interface IIdentifiable { + String getPlatformId(); + } + + @DeclareMixin("!is(InterfaceType) && !is(EnumType)") + public static IIdentifiable createIIdentifiable() { + return new IdentifiableMixin(); + } +} diff --git a/tests/bugs173/pr408014/MyAspect.java b/tests/bugs173/pr408014/MyAspect.java new file mode 100644 index 000000000..5643dbeed --- /dev/null +++ b/tests/bugs173/pr408014/MyAspect.java @@ -0,0 +1,17 @@ +import org.aspectj.lang.annotation.*; + +@Aspect +class MyAspect { + + public interface IIdentifiable { + UUID getPlatformId(); + void setPlatformId(UUID id); + } + + @DeclareMixin("!is(InterfaceType) && !is(EnumType)") + public static IIdentifiable createIIdentifiable() { + return new IdentifiableMixin(); + } +} + +class UUID {} diff --git a/tests/bugs173/pr408014/UUID.java b/tests/bugs173/pr408014/UUID.java new file mode 100644 index 000000000..320fc1c8c --- /dev/null +++ b/tests/bugs173/pr408014/UUID.java @@ -0,0 +1 @@ +class UUID {} diff --git a/tests/src/org/aspectj/systemtest/ajc173/Ajc173Tests.java b/tests/src/org/aspectj/systemtest/ajc173/Ajc173Tests.java index de3437baf..774cb84e6 100644 --- a/tests/src/org/aspectj/systemtest/ajc173/Ajc173Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc173/Ajc173Tests.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012 Contributors + * Copyright (c) 2013 Contributors * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -24,6 +24,18 @@ import org.aspectj.testing.XMLBasedAjcTestCase; */ public class Ajc173Tests extends org.aspectj.testing.XMLBasedAjcTestCase { + public void testAddRemoveAnnos_407739() throws Exception { + runTest("add remove annos"); + } + +// public void testOrdering_407966() throws Exception { +// runTest("ordering"); +// } +// +// public void testInnerInterfaceMixin_408014() throws Exception { +// runTest("inner interface mixin"); +// } + public void testClassAnnoValue_405016_1() throws Exception { // test that class literals allowed runTest("class anno value 1"); @@ -57,189 +69,6 @@ public class Ajc173Tests extends org.aspectj.testing.XMLBasedAjcTestCase { assertEquals(1,ags.length); assertEquals("LFoo;",ags[0].getTypeSignature()); } - - // still broken! -// public void testDeclareAnnoOnItd2() throws Exception { -// runTest("declare anno on itd 2"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"C"); -// Method m = getMethodStartsWith(jc, "getName"); -// assertNotNull(m); -// AnnotationGen[] ags = m.getAnnotations(); -// for (int i=0;iLjava/lang/Object;Ljava/io/Serializable;", sss); -// -// jc = getClassFrom(ajc.getSandboxDirectory(),"Bug$ClassA"); -// sss = jc.getSignatureAttribute().getSignature(); -// assertEquals("Ljava/lang/Object;Ljava/io/Serializable;", sss); -// } -// -// // extends -// public void testPSignatures_pr399590() throws Exception { -// runTest("p signatures 1"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"Cage"); -// String sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";>;>LBar;", sss); -// jc = getClassFrom(ajc.getSandboxDirectory(),"Cage2"); -// sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";>;>LBar2;Ljava/io/Serializable;", sss); -// } -// -// // extends two classes -// public void testPSignatures_pr399590_2() throws Exception { -// runTest("p signatures 2"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"Cage"); -// String sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";LIntf;>;Q:Ljava/lang/Object;>LBar;", sss); -// jc = getClassFrom(ajc.getSandboxDirectory(),"Cage2"); -// sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";LIntf2;>;Q:Ljava/lang/Object;>LBar2;Ljava/io/Serializable;", sss); -// } -// -// // super -// public void testPSignatures_pr399590_3() throws Exception { -// runTest("p signatures 3"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"Cage"); -// String sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";>;>LBar;", sss); -// jc = getClassFrom(ajc.getSandboxDirectory(),"Cage2"); -// sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";>;>LBar2;Ljava/io/Serializable;", sss); -// } -// -// // super -// public void testPSignatures_pr399590_4() throws Exception { -// runTest("p signatures 4"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"Cage"); -// String sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";LYYY;>;>LBar;", sss); -// jc = getClassFrom(ajc.getSandboxDirectory(),"Cage2"); -// sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";LYYY2;>;>LBar2;Ljava/io/Serializable;", sss); -// } -// -// // unbound -// public void testPSignatures_pr399590_5() throws Exception { -// runTest("p signatures 5"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"Cage"); -// String sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";>LBar;", sss); -// jc = getClassFrom(ajc.getSandboxDirectory(),"Cage2"); -// sss = jc.getSignatureAttribute().getSignature(); -// assertEquals(";>LBar2;Ljava/io/Serializable;", sss); -// } -// -// public void testIfPointcutNames_pr398246() throws Exception { -// runTest("if pointcut names"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); -// Method m = getMethodStartsWith(jc, "ajc$if"); -// assertEquals("ajc$if$andy", m.getName()); -// } -// -// public void testIfPointcutNames_pr398246_2() throws Exception { -// runTest("if pointcut names 2"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); -// Method m = getMethodStartsWith(jc, "ajc$if"); -// assertEquals("ajc$if$fred", m.getName()); -// } -// -// // fully qualified annotation name is used -// public void testIfPointcutNames_pr398246_3() throws Exception { -// runTest("if pointcut names 3"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); -// Method m = getMethodStartsWith(jc, "ajc$if"); -// assertEquals("ajc$if$barney", m.getName()); -// } -// -// // compiling a class later than the initial build - does it pick up the -// // right if clause name? -// public void testIfPointcutNames_pr398246_4() throws Exception { -// runTest("if pointcut names 4"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); -// Method m = getMethodStartsWith(jc, "ajc$if"); -// assertEquals("ajc$if$sid", m.getName()); -// } -// -// // new style generated names -// public void testIfPointcutNames_pr398246_5() throws Exception { -// runTest("if pointcut names 5"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); -// Method m = getMethodStartsWith(jc, "ajc$if"); -// assertEquals("ajc$if$ac0cb804", m.getName()); -// -// jc = getClassFrom(ajc.getSandboxDirectory(), "X2"); -// m = getMethodStartsWith(jc, "ajc$if"); -// assertEquals("ajc$if$ac0cb804", m.getName()); -// } -// -// // new style generated names - multiple ifs in one pointcut -// public void testIfPointcutNames_pr398246_6() throws Exception { -// runTest("if pointcut names 6"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); -// Method m = getMethodStartsWith(jc, "ajc$if",1); -// assertEquals("ajc$if$aac93da8", m.getName()); -// m = getMethodStartsWith(jc, "ajc$if",2); -// assertEquals("ajc$if$1$ae5e778a", m.getName()); -// } -// -// // new style generated names - multiple ifs in one advice -// public void testIfPointcutNames_pr398246_7() throws Exception { -// runTest("if pointcut names 7"); -// JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); -// Method m = getMethodStartsWith(jc, "ajc$if",1); -// assertEquals("ajc$if$1$ac0607c", m.getName()); -// m = getMethodStartsWith(jc, "ajc$if",2); -// assertEquals("ajc$if$1$1$4d4baf36", m.getName()); -// } -// -// public void testOptionalAspects_pr398588() { -// runTest("optional aspects"); -// } -// -// public void testInconsistentClassFile_pr389750() { -// runTest("inconsistent class file"); -// } -// -// public void testInconsistentClassFile_pr389750_2() { -// runTest("inconsistent class file 2"); -// } -// -// public void testInconsistentClassFile_pr389750_3() { -// runTest("inconsistent class file 3"); -// } -// -// public void testInconsistentClassFile_pr389750_4() { -// runTest("inconsistent class file 4"); -// } -// -// public void testAnnotationValueError_pr389752_1() { -// runTest("annotation value error 1"); -// } -// -// public void testAnnotationValueError_pr389752_2() { -// runTest("annotation value error 2"); -// } -// -// // this needs some cleverness to fix... the annotation value is parsed as a -// // string and then not checked -// // to see if the user is accidentally supplying, for example, an enum value. -// // Due to the use of strings, it -// // is hard to check. The verification code might go here: -// // WildAnnotationTypePattern, line 205 (the string case) -// // public void testAnnotationValueError_pr389752_3() { -// // runTest("annotation value error 3"); -// // } // --- diff --git a/tests/src/org/aspectj/systemtest/ajc173/ajc173.xml b/tests/src/org/aspectj/systemtest/ajc173/ajc173.xml index bcbfa7b96..f882d71c7 100644 --- a/tests/src/org/aspectj/systemtest/ajc173/ajc173.xml +++ b/tests/src/org/aspectj/systemtest/ajc173/ajc173.xml @@ -2,6 +2,28 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java index b1534ca73..fbb4d9292 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -1264,6 +1264,26 @@ class BcelClassWeaver implements IClassWeaver { return false; } + /** + * Remove an annotation from the supplied array, if it is in there. + */ + private AnnotationAJ[] removeFromAnnotationsArray(AnnotationAJ[] annotations,AnnotationAJ annotation) { + for (int i=0;i