LTW weaver/dump@within option + fix a sout in weaver

15 files changed, 175 insertions, 34 deletions

diff --git a/loadtime/src/aspectj_1_5_0.dtd b/loadtime/src/aspectj_1_5_0.dtd
index 86bc43d51..710c0dac8 100644
--- a/loadtime/src/aspectj_1_5_0.dtd
+++ b/loadtime/src/aspectj_1_5_0.dtd
@@ -46,7 +46,7 @@ weaver
     TODO: Note: the scope of the options can be ClassLoader aware but should be assumed JVM wide
 ******************************************************************************************************************************-->
 <!ELEMENT weaver (
-    (include | exclude)*
+    (include | exclude | dump)*
 )>
 <!ATTLIST weaver
     options CDATA #IMPLIED
@@ -78,6 +78,17 @@ A class must be matched by NONE of the exclude elements to be exposed to the wea
     within CDATA #REQUIRED
 >
 <!--*****************************************************************************************************************************
+dump
+*********************************************************************************************************************************
+[dump] control post-weaving dump to the "./_dump" folder (debugging purpose only)
+@within defines a type pattern
+    (it is not a startWith)
+******************************************************************************************************************************-->
+<!ELEMENT dump EMPTY>
+<!ATTLIST dump
+    within CDATA #REQUIRED
+>
+<!--*****************************************************************************************************************************
 aspects
 *********************************************************************************************************************************
 [aspects] defines a set of aspects
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
index 62389bab7..0d17dfdec 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
@@ -51,9 +51,11 @@ public class Aj implements ClassPreProcessor {
         }
 
         try {
-            byte[] weaved = WeaverContainer.getWeaver(loader).weaveClass(className, bytes);
-            //FIXME AV make dump optionnal and configurable
-            __dump(className, weaved);
+            WeavingAdaptor weavingAdaptor = WeaverContainer.getWeaver(loader);
+            byte[] weaved = weavingAdaptor.weaveClass(className, bytes);
+            if (weavingAdaptor.shouldDump(className.replace('/', '.'))) {
+                dump(className, weaved);
+            }
             return weaved;
         } catch (Throwable t) {
             //FIXME AV wondering if we should have the option to fail (throw runtime exception) here
@@ -112,23 +114,22 @@ public class Aj implements ClassPreProcessor {
     }
 
     /**
-     * Dump the given bytcode in _dump/...
+     * Dump the given bytcode in _dump/... (dev mode)
      *
      * @param name
      * @param b
      * @throws Throwable
      */
-    static void __dump(String name, byte[] b) throws Throwable {
-        if (true) return;//FIXME AV have an option
+    static void dump(String name, byte[] b) throws Throwable {
         String className = name.replace('.', '/');
         final File dir;
         if (className.indexOf('/') > 0) {
-            dir = new File("_dump" + File.separator + className.substring(0, className.lastIndexOf('/')));
+            dir = new File("_ajdump" + File.separator + className.substring(0, className.lastIndexOf('/')));
         } else {
-            dir = new File("_dump");
+            dir = new File("_ajdump");
         }
         dir.mkdirs();
-        String fileName = "_dump" + File.separator + className + ".class";
+        String fileName = "_ajdump" + File.separator + className + ".class";
         FileOutputStream os = new FileOutputStream(fileName);
         os.write(b);
         os.close();
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
index b3e361ada..8c90b978b 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
@@ -11,13 +11,6 @@
  *******************************************************************************/
 package org.aspectj.weaver.loadtime;
 
-import java.io.File;
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.Enumeration;
-import java.util.Iterator;
-import java.util.List;
-
 import org.aspectj.asm.IRelationship;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
@@ -35,6 +28,13 @@ import org.aspectj.weaver.patterns.TypePattern;
 import org.aspectj.weaver.tools.GeneratedClassHandler;
 import org.aspectj.weaver.tools.WeavingAdaptor;
 
+import java.io.File;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.Iterator;
+import java.util.List;
+
 /**
  * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
  */
@@ -42,19 +42,10 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
 
     private final static String AOP_XML = "META-INF/aop.xml";
 
-    //ATAJ LTW include/exclude
+    private List m_dumpTypePattern = new ArrayList();
     private List m_includeTypePattern = new ArrayList();
     private List m_excludeTypePattern = new ArrayList();
     private List m_aspectExcludeTypePattern = new ArrayList();
-    public void addIncludeTypePattern(TypePattern typePattern) {
-        m_includeTypePattern.add(typePattern);
-    }
-    public void addExcludeTypePattern(TypePattern typePattern) {
-        m_excludeTypePattern.add(typePattern);
-    }
-    public void addAspectExcludeTypePattern(TypePattern typePattern) {
-        m_aspectExcludeTypePattern.add(typePattern);
-    }
 
     public ClassLoaderWeavingAdaptor(final ClassLoader loader) {
         super(null);// at this stage we don't have yet a generatedClassHandler to define to the VM the closures
@@ -68,7 +59,9 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             public void acceptClass(String name, byte[] bytes) {
                 //TODO av make dump configurable
                 try {
-                    Aj.__dump(name, bytes);
+                    if (shouldDump(name.replace('/', '.'))) {
+                        Aj.dump(name, bytes);
+                    }
                 } catch (Throwable throwable) {
                     throwable.printStackTrace();
                 }
@@ -130,6 +123,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             registerAspectExclude(weaver, loader, definitions);
             registerAspects(weaver, loader, definitions);
             registerIncludeExclude(weaver, loader, definitions);
+            registerDump(weaver, loader, definitions);
         } catch (Exception e) {
             weaver.getWorld().getMessageHandler().handleMessage(
                     new Message("Register definition failed", IMessage.WARNING, e, null)
@@ -161,7 +155,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         world.setXlazyTjp(weaverOption.lazyTjp);
         weaver.setReweavableMode(weaverOption.reWeavable, false);
         world.setXnoInline(weaverOption.noInline);
-        world.setBehaveInJava5Way(weaverOption.java5);
+        world.setBehaveInJava5Way(weaverOption.java5);//TODO should be autodetected ?
         //TODO proceedOnError option
     }
 
@@ -227,6 +221,24 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         }
     }
 
+    /**
+     * Register the dump filter
+     *
+     * @param weaver
+     * @param loader
+     * @param definitions
+     */
+    private void registerDump(final BcelWeaver weaver, final ClassLoader loader, final List definitions) {
+        for (Iterator iterator = definitions.iterator(); iterator.hasNext();) {
+            Definition definition = (Definition) iterator.next();
+            for (Iterator iterator1 = definition.getDumpPatterns().iterator(); iterator1.hasNext();) {
+                String dump = (String) iterator1.next();
+                TypePattern pattern = new PatternParser(dump).parseTypePattern();
+                m_dumpTypePattern.add(pattern);
+            }
+        }
+    }
+
     protected boolean accept(String className) {
         // avoid ResolvedType if not needed
         if (m_excludeTypePattern.isEmpty() && m_includeTypePattern.isEmpty()) {
@@ -244,7 +256,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         }
         for (Iterator iterator = m_includeTypePattern.iterator(); iterator.hasNext();) {
             TypePattern typePattern = (TypePattern) iterator.next();
-            if (! typePattern.matchesStatically(classInfo)) {
+            if (!typePattern.matchesStatically(classInfo)) {
                 // include does not match - skip
                 return false;
             }
@@ -270,4 +282,21 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         return true;
     }
 
+    public boolean shouldDump(String className) {
+        // avoid ResolvedType if not needed
+        if (m_dumpTypePattern.isEmpty()) {
+            return false;
+        }
+        //TODO AV - optimize for className.startWith only
+        ResolvedTypeX classInfo = weaver.getWorld().getCoreType(TypeX.forName(className));
+        //dump
+        for (Iterator iterator = m_dumpTypePattern.iterator(); iterator.hasNext();) {
+            TypePattern typePattern = (TypePattern) iterator.next();
+            if (typePattern.matchesStatically(classInfo)) {
+                // dump match
+                return true;
+            }
+        }
+        return false;
+    }
 }
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
index e67e21f0f..73677e60f 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
@@ -23,6 +23,8 @@ public class Definition {
 
     private StringBuffer m_weaverOptions;
 
+    private List m_dumpPatterns;
+
     private List m_includePatterns;
 
     private List m_excludePatterns;
@@ -35,6 +37,7 @@ public class Definition {
 
     public Definition() {
         m_weaverOptions = new StringBuffer();
+        m_dumpPatterns = new ArrayList(0);
         m_includePatterns = new ArrayList(0);
         m_excludePatterns = new ArrayList(0);
         m_aspectClassNames = new ArrayList();
@@ -46,6 +49,10 @@ public class Definition {
         return m_weaverOptions.toString();
     }
 
+    public List getDumpPatterns() {
+        return m_dumpPatterns;
+    }
+
     public List getIncludePatterns() {
         return m_includePatterns;
     }
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
index 79faf54f4..9dbbfa18f 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
@@ -47,6 +47,7 @@ public class DocumentParser extends DefaultHandler {
 
     private final static String ASPECTJ_ELEMENT = "aspectj";
     private final static String WEAVER_ELEMENT = "weaver";
+    private final static String DUMP_ELEMENT = "dump";
     private final static String INCLUDE_ELEMENT = "include";
     private final static String EXCLUDE_ELEMENT = "exclude";
     private final static String OPTIONS_ATTRIBUTE = "options";
@@ -59,7 +60,6 @@ public class DocumentParser extends DefaultHandler {
     private final static String WITHIN_ATTRIBUTE = "within";
     private final static String EXPRESSION_ATTRIBUTE = "expression";
 
-
     private final Definition m_definition;
 
     private boolean m_inAspectJ;
@@ -176,6 +176,11 @@ public class DocumentParser extends DefaultHandler {
             if (!isNull(typePattern)) {
                 m_definition.getExcludePatterns().add(typePattern);
             }
+        } else if (DUMP_ELEMENT.equals(qName) && m_inWeaver) {
+            String typePattern = attributes.getValue(WITHIN_ATTRIBUTE);
+            if (!isNull(typePattern)) {
+                m_definition.getDumpPatterns().add(typePattern);
+            }
         } else if (EXCLUDE_ELEMENT.equals(qName) && m_inAspects) {
             String typePattern = attributes.getValue(WITHIN_ATTRIBUTE);
             if (!isNull(typePattern)) {
diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/test/DocumentParserTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/test/DocumentParserTest.java
index c0f26ab99..d8204d263 100644
--- a/loadtime/testsrc/org/aspectj/weaver/loadtime/test/DocumentParserTest.java
+++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/test/DocumentParserTest.java
@@ -37,7 +37,7 @@ public class DocumentParserTest extends TestCase {
 
         assertEquals("foo..bar.Goo+", def.getIncludePatterns().get(0));
         assertEquals("@Baz", def.getAspectExcludePatterns().get(0));
-
+        assertEquals("foo..*", def.getDumpPatterns().get(0));
     }
 
 }
diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/test/simpleWithDtd.xml b/loadtime/testsrc/org/aspectj/weaver/loadtime/test/simpleWithDtd.xml
index 03b77ff3c..ef4946e19 100644
--- a/loadtime/testsrc/org/aspectj/weaver/loadtime/test/simpleWithDtd.xml
+++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/test/simpleWithDtd.xml
@@ -3,6 +3,7 @@
 <aspectj>
     <weaver options="-showWeaveInfo">
         <include within="foo..bar.Goo+"/>
+        <dump within="foo..*"/>
     </weaver>
     <aspects>
         <exclude within="@Baz"/>
diff --git a/tests/java5/ataspectj/ajc-ant.xml b/tests/java5/ataspectj/ajc-ant.xml
index 0703756e5..e4d4008eb 100644
--- a/tests/java5/ataspectj/ajc-ant.xml
+++ b/tests/java5/ataspectj/ajc-ant.xml
@@ -51,5 +51,13 @@
         </java>
     </target>
 
+    <target name="ltw.DumpTest">
+        <java fork="yes" classname="ataspectj.DumpTest" failonerror="yes">
+            <classpath refid="aj.path"/>
+            <jvmarg value="-javaagent:${aj.root}/lib/test/loadtime5.jar"/>
+            <jvmarg value="-Daj5.def=ataspectj/aop-dump.xml"/>
+        </java>
+    </target>
+
     <target name="javac.ltw" depends="compile:javac, ltw"/>
 </project>
\ No newline at end of file
diff --git a/tests/java5/ataspectj/ataspectj/DumpTest.java b/tests/java5/ataspectj/ataspectj/DumpTest.java
new file mode 100644
index 000000000..58031604c
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/DumpTest.java
@@ -0,0 +1,40 @@
+/*******************************************************************************
+ * 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;
+
+import junit.framework.TestCase;
+
+import java.io.File;
+
+/**
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class DumpTest extends TestCase {
+
+    public static void main(String[] args) {
+        TestHelper.runAndThrowOnFailure(suite());
+    }
+
+    public static junit.framework.Test suite() {
+        return new junit.framework.TestSuite(DumpTest.class);
+    }
+
+    public void testDump() {
+        File f = new File("_ajdump/ataspectj/DumpTest.class");
+        assertFalse(f.exists());
+
+        DumpTestTheDump forceLoad = new DumpTestTheDump();
+        f = new File("_ajdump/ataspectj/DumpTestTheDump.class");
+        assertTrue(f.exists());
+    }
+
+}
diff --git a/tests/java5/ataspectj/ataspectj/DumpTestTheDump.java b/tests/java5/ataspectj/ataspectj/DumpTestTheDump.java
new file mode 100644
index 000000000..d2f65ff34
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/DumpTestTheDump.java
@@ -0,0 +1,18 @@
+/*******************************************************************************
+ * 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;
+
+/**
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class DumpTestTheDump {
+}
diff --git a/tests/java5/ataspectj/ataspectj/aop-dump.xml b/tests/java5/ataspectj/ataspectj/aop-dump.xml
new file mode 100644
index 000000000..76bfda829
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/aop-dump.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0"?>
+<aspectj>
+    <weaver options="-XmessageHolderClass:ataspectj.TestHelper">
+        <dump within="ataspectj.DumpTestThe*"/> 
+    </weaver>
+</aspectj>
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
index 145426d70..e6e354de6 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
@@ -66,4 +66,8 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase {
         runTest("AjcLTW AroundInlineMungerTest2");
     }
 
+    public void testLTWDump() {
+        runTest("LTW DumpTest");
+    }
+
 }
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
index 5a770808a..6cd0caccc 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
@@ -78,4 +78,12 @@
             options="-1.5 -Xreweavable"/>
         <ant file="ajc-ant.xml" target="ltw.AroundInlineMungerTest2" verbose="true"/>
     </ajc-test>
+
+    <ajc-test dir="java5/ataspectj" title="LTW DumpTest">
+        <compile
+            files="ataspectj/DumpTest.java,ataspectj/DumpTestTheDump.java,ataspectj/TestHelper.java"
+            options="-1.5"/>
+        <ant file="ajc-ant.xml" target="ltw.DumpTest" verbose="true"/>
+    </ajc-test>
+
 </suite>
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index a96237b19..7ae940945 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -485,7 +485,6 @@ public class BcelWeaver implements IWeaver {
                         int numArgs = advice.getSignature().getParameterTypes().length;
                         if (numFormals > 0) {
                             names = advice.getSignature().getParameterNames(world);
-                            System.out.println(advice.getSignature().toLongString());//AVB
                             validateBindings(newP,p,numArgs,names);
                         }
                     } else {
diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
index 1eb97964f..9bedb142f 100644
--- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
+++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
@@ -173,7 +173,7 @@ public class WeavingAdaptor {
 	 */
 	public byte[] weaveClass (String name, byte[] bytes) throws IOException {
 		if (shouldWeave(name)) {
-            System.out.println("WeavingAdaptor.weaveClass " + name);
+            //System.out.println("WeavingAdaptor.weaveClass " + name);
 			info("weaving '" + name + "'");
 			bytes = getWovenBytes(name, bytes);
 		}
@@ -191,6 +191,10 @@ public class WeavingAdaptor {
         return true;
     }
 
+    public boolean shouldDump(String name) {
+        return false;
+    }
+
 	private boolean shouldWeaveName (String name) {
 		return !((/*(name.startsWith("org.apache.bcel.")//FIXME AV why ? bcel is wrapped in org.aspectj.
                 ||*/ name.startsWith("org.aspectj.")