]> source.dussan.org Git - aspectj.git/commitdiff
fix for #113587: when aop.xml include/exclude is used, javac @Aspect are not passed...
authoravasseur <avasseur>
Tue, 25 Oct 2005 15:36:26 +0000 (15:36 +0000)
committeravasseur <avasseur>
Tue, 25 Oct 2005 15:36:26 +0000 (15:36 +0000)
bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java
loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java
tests/java5/ataspectj/ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java [new file with mode: 0644]
tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml [new file with mode: 0644]
tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
weaver/src/org/aspectj/weaver/IClassFileProvider.java
weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java
weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java

index f99146864c424013f5ea22ccf5975c250d6dd951..66da84a1bdb5b3975f264d6482c8296cfe7025ea 100644 (file)
@@ -73,9 +73,10 @@ public class CompilationAndWeavingContext {
        public static final int IMPLEMENTING_ON_SHADOW = 29;
        public static final int MATCHING_POINTCUT = 30;
        public static final int MUNGING_WITH = 31;
-       
-       
-       // phase names
+    public static final int PROCESSING_ATASPECTJTYPE_MUNGERS_ONLY = 32;
+
+
+    // phase names
        public static final String[] PHASE_NAMES = new String[] {
                "batch building",
                "incrementally building",
@@ -111,8 +112,9 @@ public class CompilationAndWeavingContext {
                "matching shadow",
                "implementing on shadow",
                "matching pointcut",
-               "type munging with"
-       };
+               "type munging with",
+        "type munging for @AspectJ aspectOf"
+    };
        
        // context stacks, one per thread
        private static Map contextMap = new HashMap();
index 85b718ac593a10c28acc4adf7cac77cecf137e96..fb30edd6e7b114376cecc67e885748c5eabdb7a8 100644 (file)
@@ -299,7 +299,7 @@ public class ConcreteAspectCodeGen {
                 ResolvedType.forName(m_concreteAspect.name).resolve(m_world),
                 m_perClause.getKind()
         );
-        perClauseMunger.forceMunge(cg);
+        perClauseMunger.forceMunge(cg, false);
 
         //TODO AV - unsafe cast
         // register the fresh new class into the world repository as it does not exist on the classpath anywhere
index 0511929fad36f6447e819b591df5d6af6ab055a2..36f9263603070ab4a3881cf6dfbbbd66576f5269 100644 (file)
@@ -71,8 +71,12 @@ public class WeaverAdapter implements IClassFileProvider, IWeaveRequestor, Itera
        public IWeaveRequestor getRequestor() {
                return this;
        }
-       
-       // Iteration
+
+    public boolean isApplyAtAspectJMungersOnly() {
+        return false;
+    }
+
+    // Iteration
        // ================================================================
        
        /* (non-Javadoc)
diff --git a/tests/java5/ataspectj/ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java b/tests/java5/ataspectj/ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java
new file mode 100644 (file)
index 0000000..5d4437e
--- /dev/null
@@ -0,0 +1,86 @@
+/*******************************************************************************
+ * Copyright (c) 2005 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 
+ * http://eclipse.org/legal/epl-v10.html 
+ * 
+ * Contributors:
+ *   Alexandre Vasseur         initial implementation
+ *******************************************************************************/
+package ataspectj.bugs;
+
+import junit.framework.TestCase;
+import junit.framework.Test;
+import junit.framework.TestSuite;
+import ataspectj.TestHelper;
+import org.aspectj.lang.annotation.Before;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.DeclareImplements;
+import org.aspectj.lang.Aspects;
+
+import java.io.Serializable;
+import java.lang.reflect.Method;
+
+/**
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class AspectOfWhenAspectNotInIncludeTest extends TestCase {
+
+    static int I;
+
+    void target() {
+        I++;
+    }
+
+    @Aspect
+    static class TestAspectForAspect {
+        @DeclareImplements("ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.TestAspect")
+        Serializable shouldNotHappenDueToInclude;
+    }
+
+    @Aspect("perthis(execution(* ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.target()))")
+    static class TestAspect {
+
+        public TestAspect() {
+            int i = 0;
+            i++;
+        }
+
+        @Before("execution(* ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.target())")
+        public void before() {
+            I++;
+        }
+    }
+
+    public void testInclude() {
+        I = 0;
+        target();
+        assertEquals(1, I);//aspect not applied as per aop.xml include
+    }
+
+    public void testAspectOf() {
+        // aspectOf method has been added to the aspect, no matter the aop.xml include/exclude
+        try {
+            Method m = TestAspect.class.getDeclaredMethod("aspectOf", new Class[]{Object.class});
+        } catch (Throwable t) {
+            fail(t.toString());
+        }
+    }
+
+    public void testAspectUntouched() {
+        if (TestAspect.class.getInterfaces().length > 0) {
+            fail("Aspect was touched by some aspect while NOT in aop.xml include");
+        }
+    }
+
+    public static void main(String[] args) {
+        TestHelper.runAndThrowOnFailure(suite());
+    }
+
+    public static Test suite() {
+        return new TestSuite(AspectOfWhenAspectNotInIncludeTest.class);
+    }
+
+}
diff --git a/tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml b/tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml
new file mode 100644 (file)
index 0000000..d45adb3
--- /dev/null
@@ -0,0 +1,9 @@
+<aspectj>
+    <weaver options="-1.5 -showWeaveInfo">
+        <include within="somethingelse..*"/>
+    </weaver>
+    <aspects>
+        <aspect name="ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.TestAspect"/>
+        <aspect name="ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.TestAspectForAspect"/>
+    </aspects>
+</aspectj>
\ No newline at end of file
index d0a88a27e36b5847291163e07cdb68cf9ea30661..49e9e785018f93f82cc8e6c7cd45e4bab1a2766b 100644 (file)
@@ -101,4 +101,9 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase {
     public void testConcreteAspect() {
         runTest("ConcreteAspect");
     }
+
+    public void testAspectOfWhenAspectNotInInclude() {
+        runTest("AspectOfWhenAspectNotInInclude");
+    }
+
 }
index d3bf28f7720f10990ecbe4205c9a57d1929ba49b..257d56805c1b07d6a86e78e0a9ecac9581c618bd 100644 (file)
         <run class="ataspectj.ConcreteAspectTest" ltw="ataspectj/aop-concreteaspect.xml"/>
     </ajc-test>
 
+    <ajc-test dir="java5/ataspectj" title="AspectOfWhenAspectNotInInclude">
+        <compile
+            files="ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java,ataspectj/TestHelper.java"
+            options="-1.5 -XnoWeave"/>
+        <run class="ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest" ltw="ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml"/>
+    </ajc-test>
 </suite>
\ No newline at end of file
index 73700e0bb518c9ad5ea609485d9107f111702719..e0074c3d8d533dbb8a66a465a7aa9c9e39f0ab93 100644 (file)
@@ -33,4 +33,11 @@ public interface IClassFileProvider {
         * The client to which the woven results should be returned.
         */
        IWeaveRequestor getRequestor();
+
+    /**
+     * @return true if weaver should only do some internal munging as the one needed
+     * for @AspectJ aspectOf methods creation
+     */
+    boolean isApplyAtAspectJMungersOnly();
+
 }
index 79427eb4c023796355dfe2bb72fa7b494d579c6b..e1ba9529e30d8d67932a7822f41eb9788343e8a4 100644 (file)
@@ -56,6 +56,22 @@ public class BcelPerClauseAspectAdder extends BcelTypeMunger {
     public boolean munge(BcelClassWeaver weaver) {
         LazyClassGen gen = weaver.getLazyClassGen();
 
+        doAggressiveInner(gen);
+
+        // Only munge the aspect type
+         if (!gen.getType().equals(aspectType)) {
+             return false;
+         }
+
+        return doMunge(gen, true);
+    }
+
+    public boolean forceMunge(LazyClassGen gen, boolean checkAlreadyThere) {
+        doAggressiveInner(gen);
+        return doMunge(gen, checkAlreadyThere);
+    }
+
+    private void doAggressiveInner(LazyClassGen gen) {
         // agressively generate the inner interface if any
         // Note: we do so because of the bug #75442 that leads to have this interface implemented by all classes and not
         // only those matched by the per clause, which fails under LTW since the very first class
@@ -79,16 +95,13 @@ public class BcelPerClauseAspectAdder extends BcelTypeMunger {
             }
             hasGeneratedInner = true;
         }
-
-        // Only munge the aspect type
-         if (!gen.getType().equals(aspectType)) {
-             return false;
-         }
-
-        return forceMunge(gen);
     }
 
-    public boolean forceMunge(LazyClassGen gen) {
+    private boolean doMunge(LazyClassGen gen, boolean checkAlreadyThere) {
+        if (checkAlreadyThere && hasPerClauseMembersAlready(gen)) {
+            return false;
+        }
+
         generatePerClauseMembers(gen);
 
         if (kind == PerClause.SINGLETON) {
@@ -132,6 +145,23 @@ public class BcelPerClauseAspectAdder extends BcelTypeMunger {
         //return aspectType.equals(onType);
     }
 
+    private boolean hasPerClauseMembersAlready(LazyClassGen classGen) {
+        ResolvedMember[] methods = classGen.getBcelObjectType().getDeclaredMethods();
+        for (int i = 0; i < methods.length; i++) {
+            ResolvedMember method = methods[i];
+            if ("aspectOf".equals(method.getName())) {
+                if ("()".equals(method.getParameterSignature()) && (kind == PerClause.SINGLETON || kind == PerClause.PERCFLOW)) {
+                    return true;
+                } else if ("(Ljava/lang/Object;)".equals(method.getParameterSignature()) && kind == PerClause.PEROBJECT) {
+                    return true;
+                } else if ("(Ljava/lang/Class;)".equals(method.getParameterSignature()) && kind == PerClause.PERTYPEWITHIN) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     private void generatePerClauseMembers(LazyClassGen classGen) {
         //FIXME Alex handle when field already there - or handle it with / similar to isAnnotationDefinedAspect()
         // for that use aspectType and iterate on the fields.
index 6ab999ec3da77206eecce988756401f0f997e3c6..fc35718cb544a5b854059a7a1c620cb2636d18c8 100644 (file)
@@ -858,7 +858,11 @@ public class BcelWeaver implements IWeaver {
        prepareForWeave();
        Collection c = weave( new IClassFileProvider() {
 
-                       public Iterator getClassFileIterator() {
+            public boolean isApplyAtAspectJMungersOnly() {
+                return false;
+            }
+
+            public Iterator getClassFileIterator() {
                                return addedClasses.iterator();
                        }
 
@@ -961,7 +965,37 @@ public class BcelWeaver implements IWeaver {
        Collection wovenClassNames = new ArrayList();
        IWeaveRequestor requestor = input.getRequestor();
 
-       requestor.processingReweavableState();
+        // special case for AtAspectJMungerOnly - see #113587
+        if (input.isApplyAtAspectJMungersOnly()) {
+            ContextToken atAspectJMungersOnly = CompilationAndWeavingContext.enteringPhase(CompilationAndWeavingContext.PROCESSING_ATASPECTJTYPE_MUNGERS_ONLY, "");
+            requestor.weavingAspects();
+            ContextToken aspectToken = CompilationAndWeavingContext.enteringPhase(CompilationAndWeavingContext.WEAVING_ASPECTS, "");
+            for (Iterator i = input.getClassFileIterator(); i.hasNext(); ) {
+                UnwovenClassFile classFile = (UnwovenClassFile)i.next();
+                String className = classFile.getClassName();
+                ResolvedType theType = world.resolve(className);
+                if (theType.isAnnotationStyleAspect()) {
+                    BcelObjectType classType = BcelWorld.getBcelObjectType(theType);
+                    if (classType==null) {
+                        throw new BCException("Can't find bcel delegate for "+className+" type="+theType.getClass());
+                    }
+                    LazyClassGen clazz = classType.getLazyClassGen();
+                    BcelPerClauseAspectAdder selfMunger = new BcelPerClauseAspectAdder(theType, theType.getPerClause().getKind());
+                    selfMunger.forceMunge(clazz, true);
+                    classType.finishedWith();
+                    UnwovenClassFile[] newClasses = getClassFilesFor(clazz);
+                    for (int news = 0; news < newClasses.length; news++) {
+                        requestor.acceptResult(newClasses[news]);
+                    }
+                    wovenClassNames.add(classFile.getClassName());
+                }
+            }
+            requestor.weaveCompleted();
+            CompilationAndWeavingContext.leavingPhase(atAspectJMungersOnly);
+            return wovenClassNames;
+        }
+
+        requestor.processingReweavableState();
        ContextToken reweaveToken = CompilationAndWeavingContext.enteringPhase(CompilationAndWeavingContext.PROCESSING_REWEAVABLE_STATE,"");
                prepareToProcessReweavableState();
                // clear all state from files we'll be reweaving
index d0d46db814372ee10ee63f63cc7f7005901157f0..c98773e3b714ab9cd8684facaaa9e8925c957503 100644 (file)
@@ -173,14 +173,28 @@ public class WeavingAdaptor {
             //System.out.println("WeavingAdaptor.weaveClass " + name);
                        info("weaving '" + name + "'");
                        bytes = getWovenBytes(name, bytes);
-               }
-               return bytes;
+               } else if (shouldWeaveAtAspect(name)) {
+            // an @AspectJ aspect needs to be at least munged by the aspectOf munger
+            info("weaving '" + name + "'");
+            bytes = getAtAspectJAspectBytes(name, bytes);
+        }
+
+        return bytes;
        }
 
-       private boolean shouldWeave (String name) {
+    /**
+     * @param name
+     * @return true if should weave (but maybe we still need to munge it for @AspectJ aspectof support)
+     */
+    private boolean shouldWeave (String name) {
                name = name.replace('/','.');
-               boolean b = (enabled && !generatedClasses.containsKey(name) && shouldWeaveName(name) && shouldWeaveAspect(name));
-               return b && accept(name);
+               boolean b = enabled && !generatedClasses.containsKey(name) && shouldWeaveName(name);
+        return b && accept(name);
+//        && shouldWeaveAtAspect(name);
+//        // we recall shouldWeaveAtAspect as we need to add aspectOf methods for @Aspect anyway
+//        //FIXME AV - this is half ok as the aspect will be weaved by others. In theory if the aspect
+//        // is excluded from include/exclude config we should only weave late type mungers for aspectof
+//        return b && (accept(name) || shouldWeaveAtAspect(name));
        }
 
     //ATAJ
@@ -206,11 +220,11 @@ public class WeavingAdaptor {
      * (and not part of the source compilation)
      *
      * @param name
-     * @return
+     * @return true if @Aspect
      */
-       private boolean shouldWeaveAspect(String name) {
+       private boolean shouldWeaveAtAspect(String name) {
                ResolvedType type = bcelWorld.resolve(UnresolvedType.forName(name), true);
-        return (type == null || !type.isAspect() || type.isAnnotationStyleAspect());
+        return (type == null || type.isAnnotationStyleAspect());
        }
 
        /**
@@ -224,18 +238,24 @@ public class WeavingAdaptor {
                WeavingClassFileProvider wcp = new WeavingClassFileProvider(name,bytes);
                weaver.weave(wcp);
                return wcp.getBytes();          
-               
-//             UnwovenClassFile unwoven = new UnwovenClassFile(name,bytes);
-//             
-//             // weave
-//             BcelObjectType bcelType = bcelWorld.addSourceObjectType(unwoven.getJavaClass());
-//             LazyClassGen woven = weaver.weaveWithoutDump(unwoven,bcelType);
-//             
-//             byte[] wovenBytes = woven != null ? woven.getJavaClass(bcelWorld).getBytes() : bytes;
-//             return wovenBytes;
        }
-       
-       private void registerAspectLibraries(List aspectPath) {
+
+    /**
+     * Weave a set of bytes defining a class for only what is needed to turn @AspectJ aspect
+     * in a usefull form ie with aspectOf method - see #113587
+     * @param name the name of the class being woven
+     * @param bytes the bytes that define the class
+     * @return byte[] the woven bytes for the class
+     * @throws IOException
+     */
+    private byte[] getAtAspectJAspectBytes(String name, byte[] bytes) throws IOException {
+        WeavingClassFileProvider wcp = new WeavingClassFileProvider(name,bytes);
+        wcp.setApplyAtAspectJMungersOnly();
+        weaver.weave(wcp);
+        return wcp.getBytes();
+    }
+
+    private void registerAspectLibraries(List aspectPath) {
 //             System.err.println("? WeavingAdaptor.registerAspectLibraries(" + aspectPath + ")");
                for (Iterator i = aspectPath.iterator(); i.hasNext();) {
                        String libName = (String)i.next();
@@ -346,14 +366,23 @@ public class WeavingAdaptor {
 
                private List unwovenClasses = new ArrayList(); /* List<UnovenClassFile> */
                private UnwovenClassFile wovenClass;
+        private boolean isApplyAtAspectJMungersOnly = false;
 
-               public WeavingClassFileProvider (String name, byte[] bytes) {
+        public WeavingClassFileProvider (String name, byte[] bytes) {
                        UnwovenClassFile unwoven = new UnwovenClassFile(name,bytes);
                        unwovenClasses.add(unwoven);
                        bcelWorld.addSourceObjectType(unwoven.getJavaClass());
                }
 
-               public byte[] getBytes () {
+        public void setApplyAtAspectJMungersOnly() {
+            isApplyAtAspectJMungersOnly = true;
+        }
+
+        public boolean isApplyAtAspectJMungersOnly() {
+            return isApplyAtAspectJMungersOnly;
+        }
+
+        public byte[] getBytes () {
                        return wovenClass.getBytes();
                }