fix for #113587: when aop.xml include/exclude is used, javac @Aspect are not passed thru the aspectof munger. Added custom logic for that

11 files changed, 251 insertions, 39 deletions

diff --git a/bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java b/bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java
index f99146864..66da84a1b 100644
--- a/bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java
+++ b/bridge/src/org/aspectj/bridge/context/CompilationAndWeavingContext.java
@@ -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();
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
index 85b718ac5..fb30edd6e 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
@@ -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
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java
index 0511929fa..36f926360 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/WeaverAdapter.java
@@ -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
index 000000000..5d4437eef
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java
@@ -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
index 000000000..d45adb36a
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml
@@ -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
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
index d0a88a27e..49e9e7850 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
@@ -101,4 +101,9 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase {
     public void testConcreteAspect() {
         runTest("ConcreteAspect");
     }
+
+    public void testAspectOfWhenAspectNotInInclude() {
+        runTest("AspectOfWhenAspectNotInInclude");
+    }
+
 }
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
index d3bf28f77..257d56805 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
@@ -147,4 +147,10 @@
         <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
diff --git a/weaver/src/org/aspectj/weaver/IClassFileProvider.java b/weaver/src/org/aspectj/weaver/IClassFileProvider.java
index 73700e0bb..e0074c3d8 100644
--- a/weaver/src/org/aspectj/weaver/IClassFileProvider.java
+++ b/weaver/src/org/aspectj/weaver/IClassFileProvider.java
@@ -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();
+
 }
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java b/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java
index 79427eb4c..e1ba9529e 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java
@@ -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.
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index 6ab999ec3..fc35718cb 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -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
diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
index d0d46db81..c98773e3b 100644
--- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
+++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
@@ -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();
 		}