test for if @style - a todo for Adrian

diff --git a/tests/java5/ataspectj/ataspectj/AllLTWTests.java b/tests/java5/ataspectj/ataspectj/AllLTWTests.java
index da5856674b53911faf79a84a3bc6e3a329db84ba..db9f5d95cd2d329d3eaff7288f62ec4b4cca1e49 100644 (file)
--- a/tests/java5/ataspectj/ataspectj/AllLTWTests.java
+++ b/tests/java5/ataspectj/ataspectj/AllLTWTests.java
@@ -22,17 +22,15 @@ public class AllLTWTests extends TestCase {
     public static Test suite() {
         TestSuite suite = new TestSuite("All tests");
 
-        suite.addTestSuite(ataspectj.SingletonAspectBindingsTest.class);
-        suite.addTestSuite(ataspectj.CflowTest.class);
-        suite.addTestSuite(ataspectj.PointcutReferenceTest.class);
-        suite.addTestSuite(ataspectj.AfterXTest.class);
-        //FIXME AV - fix the pc grammar to support if for @AJ aspects
-        System.err.println("(AllTests: IfPointcutTest fails)");
-        //suite.addTestSuite(IfPointcutTest.class);
-        suite.addTestSuite(ataspectj.XXJoinPointTest.class);
-        suite.addTestSuite(ataspectj.PrecedenceTest.class);
-        suite.addTestSuite(ataspectj.BindingTest.class);
-        suite.addTestSuite(ataspectj.PerClauseTest.class);
+        suite.addTestSuite(SingletonAspectBindingsTest.class);
+        suite.addTestSuite(CflowTest.class);
+        suite.addTestSuite(PointcutReferenceTest.class);
+        suite.addTestSuite(AfterXTest.class);
+        suite.addTestSuite(IfPointcutTest.class);
+        suite.addTestSuite(XXJoinPointTest.class);
+        suite.addTestSuite(PrecedenceTest.class);
+        suite.addTestSuite(BindingTest.class);
+        suite.addTestSuite(PerClauseTest.class);
         suite.addTestSuite(AroundInlineMungerTest.class);
         suite.addTestSuite(SingletonInheritanceTest.class);
         suite.addTestSuite(PerClauseInheritanceTest.class);

diff --git a/tests/java5/ataspectj/ataspectj/IfPointcutTest.java b/tests/java5/ataspectj/ataspectj/IfPointcutTest.java
index a50f5935473a3bbd1c3b4a936afe1b766f0f25ab..f793ceb59867c4b68fbf14a844eb6d2863c7a60d 100644 (file)
--- a/tests/java5/ataspectj/ataspectj/IfPointcutTest.java
+++ b/tests/java5/ataspectj/ataspectj/IfPointcutTest.java
@@ -34,10 +34,12 @@ public class IfPointcutTest extends TestCase {
     }
 
     public void testIf() {
-        fail("FIXME AV: see below, TestAspect has its advice and pointcut commented out");
+        s_log = new StringBuffer();
         IfPointcutTest me = new IfPointcutTest();
         me.hello(1);
+        assertEquals("aop ", s_log.toString());
         me.hello(-1);
+        assertEquals("aop ", s_log.toString());//unchanged
     }
 
     public static void main(String[] args) {
@@ -48,23 +50,17 @@ public class IfPointcutTest extends TestCase {
         return new junit.framework.TestSuite(IfPointcutTest.class);
     }
 
-
-
     @Aspect
     public static class TestAspect {
 
-        public boolean positive(int i) {
-            return (i>=0);
+        @Pointcut("args(i) && if()")
+        public static boolean positive(int i) {
+            return i>=0;
         }
 
-        //FIXME av if pcd support
-        //@Pointcut("args(i) && if(i>0)")
-        void ifPc(int i) {}
-
-        //FIXME av if pcd support
-        //@Before("execution(* ataspectj.IfPointcutTest.hello(int)) && ifPc(i)")
-        void before(int i) {
-            System.out.println("IfPointcutTest$TestAspect.before");
+        @Before("execution(* ataspectj.IfPointcutTest.hello(int)) && positive(i)")
+        public void before(int i) {
+            log("aop");
         }
     }
 }

diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java
index 53053a24ac89e79e0b4e0ceb651280e36c9fe704..97454a4b8d7803a76663fc1fbf4f009d2868675b 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java
@@ -98,4 +98,8 @@ public class AtAjSyntaxTests extends XMLBasedAjcTestCase {
     public void testPerClauseInheritance() {
         runTest("perClauseInheritance");
     }
+
+    public void testIfPointcut() {
+        runTest("IfPointcutTest");
+    }
 }
\ No newline at end of file

diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml
index eeaaf79e77452b39976d9c4fd99e668c35e1acd3..61d0e2572299bd0e9d8aa1b03fd5346586b14f76 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml
@@ -63,11 +63,13 @@
         <run class="ataspectj.AfterXTest"/>
     </ajc-test>
 
-    <!--    <comment>FIXME AV when we impl if support in pointcut parser and weaver</comment>-->
-    <!--    <ajc-test dir="java5/ataspectj" title="IfPointcutTest">-->
-    <!--        <compile files="ataspectj/IfPointcutTest.java,ataspectj/TestHelper.java" options="-1.5"/>-->
-    <!--        <run class="ataspectj.IfPointcutTest"/>-->
-    <!--    </ajc-test>-->
+    <ajc-test dir="java5/ataspectj" title="IfPointcutTest">
+        <compile files="ataspectj/IfPointcutTest.java,ataspectj/TestHelper.java" options="-1.5 -Xdev:NoAtAspectJProcessing"/>
+        <run class="ataspectj.IfPointcutTest"/>
+        <!-- FIXME AV - Adrian in JDT stuff -->
+<!--        <compile files="ataspectj/IfPointcutTest.java,ataspectj/TestHelper.java" options="-1.5"/>-->
+<!--        <run class="ataspectj.IfPointcutTest"/>-->
+    </ajc-test>
 
     <ajc-test dir="java5/ataspectj" title="BindingTest">
         <compile files="ataspectj/BindingTest.java,ataspectj/TestHelper.java" options="-1.5"/>

diff --git a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
index 938411661a5d183a3d492da376e8ca3ab9116ecf..a310f083802423db88238af964b1f72288309614 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
+++ b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
@@ -53,6 +53,11 @@ import org.aspectj.weaver.patterns.SimpleScope;
 import org.aspectj.weaver.patterns.TypePattern;
 import org.aspectj.weaver.patterns.PerFromSuper;
 import org.aspectj.weaver.patterns.PointcutVisitor;
+import org.aspectj.weaver.patterns.IdentityPointcutVisitor;
+import org.aspectj.weaver.patterns.IfPointcut;
+import org.aspectj.weaver.patterns.AndPointcut;
+import org.aspectj.weaver.patterns.NotPointcut;
+import org.aspectj.weaver.patterns.OrPointcut;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -446,19 +451,19 @@ public class AtAjAttributes {
         final PerClause perClause;
         if (perClauseString.startsWith(PerClause.KindAnnotationPrefix.PERCFLOW.getName())) {
             pointcutString = PerClause.KindAnnotationPrefix.PERCFLOW.extractPointcut(perClauseString);
-            poincut = parsePointcut(pointcutString, struct);
+            poincut = parsePointcut(pointcutString, struct, false);
             perClause = new PerCflow(poincut, false);
         } else if (perClauseString.startsWith(PerClause.KindAnnotationPrefix.PERCFLOWBELOW.getName())) {
             pointcutString = PerClause.KindAnnotationPrefix.PERCFLOWBELOW.extractPointcut(perClauseString);
-            poincut = parsePointcut(pointcutString, struct);
+            poincut = parsePointcut(pointcutString, struct, false);
             perClause = new PerCflow(poincut, true);
         } else if (perClauseString.startsWith(PerClause.KindAnnotationPrefix.PERTARGET.getName())) {
             pointcutString = PerClause.KindAnnotationPrefix.PERTARGET.extractPointcut(perClauseString);
-            poincut = parsePointcut(pointcutString, struct);
+            poincut = parsePointcut(pointcutString, struct, false);
             perClause = new PerObject(poincut, false);
         } else if (perClauseString.startsWith(PerClause.KindAnnotationPrefix.PERTHIS.getName())) {
             pointcutString = PerClause.KindAnnotationPrefix.PERTHIS.extractPointcut(perClauseString);
-            poincut = parsePointcut(pointcutString, struct);
+            poincut = parsePointcut(pointcutString, struct, false);
             perClause = new PerObject(poincut, true);
         } else if (perClauseString.startsWith(PerClause.KindAnnotationPrefix.PERTYPEWITHIN.getName())) {
             pointcutString = PerClause.KindAnnotationPrefix.PERTYPEWITHIN.extractPointcut(perClauseString);
@@ -539,7 +544,7 @@ public class AtAjAttributes {
                 if (preResolvedPointcut != null) {
                     pc = preResolvedPointcut.getPointcut();
                 } else {
-                    pc = parsePointcut(beforeAdvice.getValue().stringifyValue(), struct);
+                    pc = parsePointcut(beforeAdvice.getValue().stringifyValue(), struct, false);
                     if (pc == null) return false;//parse error
                     pc.resolve(binding);
                 }
@@ -593,7 +598,7 @@ public class AtAjAttributes {
                 if (preResolvedPointcut != null) {
                     pc = preResolvedPointcut.getPointcut();
                 } else {
-                    pc = parsePointcut(afterAdvice.getValue().stringifyValue(), struct);
+                    pc = parsePointcut(afterAdvice.getValue().stringifyValue(), struct, false);
                     if (pc == null) return false;//parse error
                     pc.resolve(binding);
                 }
@@ -677,7 +682,7 @@ public class AtAjAttributes {
             if (preResolvedPointcut != null) {
                 pc = preResolvedPointcut.getPointcut();
             } else {
-                pc = parsePointcut(pointcut, struct);
+                pc = parsePointcut(pointcut, struct, false);
                 if (pc == null) return false;//parse error
                 pc.resolve(binding);
             }
@@ -760,7 +765,7 @@ public class AtAjAttributes {
             if (preResolvedPointcut != null) {
                 pc = preResolvedPointcut.getPointcut();
             } else {
-                pc = parsePointcut(pointcut, struct);
+                pc = parsePointcut(pointcut, struct, false);
                 if (pc == null) return false;//parse error
                 pc.resolve(binding);
             }
@@ -813,7 +818,7 @@ public class AtAjAttributes {
                 if (preResolvedPointcut != null) {
                     pc = preResolvedPointcut.getPointcut();
                 } else {
-                    pc = parsePointcut(aroundAdvice.getValue().stringifyValue(), struct);
+                    pc = parsePointcut(aroundAdvice.getValue().stringifyValue(), struct, false);
                     if (pc == null) return false;//parse error
                     pc.resolve(binding);
                 }
@@ -878,7 +883,7 @@ public class AtAjAttributes {
 
                 // use a LazyResolvedPointcutDefinition so that the pointcut is resolved lazily
                 // since for it to be resolved, we will need other pointcuts to be registered as well
-                Pointcut pc = parsePointcut(pointcutExpr.getValue().stringifyValue(), struct);
+                Pointcut pc = parsePointcut(pointcutExpr.getValue().stringifyValue(), struct, true);
                 if (pc == null) return;//parse error
                 // do not resolve binding now but lazily
                 pc.setLocation(struct.context, -1, -1);
@@ -922,7 +927,7 @@ public class AtAjAttributes {
                         struct.context,
                         bindings
                 );
-                Pointcut pc = parsePointcut(declareError.getValue().stringifyValue(), struct);
+                Pointcut pc = parsePointcut(declareError.getValue().stringifyValue(), struct, false);
                 if (pc == null) {
                     hasError = false;//cannot parse pointcut
                 } else {
@@ -949,7 +954,7 @@ public class AtAjAttributes {
                         struct.context,
                         bindings
                 );
-                Pointcut pc = parsePointcut(declareWarning.getValue().stringifyValue(), struct);
+                Pointcut pc = parsePointcut(declareWarning.getValue().stringifyValue(), struct, false);
                 if (pc == null) {
                     hasWarning = false;//cannot parse pointcut
                 } else {
@@ -1339,11 +1344,16 @@ public class AtAjAttributes {
      *
      * @param pointcutString
      * @param location
+     * @param allowIf
      * @return
      */
-    private static Pointcut parsePointcut(String pointcutString, AjAttributeStruct location) {
+    private static Pointcut parsePointcut(String pointcutString, AjAttributeStruct location, boolean allowIf) {
         try {
             Pointcut pointcut = new PatternParser(pointcutString, location.context).parsePointcut();
+            if (!allowIf && pointcutString.indexOf("if()") >= 0 && hasIf(pointcut)) {
+                reportError("if() pointcut is not allowed at this pointcut location '" + pointcutString +"'", location);
+                return null;
+            }
             pointcut.setLocation(location.context, -1, -1);//FIXME -1,-1 is not good enough
             return pointcut;
         } catch (ParserException e) {
@@ -1352,6 +1362,12 @@ public class AtAjAttributes {
         }
     }
 
+    private static boolean hasIf(Pointcut pointcut) {
+        IfFinder visitor = new IfFinder();
+        pointcut.accept(visitor, null);
+        return visitor.hasIf;
+    }
+
     /**
      * Parse the given type pattern, return null on failure and issue an error
      *
@@ -1369,4 +1385,36 @@ public class AtAjAttributes {
             return null;
         }
     }
+
+    /**
+     * Look for an if() pointcut
+     */
+    private static class IfFinder extends IdentityPointcutVisitor {
+        boolean hasIf = false;
+        public Object visit(IfPointcut node, Object data) {
+            if (node.alwaysFalse() || node.alwaysTrue()) {
+                ;//IfFalse / IfTrue
+            } else {
+                hasIf = true;
+            }
+            return node;
+        }
+
+        public Object visit(AndPointcut node, Object data) {
+            if (!hasIf) node.getLeft().accept(this, data);
+            if (!hasIf) node.getLeft().accept(this, data);
+            return node;
+        }
+
+        public Object visit(NotPointcut node, Object data) {
+            if (!hasIf) node.getNegatedPointcut().accept(this, data);
+            return node;
+        }
+
+        public Object visit(OrPointcut node, Object data) {
+            if (!hasIf) node.getLeft().accept(this, data);
+            if (!hasIf) node.getLeft().accept(this, data);
+            return node;
+        }
+    }
 }

diff --git a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
index dc3ffc57679ecf906d372161937447c3df5e4f43..9aa1246f2e4c927b13c80a60825e24b24b6d380b 100644 (file)
--- a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
@@ -117,11 +117,12 @@ public class IfPointcut extends Pointcut {
        
        public void write(DataOutputStream s) throws IOException {
                s.writeByte(Pointcut.IF);
-               testMethod.write(s);
+               testMethod.write(s);//TODO Adrian, do something if this one happens to be null for @style if() from JDT stuff
                s.writeByte(extraParameterFlags);
                writeLocation(s);
        }
        public static Pointcut read(VersionedDataInputStream s, ISourceContext context) throws IOException {
+        //TODO Adrian, read may failt if testMethod happens to be null for @style if() from JDT stuff
                IfPointcut ret = new IfPointcut(ResolvedMember.readResolvedMember(s, context), s.readByte());
                ret.readLocation(context, s);
                return ret;