impl and test for dec precedence in aop.xml without extends

3 files changed, 40 insertions, 17 deletions

diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
index fb30edd6e..897e17f52 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
@@ -34,6 +34,7 @@ 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 org.aspectj.weaver.patterns.PerSingleton;
 
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
@@ -102,6 +103,26 @@ public class ConcreteAspectCodeGen {
             return false;
         }
 
+        // name 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;
+        }
+
+        // it can happen that extends is null, for precedence only declaration
+        if (m_concreteAspect.extend == null && m_concreteAspect.precedence != null) {
+            if (m_concreteAspect.pointcuts.isEmpty()) {
+                m_isValid = true;
+                m_perClause = new PerSingleton();
+                m_parent = null;
+                return true;// no need to checks more in that special case
+            } else {
+                reportError("Attempt to use nested pointcuts without extends clause: "+stringify());
+                return false;
+            }
+        }
+
         m_parent = m_world.resolve(m_concreteAspect.extend, true);
         // handle inner classes
         if (m_parent.equals(ResolvedType.MISSING)) {
@@ -136,13 +157,6 @@ public class ConcreteAspectCodeGen {
             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();
@@ -202,15 +216,15 @@ public class ConcreteAspectCodeGen {
         //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() {}
+        // public class xxxName [extends xxxExtends] {
+        //    [@Pointcut(xxxExpression-n)
+        //    public void xxxName-n() {}]
         // }
 
         // @Aspect public class ...
         LazyClassGen cg = new LazyClassGen(
                 m_concreteAspect.name.replace('.', '/'),
-                m_parent.getName(),
+                (m_parent==null)?"java/lang/Object":m_parent.getName().replace('.', '/'),
                 null,//TODO AV - we could point to the aop.xml that defines it and use JSR-45
                 Modifier.PUBLIC + Constants.ACC_SUPER,
                 EMPTY_STRINGS,
@@ -252,7 +266,7 @@ public class ConcreteAspectCodeGen {
         InstructionList cbody = init.getBody();
         cbody.append(InstructionConstants.ALOAD_0);
         cbody.append(cg.getFactory().createInvoke(
-                m_parent.getName().replace('.', '/'),
+                (m_parent==null)?"java/lang/Object":m_parent.getName().replace('.', '/'),
                 "<init>",
                 Type.VOID,
                 EMPTY_TYPES,
@@ -265,7 +279,7 @@ public class ConcreteAspectCodeGen {
             Definition.Pointcut abstractPc = (Definition.Pointcut) it.next();
 
             LazyMethodGen mg = new LazyMethodGen(
-                    Modifier.PUBLIC,
+                    Modifier.PUBLIC,//TODO AV - respect visibility instead of opening up?
                     Type.VOID,
                     abstractPc.name,
                     EMPTY_TYPES,
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
index 87c2f82e2..9bea5cd2d 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
@@ -85,7 +85,15 @@ public class Definition {
 
         public ConcreteAspect(String name, String extend, String precedence) {
             this.name = name;
-            this.extend = extend;
+            // make sure extend set to null if ""
+            if (extend == null || extend.length() == 0) {
+                this.extend = null;
+                if (precedence == null || precedence.length() == 0) {
+                    throw new RuntimeException("Not allowed");
+                }
+            } else {
+                this.extend = extend;
+            }
             this.precedence = precedence;
             this.pointcuts = new ArrayList();
         }
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
index 1aeccaf1d..ac80afd20 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
@@ -151,10 +151,11 @@ public class DocumentParser extends DefaultHandler {
             String name = attributes.getValue(NAME_ATTRIBUTE);
             String extend = attributes.getValue(EXTEND_ATTRIBUTE);
             String precedence = attributes.getValue(PRECEDENCE_ATTRIBUTE);
-            if (!isNull(name) && !isNull(extend)) {
-                if (isNull(precedence)) {
+            if (!isNull(name)) {
+                if (isNull(precedence) && !isNull(extend)) {//if no precedence, then extends must be there
                     m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend);
-                } else {
+                } else if (!isNull(precedence)) {
+                    // wether a pure precedence def, or an extendsANDprecedence def.
                     m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend, precedence);
                 }
                 m_definition.getConcreteAspects().add(m_lastConcreteAspect);