tests (?) and fixes for 152873 - optimized check for @AJ aspects in LTW

testing/newsrc/org/aspectj/testing/AntSpec.java

@@ -50,7 +50,8 @@ public class AntSpec implements ITestStep {
             + File.pathSeparator + ".." + File.separator + "loadtime/bin"
             + File.pathSeparator + ".." + File.separator + "loadtime5/bin"
             + File.pathSeparator + ".." + File.separator + "weaver/bin"
-            + File.pathSeparator + ".." + File.separator + "lib/bcel/bcel.jar";
+            + File.pathSeparator + ".." + File.separator + "lib/bcel/bcel.jar"
+    		+ File.pathSeparator + ".." + File.separator + "lib/asm/asm-aj.jar";
 
 
     private boolean m_verbose = false;

tests/java5/ataspectj/ataspectj/bugs/NotAspect.java

@@ -0,0 +1,6 @@
+package ataspectj.bugs;
+
+@Deprecated
+public class NotAspect {
+	public static void main(String argz[]) {}
+}

tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhenaspectexcludedtest.xml

@@ -0,0 +1,9 @@
+<aspectj>
+    <weaver options="-1.5 -showWeaveInfo">
+        <exclude within="ataspectj..*"/>
+    </weaver>
+    <aspects>
+        <aspect name="ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.TestAspect"/>
+        <aspect name="ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest.TestAspectForAspect"/>
+    </aspects>
+</aspectj>

tests/java5/ataspectj/ataspectj/bugs/aop-aspectofwhennonaspectexcludedtest.xml

@@ -0,0 +1,8 @@
+<aspectj>
+    <weaver options="-1.5 -verbose">
+        <exclude within="ataspectj..*"/>
+    </weaver>
+    <aspects>
+        <aspect name="ataspectj.bugs.NotAspect"/>
+    </aspects>
+</aspectj>

tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java

@@ -201,6 +201,13 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase {
         runTest("AspectOfWhenAspectNotInInclude");
     }
 
+    public void testAspectOfWhenAspectExcluded_pr152873() {
+        runTest("AspectOfWhenAspectExcluded");
+    }
+    public void testAspectOfWhenNonAspectExcluded_pr152873() {
+    	runTest("AspectOfWhenNonAspectExcluded");
+    }
+
     public void testAppContainer() {
         runTest("AppContainer");
     }

tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml

@@ -298,6 +298,20 @@
         <run class="ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest" ltw="ataspectj/bugs/aop-aspectofwhenaspectnotinincludetest.xml"/>
     </ajc-test>
 
+    <ajc-test dir="java5/ataspectj" title="AspectOfWhenAspectExcluded">
+        <compile
+            files="ataspectj/bugs/AspectOfWhenAspectNotInIncludeTest.java,ataspectj/TestHelper.java"
+            options="-1.5 -XterminateAfterCompilation"/>
+        <run class="ataspectj.bugs.AspectOfWhenAspectNotInIncludeTest" ltw="ataspectj/bugs/aop-aspectofwhenaspectexcludedtest.xml"/>
+    </ajc-test>
+
+    <ajc-test dir="java5/ataspectj" title="AspectOfWhenNonAspectExcluded">
+        <compile
+            files="ataspectj/bugs/NotAspect.java"
+            options="-1.5 -XterminateAfterCompilation"/>
+        <run class="ataspectj.bugs.NotAspect" ltw="ataspectj/bugs/aop-aspectofwhennonaspectexcludedtest.xml"/>
+    </ajc-test>
+
     <ajc-test dir="java5/ataspectj" title="AppContainer">
         <compile
             files="ataspectj/hierarchy/AppContainerTest.java,ataspectj/hierarchy/app/SubApp.java,ataspectj/TestHelper.java"

weaver/src/org/aspectj/weaver/World.java

@@ -1202,4 +1202,8 @@ public abstract class World implements Dump.INode {
 	    	if (trace.isTraceEnabled()) trace.exit("setSynchronizationPointcutsInUse");
 	    }
 	    public boolean areSynchronizationPointcutsInUse() {return synchronizationPointcutsInUse;}
+	    
+	    public boolean isASMAround() { 
+	    	return isASMAround;
+	    }
 }

weaver/src/org/aspectj/weaver/tools/IsAtAspectAnnotationVisitor.java

@@ -0,0 +1,148 @@
+/* *******************************************************************
+ * Copyright (c) 2006 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://www.eclipse.org/legal/epl-v10.html 
+ *  
+ * Contributors: 
+ *     Eugene Kuleshov, Ron Bodkin    initial implementation 
+ * ******************************************************************/
+package org.aspectj.weaver.tools;
+
+import org.aspectj.org.objectweb.asm.AnnotationVisitor;
+import org.aspectj.org.objectweb.asm.Attribute;
+import org.aspectj.org.objectweb.asm.ClassVisitor;
+import org.aspectj.org.objectweb.asm.FieldVisitor;
+import org.aspectj.org.objectweb.asm.Label;
+import org.aspectj.org.objectweb.asm.MethodVisitor;
+
+// should extend EmptyVisitor but it's not currently included with AspectJ's ASM...
+public class IsAtAspectAnnotationVisitor implements ClassVisitor, FieldVisitor,
+		MethodVisitor, AnnotationVisitor {
+	private boolean isAspect = false;
+
+	public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
+		if ("Lorg/aspectj/lang/annotation/Aspect;".equals(desc)) {
+			isAspect = true;
+		}
+		return this;
+	}
+
+	public FieldVisitor visitField(int access, String name, String desc,
+			String signature, Object value) {
+		return null;
+	}
+
+	public MethodVisitor visitMethod(int access, String name, String desc,
+			String signature, String[] exceptions) {
+		return null;
+	}
+
+	public boolean isAspect() {
+		return isAspect;
+	}
+
+	public void visit(int version, int access, String name, String signature,
+			String superName, String[] interfaces) {
+	}
+
+	public void visitSource(String source, String debug) {
+	}
+
+	public void visitOuterClass(String owner, String name, String desc) {
+	}
+
+	public void visitAttribute(Attribute attr) {
+	}
+
+	public void visitInnerClass(String name, String outerName,
+			String innerName, int access) {
+	}
+
+	public void visitEnd() {
+	}
+
+	public AnnotationVisitor visitAnnotationDefault() {
+		return this;
+	}
+
+	public AnnotationVisitor visitParameterAnnotation(int parameter,
+			String desc, boolean visible) {
+		return this;
+	}
+
+	public void visitCode() {
+	}
+
+	public void visitInsn(int opcode) {
+	}
+
+	public void visitIntInsn(int opcode, int operand) {
+	}
+
+	public void visitVarInsn(int opcode, int var) {
+	}
+
+	public void visitTypeInsn(int opcode, String desc) {
+	}
+
+	public void visitFieldInsn(int opcode, String owner, String name,
+			String desc) {
+	}
+
+	public void visitMethodInsn(int opcode, String owner, String name,
+			String desc) {
+	}
+
+	public void visitJumpInsn(int opcode, Label label) {
+	}
+
+	public void visitLabel(Label label) {
+	}
+
+	public void visitLdcInsn(Object cst) {
+	}
+
+	public void visitIincInsn(int var, int increment) {
+	}
+
+	public void visitTableSwitchInsn(int min, int max, Label dflt,
+			Label labels[]) {
+	}
+
+	public void visitLookupSwitchInsn(Label dflt, int keys[], Label labels[]) {
+	}
+
+	public void visitMultiANewArrayInsn(String desc, int dims) {
+	}
+
+	public void visitTryCatchBlock(Label start, Label end, Label handler,
+			String type) {
+	}
+
+	public void visitLocalVariable(String name, String desc, String signature,
+			Label start, Label end, int index) {
+	}
+
+	public void visitLineNumber(int line, Label start) {
+	}
+
+	public void visitMaxs(int maxStack, int maxLocals) {
+	}
+
+	public void visit(String name, Object value) {
+	}
+
+	public void visitEnum(String name, String desc, String value) {
+	}
+
+	public AnnotationVisitor visitAnnotation(String name, String desc) {
+		return this;
+	}
+
+	public AnnotationVisitor visitArray(String name) {
+		return this;
+	}
+}

weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java

@@ -37,6 +37,7 @@ import org.aspectj.bridge.MessageUtil;
 import org.aspectj.bridge.MessageWriter;
 import org.aspectj.bridge.Version;
 import org.aspectj.bridge.IMessage.Kind;
+import org.aspectj.org.objectweb.asm.ClassReader;
 import org.aspectj.util.FileUtil;
 import org.aspectj.util.LangUtil;
 import org.aspectj.weaver.IClassFileProvider;
@@ -212,16 +213,25 @@ public class WeavingAdaptor {
 			try {
 				delegateForCurrentClass=null; 
 		    	if (trace.isTraceEnabled()) trace.enter("weaveClass",this,new Object[] {name,bytes});
-				if (shouldWeave(name, bytes)) {
-					info("weaving '" + name + "'");
-					bytes = getWovenBytes(name, bytes);
-				} else if (shouldWeaveAnnotationStyleAspect(name, bytes)) {
-		            // an @AspectJ aspect needs to be at least munged by the aspectOf munger
-		            info("weaving '" + name + "'");
-		            bytes = getAtAspectJAspectBytes(name, bytes);
-		        }
-				else {
-					info("not weaving '" + name + "'");
+				name = name.replace('/','.');
+				if (couldWeave(name, bytes)) {
+			        if (accept(name, bytes)) {
+			            // TODO @AspectJ problem
+			            // Annotation style aspects need to be included regardless in order to get
+			            // a valid aspectOf()/hasAspect() generated in them.  However - if they are excluded
+			            // (via include/exclude in aop.xml) they really should only get aspectOf()/hasAspect()
+			            // and not be included in the full set of aspects being applied by 'this' weaver
+						info("weaving '" + name + "'");
+						bytes = getWovenBytes(name, bytes);
+					} else if (shouldWeaveAnnotationStyleAspect(name, bytes)) {
+			            // an @AspectJ aspect needs to be at least munged by the aspectOf munger
+			            info("weaving '" + name + "'");
+			            bytes = getAtAspectJAspectBytes(name, bytes);
+					} else {
+						info("not weaving '" + name + "'");
+					}
+		        } else {
+					info("cannot weave '" + name + "'");
 				}
 	
 				if (trace.isTraceEnabled()) trace.exit("weaveClass",bytes);
@@ -235,17 +245,10 @@ public class WeavingAdaptor {
 
     /**
      * @param name
-     * @return true if should weave (but maybe we still need to munge it for @AspectJ aspectof support)
+     * @return true if even valid to weave: either with an accept check or to munge it for @AspectJ aspectof support
      */
-    private boolean shouldWeave (String name, byte[] bytes) {
-		name = name.replace('/','.');
-		boolean b = !generatedClasses.containsKey(name) && shouldWeaveName(name);
-        return b && accept(name, bytes);
-//        && shouldWeaveAnnotationStyleAspect(name);
-//        // we recall shouldWeaveAnnotationStyleAspect 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) || shouldWeaveAnnotationStyleAspect(name));
+    private boolean couldWeave (String name, byte[] bytes) {
+		return !generatedClasses.containsKey(name) && shouldWeaveName(name);
 	}
 
     //ATAJ
@@ -276,12 +279,29 @@ public class WeavingAdaptor {
      * @return true if @Aspect
      */
 	private boolean shouldWeaveAnnotationStyleAspect(String name, byte[] bytes) {
-		// AV: instead of doing resolve that would lookup stuff on disk thru BCEL ClassLoaderRepository
-        // we reuse bytes[] here to do a fast lookup for @Aspect annotation
-		ensureDelegateInitialized(name,bytes);
+    	if (delegateForCurrentClass==null) {
+    		if (weaver.getWorld().isASMAround()) return asmCheckAnnotationStyleAspect(bytes);
+    		else ensureDelegateInitialized(name, bytes);
+    	}
 		return (delegateForCurrentClass.isAnnotationStyleAspect());
 	}
 
+	private boolean asmCheckAnnotationStyleAspect(byte[] bytes) {
+		IsAtAspectAnnotationVisitor detector = new IsAtAspectAnnotationVisitor();
+
+		ClassReader cr = new ClassReader(bytes);
+	    try {
+	    	cr.accept(detector, true);//, ClassReader.SKIP_DEBUG | ClassReader.SKIP_CODE | ClassReader.SKIP_FRAMES);
+	    } catch (Exception spe) {
+	    	// if anything goes wrong, e.g., an NPE, then assume it's NOT an @AspectJ aspect...
+	    	System.err.println("Unexpected problem parsing bytes to discover @Aspect annotation");
+	    	spe.printStackTrace();
+	    	return false;
+	    }
+	    
+	    return detector.isAspect();
+	}
+	
     protected void ensureDelegateInitialized(String name,byte[] bytes) {
     	if (delegateForCurrentClass==null)
     	  delegateForCurrentClass =  ((BcelWorld)weaver.getWorld()).addSourceObjectType(Utility.makeJavaClass(name, bytes));