Fix annotation style support for if(true) and if(false)

The documentation specifies annotation style pointcuts
can use if(false) or if(true) and not require a boolean
return value and body for the @Pointcut annotated
method but it doesn't work without this change to validation
that recognizes the situation.

Fixes #115

diff --git a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
index 285f6f5bf669314361af8fea3862f732d1cb229f..a6cc3c19fdd5c1dfcd9a57648e3b18285b0bfba4 100644 (file)
--- a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
+++ b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
@@ -528,6 +528,7 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor {
 
                boolean noValueSupplied = true;
                boolean containsIfPcd = false;
+               boolean isIfTrueOrFalse = false;
                int[] pcLocation = new int[2];
                String pointcutExpression = getStringLiteralFor("value", ajAnnotations.pointcutAnnotation, pcLocation);
                try {
@@ -538,6 +539,11 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor {
                        } else {
                                noValueSupplied = false;
                                pc = new PatternParser(pointcutExpression, context).parsePointcut();
+                               if (pc instanceof IfPointcut) {
+                                       if (((IfPointcut)pc).alwaysFalse() || ((IfPointcut)pc).alwaysTrue()) {
+                                               isIfTrueOrFalse = true;
+                                       }
+                               }
                        }
                        pcDecl.pointcutDesignator = (pc == null) ? null : new PointcutDesignator(pc);
                        pcDecl.setGenerateSyntheticPointcutMethod();
@@ -592,10 +598,10 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor {
                                        methodDeclaration.returnType.sourceEnd,
                                        "Methods annotated with @Pointcut must return void unless the pointcut contains an if() expression");
                }
-               if (!returnsBoolean && containsIfPcd) {
+               if (!returnsBoolean && containsIfPcd && !isIfTrueOrFalse) {
                        methodDeclaration.scope.problemReporter().signalError(methodDeclaration.returnType.sourceStart,
                                        methodDeclaration.returnType.sourceEnd,
-                                       "Methods annotated with @Pointcut must return boolean when the pointcut contains an if() expression");
+                                       "Methods annotated with @Pointcut must return boolean when the pointcut contains an if() expression unless it is if(false) or if(true)");
                }
 
                if (methodDeclaration.statements != null && methodDeclaration.statements.length > 0 && !containsIfPcd) {

diff --git a/tests/bugs198/github_115/A.java b/tests/bugs198/github_115/A.java
new file mode 100644 (file)
index 0000000..07df21f
--- /dev/null
+++ b/tests/bugs198/github_115/A.java
@@ -0,0 +1,32 @@
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+import org.aspectj.lang.annotation.Pointcut;
+
+public class A {
+
+       public static void main(String []argv) {
+               System.out.println("A.main");
+       }
+
+}
+
+@Aspect
+class Azpect {
+       
+       @Pointcut("if(false)")
+       public void isFalse() { }
+
+       @Pointcut("if(true)")
+       public void isTrue() { }
+
+       @Before("isTrue() && execution(* A.main(..))")
+       public void beforeTrue() {
+               System.out.println("Azpect.beforeTrue");
+       }
+
+       @Before("isFalse() && execution(* A.main(..))")
+       public void beforeFalse() {
+               System.out.println("Azpect.beforeFalse");
+       }
+}
+

diff --git a/tests/bugs198/github_115/B.java b/tests/bugs198/github_115/B.java
new file mode 100644 (file)
index 0000000..eba199b
--- /dev/null
+++ b/tests/bugs198/github_115/B.java
@@ -0,0 +1,33 @@
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+import org.aspectj.lang.annotation.Pointcut;
+
+public class B {
+
+       public static void main(String []argv) {
+               System.out.println("B.main");
+       }
+
+}
+
+@Aspect
+abstract class AbstractAzpect {
+
+       @Pointcut
+       public abstract void isTrue();
+       
+       @Before("isTrue() && execution(* B.main(..))")
+       public void beforeFalse() {
+               System.out.println("Azpect.beforeFalse");
+       }
+}
+
+@Aspect
+class Azpect extends AbstractAzpect {
+       
+       @Override
+       @Pointcut("if(true)")
+       public void isTrue() { }
+
+}
+

diff --git a/tests/src/test/java/org/aspectj/systemtest/ajc198/Ajc198TestsJava.java b/tests/src/test/java/org/aspectj/systemtest/ajc198/Ajc198TestsJava.java
index 8fa9110f2e09f1036d309fa21224e2d723428e8b..6ee1ddef721267af8a9ccbd4553f1c7f1e47c981 100644 (file)
--- a/tests/src/test/java/org/aspectj/systemtest/ajc198/Ajc198TestsJava.java
+++ b/tests/src/test/java/org/aspectj/systemtest/ajc198/Ajc198TestsJava.java
@@ -35,6 +35,14 @@ public class Ajc198TestsJava extends XMLBasedAjcTestCaseForJava17OrLater {
                // TODO: replace 0 by Constants.PREVIEW_MINOR_VERSION after no longer using EA build, but final JDK version
                checkVersion("PersonAspect", Constants.MAJOR_17, 0 /*Constants.PREVIEW_MINOR_VERSION*/);
        }
+       
+       public void testAnnotationStyleSpecialIfClauses() {
+               runTest("annotation style A");
+       }
+
+       public void testAnnotationStylePointcutInheritanceWithIfClauses() {
+               runTest("annotation style B");
+       }
 
   public static Test suite() {
     return XMLBasedAjcTestCase.loadSuite(Ajc198TestsJava.class);

diff --git a/tests/src/test/resources/org/aspectj/systemtest/ajc198/ajc198.xml b/tests/src/test/resources/org/aspectj/systemtest/ajc198/ajc198.xml
index 202258c94c9684c71648415b5fd151673dd4c3d7..64ba56bd98a1f0372665fe25ae6289e2d68f46c8 100644 (file)
--- a/tests/src/test/resources/org/aspectj/systemtest/ajc198/ajc198.xml
+++ b/tests/src/test/resources/org/aspectj/systemtest/ajc198/ajc198.xml
@@ -100,4 +100,26 @@
                <compile files="Buffers.java" options="--release 8"/>
        </ajc-test>
 
+       <ajc-test dir="bugs198/github_115" title="annotation style A">
+               <compile files="A.java" options="-1.5">
+                       <message kind="warning" line="28" text="advice defined in Azpect has not been applied [Xlint:adviceDidNotMatch]"/>
+               </compile>
+               <run class="A">
+                       <stdout>
+                               <line text="Azpect.before"/>
+                               <line text="A.main"/>
+                       </stdout>
+               </run>
+       </ajc-test>
+
+       <ajc-test dir="bugs198/github_115" title="annotation style B">
+               <compile files="B.java" options="-1.5"/>
+               <run class="B">
+                       <stdout>
+                               <line text="Azpect.before"/>
+                               <line text="B.main"/>
+                       </stdout>
+               </run>
+       </ajc-test>
+
 </suite>