From b720626100766ec0c34985404d774eccb51efdc0 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Mon, 17 Jan 2022 17:47:37 -0800 Subject: [PATCH] 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 --- .../ValidateAtAspectJAnnotationsVisitor.java | 10 ++++-- tests/bugs198/github_115/A.java | 32 ++++++++++++++++++ tests/bugs198/github_115/B.java | 33 +++++++++++++++++++ .../systemtest/ajc198/Ajc198TestsJava.java | 8 +++++ .../org/aspectj/systemtest/ajc198/ajc198.xml | 22 +++++++++++++ 5 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 tests/bugs198/github_115/A.java create mode 100644 tests/bugs198/github_115/B.java 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 285f6f5bf..a6cc3c19f 100644 --- 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 index 000000000..07df21f10 --- /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 index 000000000..eba199b67 --- /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 8fa9110f2..6ee1ddef7 100644 --- 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 202258c94..64ba56bd9 100644 --- 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 @@ + + + + + + + + + + + + + + + + + + + + + + -- 2.39.5