concrete-aspect impl and doc for LTW - see #95529

pbly some issue on abstract @Pointcut() in ajdt core - fix coming

7 files changed, 398 insertions, 16 deletions

diff --git a/loadtime/.classpath b/loadtime/.classpath
index 5a9f37bcd..8ac0f898e 100644
--- a/loadtime/.classpath
+++ b/loadtime/.classpath
@@ -7,6 +7,7 @@
 	<classpathentry combineaccessrules="false" kind="src" path="/bridge"/>
 	<classpathentry combineaccessrules="false" kind="src" path="/util"/>
 	<classpathentry combineaccessrules="false" kind="src" path="/weaver"/>
+	<classpathentry combineaccessrules="false" kind="src" path="/bcel-builder"/>
 	<classpathentry kind="lib" path="/lib/ext/jrockit/managementapi-jrockit81.jar"/>
 	<classpathentry sourcepath="/lib/junit/junit-src.jar" kind="lib" path="/lib/junit/junit.jar"/>
 	<classpathentry kind="lib" path="/lib/ant/lib/xml-apis.jar"/>
diff --git a/loadtime/src/aspectj_1_5_0.dtd b/loadtime/src/aspectj_1_5_0.dtd
index 710c0dac8..060962745 100644
--- a/loadtime/src/aspectj_1_5_0.dtd
+++ b/loadtime/src/aspectj_1_5_0.dtd
@@ -130,6 +130,7 @@ concrete-aspect
 <!ATTLIST concrete-aspect
     name CDATA #REQUIRED
     extends CDATA #REQUIRED
+    precedence CDATA #IMPLIED
 >
 <!--*****************************************************************************************************************************
 pointcut
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
index 547b14170..ebfdce180 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
@@ -71,7 +71,6 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
              * @param bytes
              */
             public void acceptClass(String name, byte[] bytes) {
-                //TODO av make dump configurable
                 try {
                     if (shouldDump(name.replace('/', '.'))) {
                         Aj.dump(name, bytes);
@@ -79,6 +78,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                 } catch (Throwable throwable) {
                     throwable.printStackTrace();
                 }
+
                 Aj.defineClass(loader, name, bytes);// could be done lazily using the hook
             }
         };
@@ -113,6 +113,23 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             messageHandler = bcelWorld.getMessageHandler();
             // after adding aspects
             weaver.prepareForWeave();
+//            // weave and flush what was registered so far
+//            for (Iterator iterator = m_codeGens.iterator(); iterator.hasNext();) {
+//                ConcreteAspectCodeGen concreteAspectCodeGen = (ConcreteAspectCodeGen) iterator.next();
+//                byte[] partiallyWoven = concreteAspectCodeGen.getBytes(this);
+//                this.generatedClassHandler.acceptClass(
+//                        concreteAspectCodeGen.m_concreteAspect.name,
+//                        partiallyWoven
+//                );
+//                ResolvedType aspect = weaver.addLibraryAspect(concreteAspectCodeGen.m_concreteAspect.name);
+//                //generate key for SC
+//                String aspectCode = readAspect(concreteAspectCodeGen.m_concreteAspect.name, loader);
+//                if(namespace==null){
+//                    namespace=new StringBuffer(aspectCode);
+//                }else{
+//                    namespace = namespace.append(";"+aspectCode);
+//                }
+//            }
         }
     }
 
@@ -257,16 +274,21 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         //TODO: the exclude aspect allow to exclude aspect defined upper in the CL hierarchy - is it what we want ??
         // if not, review the getResource so that we track which resource is defined by which CL
 
-        //it aspectClassNames
+        //iterate aspectClassNames
         //exclude if in any of the exclude list
         for (Iterator iterator = definitions.iterator(); iterator.hasNext();) {
             Definition definition = (Definition) iterator.next();
             for (Iterator aspects = definition.getAspectClassNames().iterator(); aspects.hasNext();) {
                 String aspectClassName = (String) aspects.next();
                 if (acceptAspect(aspectClassName)) {
-                    weaver.addLibraryAspect(aspectClassName);
-                	
-                	//generate key for SC
+                    ResolvedType aspect = weaver.addLibraryAspect(aspectClassName);
+                    if (aspect.isAbstract()) {
+                        // this is a warning
+                        weaver.getWorld().getMessageHandler().handleMessage(
+                                new Message("Abstract aspect registered in aop.xml, use a <concrete-aspect> element instead", IMessage.WARNING, null, null)
+                        );
+                    }
+                    //generate key for SC
                 	String aspectCode = readAspect(aspectClassName, loader);
                     if(namespace==null){
                     	namespace=new StringBuffer(aspectCode);
@@ -277,9 +299,35 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             }
         }
 
-        //it concreteAspects
-        //exclude if in any of the exclude list
-        //TODO
+        //iterate concreteAspects
+        //exclude if in any of the exclude list - note that the user defined name matters for that to happen
+        for (Iterator iterator = definitions.iterator(); iterator.hasNext();) {
+            Definition definition = (Definition) iterator.next();
+            for (Iterator aspects = definition.getConcreteAspects().iterator(); aspects.hasNext();) {
+                Definition.ConcreteAspect concreteAspect = (Definition.ConcreteAspect) aspects.next();
+                if (acceptAspect(concreteAspect.name)) {
+                    ConcreteAspectCodeGen gen = new ConcreteAspectCodeGen(concreteAspect, weaver.getWorld());
+                    if (!gen.validate()) {
+                        weaver.getWorld().getMessageHandler().handleMessage(
+                                new Message("Concrete-aspect '"+concreteAspect.name+"' could not be registered", IMessage.ERROR, null, null)
+                        );
+                        break;
+                    }
+                    this.generatedClassHandler.acceptClass(
+                            concreteAspect.name,
+                            gen.getBytes()
+                    );
+                    ResolvedType aspect = weaver.addLibraryAspect(concreteAspect.name);
+                    //generate key for SC
+                	String aspectCode = readAspect(concreteAspect.name, loader);
+                    if(namespace==null){
+                    	namespace=new StringBuffer(aspectCode);
+                    }else{
+                    	namespace = namespace.append(";"+aspectCode);
+                    }
+                }
+            }
+        }
     }
 
     /**
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
new file mode 100644
index 000000000..b90ddeda0
--- /dev/null
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
@@ -0,0 +1,320 @@
+/*******************************************************************************
+ * 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 org.aspectj.weaver.loadtime;
+
+import org.aspectj.apache.bcel.Constants;
+import org.aspectj.apache.bcel.classfile.JavaClass;
+import org.aspectj.apache.bcel.generic.InstructionConstants;
+import org.aspectj.apache.bcel.generic.InstructionList;
+import org.aspectj.apache.bcel.generic.ObjectType;
+import org.aspectj.apache.bcel.generic.Type;
+import org.aspectj.apache.bcel.generic.annotation.AnnotationGen;
+import org.aspectj.apache.bcel.generic.annotation.ElementNameValuePairGen;
+import org.aspectj.apache.bcel.generic.annotation.ElementValueGen;
+import org.aspectj.apache.bcel.generic.annotation.SimpleElementValueGen;
+import org.aspectj.bridge.IMessage;
+import org.aspectj.bridge.Message;
+import org.aspectj.weaver.AnnotationX;
+import org.aspectj.weaver.ResolvedMember;
+import org.aspectj.weaver.ResolvedType;
+import org.aspectj.weaver.UnresolvedType;
+import org.aspectj.weaver.World;
+import org.aspectj.weaver.bcel.BcelPerClauseAspectAdder;
+import org.aspectj.weaver.bcel.BcelWorld;
+import org.aspectj.weaver.bcel.LazyClassGen;
+import org.aspectj.weaver.bcel.LazyMethodGen;
+import org.aspectj.weaver.loadtime.definition.Definition;
+import org.aspectj.weaver.patterns.PerClause;
+
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * Generates bytecode for concrete-aspect
+ * <p/>
+ * The concrete aspect is @AspectJ code generated. As it is build during aop.xml definitions registration
+ * we perform the type munging for perclause ie aspectOf artifact directly, instead of waiting for it
+ * to go thru the weaver (that we are in the middle of configuring).
+ *
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class ConcreteAspectCodeGen {
+
+    private final static String[] EMPTY_STRINGS = new String[0];
+    private final static Type[] EMPTY_TYPES = new Type[0];
+
+    /**
+     * Concrete aspect definition we build for
+     */
+    private final Definition.ConcreteAspect m_concreteAspect;
+
+    /**
+     * World for which we build for
+     */
+    private final World m_world;
+
+    /**
+     * Set to true when all is checks are verified
+     */
+    private boolean m_isValid = false;
+
+    /**
+     * The parent aspect, not concretized
+     */
+    private ResolvedType m_parent;
+
+    /**
+     * Aspect perClause, used for direct munging of aspectOf artifacts
+     */
+    private PerClause m_perClause;
+
+    /**
+     * Create a new compiler for a concrete aspect
+     *
+     * @param concreteAspect
+     * @param world
+     */
+    ConcreteAspectCodeGen(Definition.ConcreteAspect concreteAspect, World world) {
+        m_concreteAspect = concreteAspect;
+        m_world = world;
+    }
+
+    /**
+     * Checks that concrete aspect is valid
+     *
+     * @return true if ok, false otherwise
+     */
+    public boolean validate() {
+        if (!(m_world instanceof BcelWorld)) {
+            reportError("Internal error: world must be of type BcelWorld");
+            return false;
+        }
+
+        m_parent = m_world.resolve(m_concreteAspect.extend, true);
+        // handle inner classes
+        if (m_parent.equals(ResolvedType.MISSING)) {
+            // fallback on inner class lookup mechanism
+            String fixedName = m_concreteAspect.extend;
+            int hasDot = fixedName.lastIndexOf('.');
+            while (hasDot > 0) {
+                char[] fixedNameChars = fixedName.toCharArray();
+                fixedNameChars[hasDot] = '$';
+                fixedName = new String(fixedNameChars);
+                hasDot = fixedName.lastIndexOf('.');
+                m_parent = m_world.resolve(UnresolvedType.forName(fixedName), true);
+                if (!m_parent.equals(ResolvedType.MISSING)) {
+                    break;
+                }
+            }
+        }
+        if (m_parent.isMissing()) {
+            reportError("Cannot find m_parent aspect for: " + stringify());
+            return false;
+        }
+
+        // extends must be abstract
+        if (!m_parent.isAbstract()) {
+            reportError("Attempt to concretize a non-abstract aspect: " + stringify());
+            return false;
+        }
+
+        // m_parent must be aspect
+        if (!m_parent.isAspect()) {
+            reportError("Attempt to concretize a non aspect: " + stringify());
+            return false;
+        }
+
+        // must be undefined so far
+        ResolvedType current = m_world.resolve(m_concreteAspect.name, true);
+        if (!current.isMissing()) {
+            reportError("Attempt to concretize but choosen aspect name already defined:" + stringify());
+            return false;
+        }
+
+        // must have all abstractions defined
+        List elligibleAbstractions = new ArrayList();
+        Iterator methods = m_parent.getMethods();
+        while (methods.hasNext()) {
+            ResolvedMember method = (ResolvedMember) methods.next();
+            if (method.isAbstract()) {
+                if ("()V".equals(method.getSignature())) {
+                    elligibleAbstractions.add(method.getName());
+                } else {
+                    reportError("Abstract member '" + method.getName() + "' cannot be concretized as a pointcut (illegal signature): " + stringify());
+                    return false;
+                }
+            }
+        }
+        List pointcutNames = new ArrayList();
+        for (Iterator it = m_concreteAspect.pointcuts.iterator(); it.hasNext();) {
+            Definition.Pointcut abstractPc = (Definition.Pointcut) it.next();
+            pointcutNames.add(abstractPc.name);
+        }
+        for (Iterator it = elligibleAbstractions.iterator(); it.hasNext();) {
+            String elligiblePc = (String) it.next();
+            if (!pointcutNames.contains(elligiblePc)) {
+                reportError("Abstract pointcut '" + elligiblePc + "' not configured: " + stringify());
+                return false;
+            }
+        }
+
+        m_perClause = m_parent.getPerClause();
+        m_isValid = true;
+        return m_isValid;
+    }
+
+    /**
+     * Rebuild the XML snip that defines this concrete aspect, for log error purpose
+     *
+     * @return string repr.
+     */
+    private String stringify() {
+        StringBuffer sb = new StringBuffer("<concrete-aspect name='");
+        sb.append(m_concreteAspect.name);
+        sb.append("' extends='");
+        sb.append(m_concreteAspect.extend);
+        sb.append("'/> in aop.xml");
+        return sb.toString();
+    }
+
+    /**
+     * Build the bytecode for the concrete aspect
+     *
+     * @return concrete aspect bytecode
+     */
+    public byte[] getBytes() {
+        if (!m_isValid) {
+            throw new RuntimeException("Must validate first");
+        }
+
+        //TODO AV - abstract away from BCEL...
+        // @Aspect //inherit clause from m_parent
+        // @DeclarePrecedence("....") // if any
+        // public class xxxName extends xxxExtends {
+        //    @Pointcut(xxxExpression-n)
+        //    private void xxxName-n() {}
+        // }
+
+        // @Aspect public class ...
+        LazyClassGen cg = new LazyClassGen(
+                m_concreteAspect.name.replace('.', '/'),
+                m_parent.getName(),
+                null,//TODO AV - we could point to the aop.xml that defines it and use JSR-45
+                Modifier.PUBLIC + Constants.ACC_SUPER,
+                EMPTY_STRINGS,
+                m_world
+        );
+        AnnotationGen ag = new AnnotationGen(
+                new ObjectType("org/aspectj/lang/annotation/Aspect"),
+                Collections.EMPTY_LIST,
+                true,
+                cg.getConstantPoolGen()
+        );
+        cg.addAnnotation(ag.getAnnotation());
+        if (m_concreteAspect.precedence != null) {
+            SimpleElementValueGen svg = new SimpleElementValueGen(
+                    ElementValueGen.STRING,
+                    cg.getConstantPoolGen(),
+                    m_concreteAspect.precedence
+            );
+            List elems = new ArrayList();
+            elems.add(new ElementNameValuePairGen("value", svg, cg.getConstantPoolGen()));
+            AnnotationGen agprec = new AnnotationGen(
+                    new ObjectType("org/aspectj/lang/annotation/DeclarePrecedence"),
+                    elems,
+                    true,
+                    cg.getConstantPoolGen()
+            );
+            cg.addAnnotation(agprec.getAnnotation());
+        }
+
+        // default constructor
+        LazyMethodGen init = new LazyMethodGen(
+                Modifier.PUBLIC,
+                Type.VOID,
+                "<init>",
+                EMPTY_TYPES,
+                EMPTY_STRINGS,
+                cg
+        );
+        InstructionList cbody = init.getBody();
+        cbody.append(InstructionConstants.ALOAD_0);
+        cbody.append(cg.getFactory().createInvoke(
+                m_parent.getName().replace('.', '/'),
+                "<init>",
+                Type.VOID,
+                EMPTY_TYPES,
+                Constants.INVOKESPECIAL
+        ));
+        cbody.append(InstructionConstants.RETURN);
+        cg.addMethodGen(init);
+
+        for (Iterator it = m_concreteAspect.pointcuts.iterator(); it.hasNext();) {
+            Definition.Pointcut abstractPc = (Definition.Pointcut) it.next();
+
+            LazyMethodGen mg = new LazyMethodGen(
+                    Modifier.PUBLIC,
+                    Type.VOID,
+                    abstractPc.name,
+                    EMPTY_TYPES,
+                    EMPTY_STRINGS,
+                    cg
+            );
+            SimpleElementValueGen svg = new SimpleElementValueGen(
+                    ElementValueGen.STRING,
+                    cg.getConstantPoolGen(),
+                    abstractPc.expression
+            );
+            List elems = new ArrayList();
+            elems.add(new ElementNameValuePairGen("value", svg, cg.getConstantPoolGen()));
+            AnnotationGen mag = new AnnotationGen(
+                    new ObjectType("org/aspectj/lang/annotation/Pointcut"),
+                    elems,
+                    true,
+                    cg.getConstantPoolGen()
+            );
+            AnnotationX max = new AnnotationX(mag.getAnnotation(), m_world);
+            mg.addAnnotation(max);
+
+            InstructionList body = mg.getBody();
+            body.append(InstructionConstants.RETURN);
+
+            cg.addMethodGen(mg);
+        }
+
+        // handle the perClause
+        BcelPerClauseAspectAdder perClauseMunger = new BcelPerClauseAspectAdder(
+                ResolvedType.forName(m_concreteAspect.name).resolve(m_world),
+                m_perClause.getKind()
+        );
+        perClauseMunger.forceMunge(cg);
+
+        //TODO AV - unsafe cast
+        // register the fresh new class into the world repository as it does not exist on the classpath anywhere
+        JavaClass jc = cg.getJavaClass((BcelWorld) m_world);
+        ((BcelWorld) m_world).addSourceObjectType(jc);
+
+        return jc.getBytes();
+    }
+
+    /**
+     * Error reporting
+     *
+     * @param message
+     */
+    private void reportError(String message) {
+        m_world.getMessageHandler().handleMessage(new Message(message, IMessage.ERROR, null, null));
+    }
+}
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
index a80a4a918..8c7035b5e 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
@@ -42,7 +42,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 	 */
 	public WeavingURLClassLoader (ClassLoader parent) {
 		this(getURLs(getClassPath()),getURLs(getAspectPath()),parent);
-//		System.err.println("? WeavingURLClassLoader.<init>(" + parent + ")");
+//		System.err.println("? WeavingURLClassLoader.<init>(" + m_parent + ")");
 	}
 	
 	public WeavingURLClassLoader (URL[] urls, ClassLoader parent) {
@@ -55,7 +55,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 //		System.err.println("? WeavingURLClassLoader.<init>() classURLs=" + classURLs.length + ", aspectURLs=" + aspectURLs.length);
 		this.aspectURLs = aspectURLs;
 		
-		/* If either we nor our parent is using an ASPECT_PATH use a new-style
+		/* If either we nor our m_parent is using an ASPECT_PATH use a new-style
 		 * adaptor
 		 */ 
 		if (this.aspectURLs.length > 0 || parent instanceof WeavingClassLoader) {
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
index 73677e60f..87c2f82e2 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
@@ -74,20 +74,26 @@ public class Definition {
     }
 
     public static class ConcreteAspect {
-        String name;
-        String extend;
-        List pointcuts;
+        public final String name;
+        public final String extend;
+        public final String precedence;
+        public final List pointcuts;
 
         public ConcreteAspect(String name, String extend) {
+            this(name, extend,  null);
+        }
+
+        public ConcreteAspect(String name, String extend, String precedence) {
             this.name = name;
             this.extend = extend;
+            this.precedence = precedence;
             this.pointcuts = new ArrayList();
         }
     }
 
     public static class Pointcut {
-        String name;
-        String expression;
+        public final String name;
+        public final String expression;
         public Pointcut(String name, String expression) {
             this.name = name;
             this.expression = expression;
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
index 9dbbfa18f..1aeccaf1d 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
@@ -56,6 +56,7 @@ public class DocumentParser extends DefaultHandler {
     private final static String CONCRETE_ASPECT_ELEMENT = "concrete-aspect";
     private final static String NAME_ATTRIBUTE = "name";
     private final static String EXTEND_ATTRIBUTE = "extends";
+    private final static String PRECEDENCE_ATTRIBUTE = "precedence";
     private final static String POINTCUT_ELEMENT = "pointcut";
     private final static String WITHIN_ATTRIBUTE = "within";
     private final static String EXPRESSION_ATTRIBUTE = "expression";
@@ -149,8 +150,13 @@ public class DocumentParser extends DefaultHandler {
         } else if (CONCRETE_ASPECT_ELEMENT.equals(qName)) {
             String name = attributes.getValue(NAME_ATTRIBUTE);
             String extend = attributes.getValue(EXTEND_ATTRIBUTE);
+            String precedence = attributes.getValue(PRECEDENCE_ATTRIBUTE);
             if (!isNull(name) && !isNull(extend)) {
-                m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend);
+                if (isNull(precedence)) {
+                    m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend);
+                } else {
+                    m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend, precedence);
+                }
                 m_definition.getConcreteAspects().add(m_lastConcreteAspect);
             }
         } else if (POINTCUT_ELEMENT.equals(qName) && m_lastConcreteAspect != null) {