diff options
26 files changed, 696 insertions, 221 deletions
diff --git a/bridge/src/org/aspectj/bridge/Message.java b/bridge/src/org/aspectj/bridge/Message.java index 02ecbb820..610bc9058 100644 --- a/bridge/src/org/aspectj/bridge/Message.java +++ b/bridge/src/org/aspectj/bridge/Message.java @@ -37,7 +37,7 @@ public class Message implements IMessage { /** * Create a (compiler) error or warning message * @param message the String used as the underlying message - * @param sourceLocation the ISourceLocation, if any, associated with this message + * @param location the ISourceLocation, if any, associated with this message * @param isError if true, use IMessage.ERROR; else use IMessage.WARNING */ public Message(String message, ISourceLocation location, boolean isError) { diff --git a/build/build-common.xml b/build/build-common.xml index e254ab608..5c4090c1f 100644 --- a/build/build-common.xml +++ b/build/build-common.xml @@ -107,18 +107,26 @@ </classpath> <test name="@{suite}"/> </junit> - <!-- TODO AV - using java to invoke JUnit since the junit task is hidding errors - don't know why --> - <!--<java classname="@{suite}" fork="on" dir="../@{project}"> + </sequential> + </macrodef> + + <macrodef name="testrun2"> + <attribute name="project"/> + <attribute name="path"/> + <attribute name="suite"/> + <sequential> + <java classname="@{suite}" fork="on" dir="../@{project}"> <jvmarg line=""/> <classpath> <pathelement path="../@{project}/${build.dir}"/> <pathelement path="../@{project}/${test.build.dir}"/> <path refid="@{path}"/> </classpath> - </java>--> + </java> </sequential> </macrodef> + <target name="all" depends="init, compile, test:compile"/> diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java index 605955cc9..ad654f8e3 100644 --- a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java +++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java @@ -46,11 +46,21 @@ public class Ajc { private static final String TESTER_PATH = ".."+File.separator+"testing-client"+File.separator+"bin" + File.pathSeparator + ".."+File.separator+"runtime"+File.separator+"bin" + File.pathSeparator + + //FIXME AV - can someone tell why those jar needs to be there ?? + // 1/ see the line before: bin/ will take precedence.. + // 2/ see below - aspectj5rt is added last which makes it UNconsistent with the way runtime is handled here.. ".."+File.separator+"lib"+File.separator+"test"+File.separator+"aspectjrt.jar"+ File.pathSeparator+ ".."+File.separator+"lib"+File.separator+"test"+File.separator+"testing-client.jar" + File.pathSeparator + - ".."+File.separator+"aspectj5rt"+File.separator+"bin" + File.pathSeparator + ".."+File.separator+"aspectj5rt"+File.separator+"bin" + File.pathSeparator + //Alex: adding "_IDE" since there is no "bin" output when working within IDEA. + // my convention is thus to have a "modules/_IDE" folder where IDEA will write + // Since modules/* have circular dependancies, there is no way to have multiple "modules" + // (like Eclipse projects in one workspace) in IDEA, so all will be build there. + // Note: adding it last means that a change in the IDE aspectj5rt module f.e. without + // "ant compile" to rebuild "aspect5rt/bin" will not expose the IDE changes... + // but I don't want to have it first to avoid side effects when running from Ant. + + File.pathSeparator + ".." + File.separator + "_IDE" + File.pathSeparator+ ".."+File.separator+"lib"+File.separator+"junit"+File.separator+"junit.jar" - + File.pathSeparator+".."+File.separator+"aspectj5rt"+File.separator+"bin" ; private CompilationResult result; private File sandbox; diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/AjcTestCase.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/AjcTestCase.java index 111b631a1..77e8a3b40 100644 --- a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/AjcTestCase.java +++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/AjcTestCase.java @@ -68,6 +68,14 @@ public class AjcTestCase extends TestCase { File.pathSeparator + ".." + File.separator + "util" + File.separator + "bin" + File.pathSeparator + ".." + File.separator + "aspectj5rt" + File.separator + "bin" + + //Alex: adding "_IDE" since there is no "bin" output when working within IDEA. + // my convention is thus to have a "modules/_IDE" folder where IDEA will write + // Since modules/* have circular dependancies, there is no way to have multiple "modules" + // (like Eclipse projects in one workspace) in IDEA, so all will be build there. + // Note: adding it last means that a change in the IDE aspectj5rt module f.e. without + // "ant compile" to rebuild "aspect5rt/bin" will not expose the IDE changes... + // but I don't want to have it first to avoid side effects when running from Ant. + File.pathSeparator + ".." + File.separator + "_IDE" + File.pathSeparator+ ".."+File.separator+"lib"+File.separator+"junit"+File.separator+"junit.jar"; /** diff --git a/run-all-junit-tests/build.xml b/run-all-junit-tests/build.xml index 5fc2499d8..9fbf00812 100644 --- a/run-all-junit-tests/build.xml +++ b/run-all-junit-tests/build.xml @@ -5,6 +5,7 @@ <import file="../ajbrowser/build.xml"/> <import file="../ajde/build.xml"/> <import file="../asm/build.xml"/> + <import file="../aspectj5rt/build.xml"/> <import file="../bridge/build.xml"/> <import file="../org.aspectj.ajdt.core/build.xml"/> <import file="../runtime/build.xml"/> @@ -26,6 +27,8 @@ <pathelement path="../ajbrowser/bin"/> <pathelement path="../asm/bintest"/> <pathelement path="../asm/bin"/> + <pathelement path="../aspectj5rt/bintest"/> + <pathelement path="../aspectj5rt/bin"/> <pathelement path="../bridge/bintest"/> <pathelement path="../bridge/bin"/> <pathelement path="../org.aspectj.ajdt.core/bintest"/> @@ -71,6 +74,7 @@ ajde.test:compile, asm.compile, asm.test:compile, + aspectj5rt.compile, bridge.compile, bridge.test:compile, org.aspectj.ajdt.core.compile, @@ -90,6 +94,7 @@ util.test:compile, weaver.compile, weaver.test:compile"> + <ant antfile="../aspectj5rt/build.xml" target="test:compile"/> <testcompile project="run-all-junit-tests" path="run-all-junit-tests.test.src.path"/> </target> diff --git a/taskdefs/src/org/aspectj/tools/ant/taskdefs/AjcTask.java b/taskdefs/src/org/aspectj/tools/ant/taskdefs/AjcTask.java index a9d4dde48..56125af4d 100644 --- a/taskdefs/src/org/aspectj/tools/ant/taskdefs/AjcTask.java +++ b/taskdefs/src/org/aspectj/tools/ant/taskdefs/AjcTask.java @@ -1142,7 +1142,8 @@ public class AjcTask extends MatchingTask { sb.append("inpathDirCopyFilter"); } else { sb.append("sourceRootCopyFilter and inpathDirCopyFilter"); - }
sb.append(REQ); + } + sb.append(REQ); } if (0 < sb.length()) { throw new BuildException(sb.toString()); @@ -1378,7 +1379,7 @@ public class AjcTask extends MatchingTask { // replace above two lines with what follows as an aid to debugging when running the unit tests.... // LogStreamHandler handler = new LogStreamHandler(this, // Project.MSG_INFO, Project.MSG_WARN) { -// +// // ByteArrayOutputStream baos = new ByteArrayOutputStream(); // /* (non-Javadoc) // * @see org.apache.tools.ant.taskdefs.PumpStreamHandler#createProcessOutputPump(java.io.InputStream, java.io.OutputStream) @@ -1387,7 +1388,7 @@ public class AjcTask extends MatchingTask { // OutputStream os) { // super.createProcessErrorPump(is, baos); // } -// +// // /* (non-Javadoc) // * @see org.apache.tools.ant.taskdefs.LogStreamHandler#stop() // */ diff --git a/testing/newsrc/org/aspectj/testing/AutowiredXMLBasedAjcTestCase.java b/testing/newsrc/org/aspectj/testing/AutowiredXMLBasedAjcTestCase.java new file mode 100644 index 000000000..f77e289d7 --- /dev/null +++ b/testing/newsrc/org/aspectj/testing/AutowiredXMLBasedAjcTestCase.java @@ -0,0 +1,139 @@ +/******************************************************************************* + * Copyright (c) 2005 Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Common Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * initial implementation Alexandre Vasseur + *******************************************************************************/ +package org.aspectj.testing; + +import junit.framework.Test; +import junit.framework.TestCase; +import junit.framework.TestSuite; +import junit.extensions.TestSetup; + +import java.lang.reflect.Method; +import java.util.Iterator; +import java.util.Map; +import java.util.HashMap; +import java.io.InputStreamReader; +import java.io.FileInputStream; + +import org.apache.commons.digester.Digester; +import org.aspectj.tools.ajc.Ajc; + +/** + * Autowiring of XML test spec file as JUnit tests. + * <p/> + * Extend this class and implement the getSpecFile and the static suite() method. + * All tests described in the XML spec file will be auto-registered as JUnit tests. + * Any regular test() method will be registered as well. + * + * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> + */ +public abstract class AutowiredXMLBasedAjcTestCase extends XMLBasedAjcTestCase { + + private Map testMap = new HashMap(); + + public void addTest(AjcTest test) { + testMap.put(test.getTitle(), test); + } + + /* + * Return a map from (String) test title -> AjcTest + */ + protected Map getSuiteTests() { + return testMap; + } + + public static Test loadSuite(Class testCaseClass) { + TestSuite suite = new TestSuite(testCaseClass.getName()); + //suite.addTestSuite(testCaseClass); + + // wire the spec file + try { + final AutowiredXMLBasedAjcTestCase wired = (AutowiredXMLBasedAjcTestCase) testCaseClass.newInstance(); + System.out.println("LOADING SUITE: " + wired.getSpecFile().getPath()); + Digester d = wired.getDigester(); + try { + InputStreamReader isr = new InputStreamReader(new FileInputStream(wired.getSpecFile())); + d.parse(isr); + } catch (Exception ex) { + fail("Unable to load suite " + wired.getSpecFile().getPath() + " : " + ex); + } + wired.ajc = new Ajc(); + + Map ajTests = wired.getSuiteTests(); + + for (Iterator iterator = ajTests.entrySet().iterator(); iterator.hasNext();) { + final Map.Entry entry = (Map.Entry) iterator.next(); + + suite.addTest( + new TestCase(entry.getKey().toString()) { + + protected void runTest() { + ((AjcTest) entry.getValue()).runTest(wired); + } + + public String getName() { + return (String) entry.getKey(); + } + } + ); + } + } catch (Throwable t) { + final String message = t.toString(); + suite.addTest( + new TestCase("error") { + protected void runTest() { + fail(message); + } + } + ); + } + + // wire the test methods as well if any + // this simple check avoids failure when no test.. method is found. + // it could be refined to lookup in the hierarchy as well, and excluding private method as JUnit does. + Method[] testMethods = testCaseClass.getDeclaredMethods(); + for (int i = 0; i < testMethods.length; i++) { + Method testMethod = testMethods[i]; + if (testMethod.getName().startsWith("test")) { + suite.addTestSuite(testCaseClass); + break; + } + } + + TestSetup wrapper = new TestSetup(suite) { + /* (non-Javadoc) + * @see junit.extensions.TestSetup#setUp() + */ + protected void setUp() throws Exception { + super.setUp(); + //suiteLoaded = false; + } + /* (non-Javadoc) + * @see junit.extensions.TestSetup#tearDown() + */ + protected void tearDown() throws Exception { + super.tearDown(); + //suiteLoaded = false; + } + }; + return wrapper; + + //return suite; + } + + /* (non-Javadoc) + * @see org.aspectj.tools.ajc.AjcTestCase#setUp() + */ + protected void setUp() throws Exception { + super.setUp(); + } + + +} diff --git a/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java b/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java index 32379c775..91ded4c1a 100644 --- a/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java +++ b/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java @@ -16,10 +16,12 @@ import java.io.FileInputStream; import java.io.InputStreamReader; import java.util.HashMap; import java.util.Map; +import java.util.Iterator; import junit.extensions.TestSetup; import junit.framework.Test; import junit.framework.TestSuite; +import junit.framework.TestCase; import org.apache.commons.digester.Digester; import org.aspectj.tools.ajc.AjcTestCase; @@ -85,7 +87,7 @@ public abstract class XMLBasedAjcTestCase extends AjcTestCase { /* * Return a map from (String) test title -> AjcTest */ - private Map getSuiteTests() { + protected Map getSuiteTests() { return testMap; } @@ -125,7 +127,7 @@ public abstract class XMLBasedAjcTestCase extends AjcTestCase { * in the XML document to properties in the associated classes, so this simple implementation should * be very easy to maintain and extend should you ever need to. */ - private Digester getDigester() { + protected Digester getDigester() { Digester digester = new Digester(); digester.push(this); digester.addObjectCreate("suite/ajc-test",AjcTest.class); @@ -173,7 +175,6 @@ public abstract class XMLBasedAjcTestCase extends AjcTestCase { suiteLoaded = true; } } - protected long nextIncrement(boolean doWait) { long time = System.currentTimeMillis(); diff --git a/tests/java5/ataspectj/SimpleBefore.java b/tests/java5/ataspectj/SimpleBefore.java index 8b5019f94..415cc31db 100644 --- a/tests/java5/ataspectj/SimpleBefore.java +++ b/tests/java5/ataspectj/SimpleBefore.java @@ -1,8 +1,18 @@ import org.aspectj.lang.annotation.*; +import java.lang.annotation.Annotation; + public class SimpleBefore { public static void main(String []argv) { + + Class x = X.class; + Aspect ann = (Aspect) x.getAnnotation(Aspect.class); + if (ann == null) { + throw new RuntimeException("could not see runtime visible annotation"); + } + + SimpleBefore instance = new SimpleBefore(); X.s.append("1"); instance.m(); diff --git a/tests/java5/ataspectj/ataspectj/PrecedenceTest.java b/tests/java5/ataspectj/ataspectj/PrecedenceTest.java index c5959fc4b..fe96161f8 100644 --- a/tests/java5/ataspectj/ataspectj/PrecedenceTest.java +++ b/tests/java5/ataspectj/ataspectj/PrecedenceTest.java @@ -18,6 +18,8 @@ import org.aspectj.lang.annotation.DeclarePrecedence; import org.aspectj.lang.annotation.Aspect; import org.aspectj.lang.annotation.Before; +import java.lang.annotation.Annotation; + /** * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> */ @@ -40,6 +42,11 @@ public class PrecedenceTest extends TestCase { log("hello"); } + public void testRuntimeVisible() { + Annotation annotation = TestAspect_Order.class.getAnnotation(DeclarePrecedence.class); + assertNotNull(annotation); + } + public void testPrecedence() { s_log = new StringBuffer(); hello(); diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test005.java b/tests/java5/ataspectj/ataspectj/misuse/Test005.java new file mode 100644 index 000000000..428e002dd --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test005.java @@ -0,0 +1,14 @@ +// "@Aspect class extending @Aspect class" +package ataspectj.misuse; + +import org.aspectj.lang.annotation.*; + +@Aspect +public class Test005 { + + @Aspect + public static class Test005B extends Test005 { + } + + +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test006.java b/tests/java5/ataspectj/ataspectj/misuse/Test006.java new file mode 100644 index 000000000..5b342b6a8 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test006.java @@ -0,0 +1,15 @@ +// "class with @Before extending @Aspect class" + +// shouldn't allow advice in a non-aspect type +package ataspectj.misuse; + +import org.aspectj.lang.annotation.*; + +@Aspect +public class Test006{ +} +class Test006B extends Test006{ + @Before("call(* org..*(..))") + public void someCall(){ + } +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test008.java b/tests/java5/ataspectj/ataspectj/misuse/Test008.java new file mode 100644 index 000000000..04dbdb1f2 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test008.java @@ -0,0 +1,14 @@ +// "@Pointcut not returning void" +package ataspectj.misuse; + + +import org.aspectj.lang.annotation.*; + + +@Aspect +class Test008{ + @Pointcut("call(* *.*(..))") + int someCall(){ + return 42; + } +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test010.java b/tests/java5/ataspectj/ataspectj/misuse/Test010.java new file mode 100644 index 000000000..8656424c4 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test010.java @@ -0,0 +1,8 @@ +// "@Aspect on interface" +package ataspectj.misuse; + +import org.aspectj.lang.annotation.*; + +@Aspect +interface Test010{ +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test014.java b/tests/java5/ataspectj/ataspectj/misuse/Test014.java new file mode 100644 index 000000000..9e135a93c --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test014.java @@ -0,0 +1,11 @@ +// "@Pointcut with garbage string" +package ataspectj.misuse; + +import org.aspectj.lang.annotation.*; + +@Aspect +public class Test014{ + @Pointcut("call%dddd\n\n\n\n\n\n\n\n\n\n\n%dwdwudwdwbuill817pe;][{\ngrgrgnjk78877&&<:{{{+=``\"") + void somecall(){ + } +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test016.java b/tests/java5/ataspectj/ataspectj/misuse/Test016.java new file mode 100644 index 000000000..8a50afa74 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test016.java @@ -0,0 +1,10 @@ +// "@Pointcut with throws clause" +package ataspectj.misuse; + +import org.aspectj.lang.annotation.*; + +@Aspect +public class Test016{ + @Pointcut("call(* *.*(..))") + void someCall() throws Exception {} +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test019.java b/tests/java5/ataspectj/ataspectj/misuse/Test019.java new file mode 100644 index 000000000..301c3de35 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test019.java @@ -0,0 +1,11 @@ +// "@AfterReturning with wrong number of args" +package ataspectj.misuse; + +import org.aspectj.lang.annotation.*; + +@Aspect +public class Test019 { + @AfterReturning(value="call(* *..*(..))",returning="f") + public void itsAFoo(Object f, int x) { + } +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test020.java b/tests/java5/ataspectj/ataspectj/misuse/Test020.java new file mode 100644 index 000000000..044f527d8 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test020.java @@ -0,0 +1,11 @@ +// "@Before on non-public method" +package ataspectj.misuse; + +import org.aspectj.lang.annotation.*; + +@Aspect +public class Test020 { + @Before("call(* org..*(..))") + private void someCall(){ + } +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test021.java b/tests/java5/ataspectj/ataspectj/misuse/Test021.java new file mode 100644 index 000000000..ac230adee --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test021.java @@ -0,0 +1,12 @@ +// "@Before on method not returning void" +package ataspectj.misuse; + +import org.aspectj.lang.annotation.*; + +@Aspect +public class Test021 { + @Before("call(* org..*(..))") + public int someCall(){ + return 42; + } +} diff --git a/tests/src/org/aspectj/systemtest/ajc150/AllTestsAspectJ150.java b/tests/src/org/aspectj/systemtest/ajc150/AllTestsAspectJ150.java index 1f0ad25f9..c71386e41 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/AllTestsAspectJ150.java +++ b/tests/src/org/aspectj/systemtest/ajc150/AllTestsAspectJ150.java @@ -11,6 +11,7 @@ package org.aspectj.systemtest.ajc150; import org.aspectj.systemtest.ajc150.ataspectj.AtAjSyntaxTests; +import org.aspectj.systemtest.ajc150.ataspectj.AtAjMisuseTests; import junit.framework.Test; import junit.framework.TestSuite; @@ -44,6 +45,7 @@ public class AllTestsAspectJ150 { suite.addTest(DeclareAnnotationTests.suite()); suite.addTest(GenericsTests.suite()); suite.addTest(AtAjSyntaxTests.suite()); + suite.addTest(AtAjMisuseTests.suite()); //$JUnit-END$ return suite; } diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjMisuseTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjMisuseTests.java new file mode 100644 index 000000000..e1e2ce7f0 --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjMisuseTests.java @@ -0,0 +1,32 @@ +/******************************************************************************* + * Copyright (c) 2005 Contributors. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Common Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * Alexandre Vasseur initial implementation + *******************************************************************************/ +package org.aspectj.systemtest.ajc150.ataspectj; + +import org.aspectj.testing.AutowiredXMLBasedAjcTestCase; +import org.aspectj.testing.XMLBasedAjcTestCase; + +import java.io.File; + +import junit.framework.Test; + +/** + * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> + */ +public class AtAjMisuseTests extends AutowiredXMLBasedAjcTestCase { + + protected File getSpecFile() { + return new File("../tests/src/org/aspectj/systemtest/ajc150/ataspectj/misuse.xml"); + } + + public static Test suite() { + return AutowiredXMLBasedAjcTestCase.loadSuite(AtAjMisuseTests.class); + } +} diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java index 19d6dc8eb..f1255b6aa 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java @@ -35,23 +35,23 @@ public class AtAjSyntaxTests extends XMLBasedAjcTestCase { public void testSimpleBefore() { runTest("SimpleBefore"); } - + public void testSimpleAfter() { runTest("SimpleAfter"); } - + public void testSingletonAspectBinding() { runTest("singletonAspectBindings"); } - + public void testCflow() { runTest("CflowTest"); } - + public void testPointcutReference() { runTest("PointcutReferenceTest"); } - + public void testXXJoinPoint() { runTest("XXJoinPointTest"); } diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/atajc150-tests.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/atajc150-tests.xml index 0c436854b..87c0be982 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/atajc150-tests.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/atajc150-tests.xml @@ -2,11 +2,11 @@ <ajc-test dir="java5/ataspectj" title="SimpleBefore"> <compile files="SimpleBefore.java" options="-1.5 -showWeaveInfo -XnoInline"> - <message kind="weave" text="(SimpleBefore.java:13) advised by before advice from 'SimpleBefore$X' (SimpleBefore.java:1)"/> + <message kind="weave" text="(SimpleBefore.java:23) advised by before advice from 'SimpleBefore$X' (SimpleBefore.java:1)"/> </compile> <run class="SimpleBefore"/> <compile files="SimpleBefore.java" options="-1.5 -showWeaveInfo -XnoInline -Xdev:NoAtAspectJProcessing"> - <message kind="weave" text="(SimpleBefore.java:13) advised by before advice from 'SimpleBefore$X' (SimpleBefore.java:1)"/> + <message kind="weave" text="(SimpleBefore.java:23) advised by before advice from 'SimpleBefore$X' (SimpleBefore.java:1)"/> </compile> <run class="SimpleBefore"/> </ajc-test> diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/coverage/CoverageTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/coverage/CoverageTests.java index 4d2c960b3..a0f75eac7 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/coverage/CoverageTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/coverage/CoverageTests.java @@ -7,178 +7,15 @@ import junit.framework.Test; import org.aspectj.testing.XMLBasedAjcTestCase; -public class CoverageTests extends org.aspectj.testing.XMLBasedAjcTestCase { +public class CoverageTests extends org.aspectj.testing.AutowiredXMLBasedAjcTestCase { public static Test suite() { - return XMLBasedAjcTestCase.loadSuite(CoverageTests.class); + return org.aspectj.testing.AutowiredXMLBasedAjcTestCase.loadSuite(CoverageTests.class); } protected File getSpecFile() { return new File("../tests/src/org/aspectj/systemtest/ajc150/ataspectj/coverage/coverage.xml"); } - - public void test001(){ - runTest("@Aspect extending Aspect"); - } - - public void test002(){ - runTest("@Aspect with codestyle pointcut"); - } - - public void test003(){ - runTest("Codestyle Aspect with @Pointcut"); -} - - public void test004(){ - runTest("@Pointcut declared on codestyle advice"); -} - - public void test005(){ - runTest("@Aspect class extending @Aspect class"); -} - - public void test006(){ - runTest("class with @Before extending @Aspect class"); -} - - public void test007(){ - runTest("@Before declared on codestyle advice"); -} - - public void test008(){ - runTest("@Pointcut not returning void"); -} - - public void test009(){ - runTest("@Pointcut on @Aspect class constructor"); -} - - public void test010(){ - runTest("@Aspect on interface"); -} - - public void test011(){ - runTest("@Pointcut on non-aspect class method"); -} - - public void test012(){ - runTest("@Before on non-aspect class method"); -} - - public void test013(){ - runTest("@Pointcut on Interface method"); -} - - public void test014(){ - runTest("@Pointcut with garbage string"); -} - - public void test015(){ - runTest("@Pointcut with non-empty method body"); -} - - - public void test016(){ - runTest("@Pointcut with throws clause"); -} - - public void test017(){ - runTest("@Aspect used badly"); -} - - public void test018(){ - runTest("@Before declared on @Aspect class constructor"); -} - - public void test019(){ - runTest("@AfterReturning with wrong number of args"); -} - - public void test020(){ - runTest("@Before on non-public method"); -} - - public void test021(){ - runTest("@Before on method not returning void"); -} - public void test022(){ - runTest("@Pointcut with wrong number of args"); -} - public void test023(){ - runTest("@DeclareParents with interface extending interface"); -} - public void test024(){ - runTest("@DeclareParents implementing more than one interface"); -} - public void test025(){ - runTest("@DeclareParents used outside of an Aspect"); -} - public void test026(){ - runTest("@DeclareParents on an @Aspect"); -} - public void test027(){ - runTest("@DeclareParents on an @Aspect with @DeclarePrecidence"); -} - public void test028(){ - runTest("@DeclareWarning with a non-final String"); -} - public void test029(){ - runTest("@DeclareWarning with a static final Object (that is a String)"); -} - public void test030(){ - runTest("@DeclareWarning with a static final Integer"); -} - public void test031(){ - runTest("@Around given an extension of ProceedingJoinPoint"); -} - public void test032(){ - runTest("calling @Before advice explicitly as a method"); -} - public void test033(){ - runTest("@Before on Interface method"); -} - public void test034(){ - runTest("@Aspect Aspect double declaration"); -} - public void test035(){ - runTest("@Before and @After on one method"); -} - public void test036(){ - runTest("@Before twice on one method"); -} - public void test037(){ - runTest("@Before advice with empty string"); -} -public void test038(){ - runTest("isPrivileged=truu misspelling"); -} - -public void test039(){ - runTest("@Pointcut with an empty string"); -} - -public void test040(){ - runTest("@Before with && in string"); -} - -public void test041(){ - runTest("@AdviceName given an empty string"); -} - -public void test042(){ - runTest("@AdviceName used on @Before advice"); -} - -public void test043(){ - runTest("The Moody example"); -} - -public void test044(){ - runTest("@DeclareWarning"); -} - - - } diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/misuse.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/misuse.xml new file mode 100644 index 000000000..1717bf9e0 --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/misuse.xml @@ -0,0 +1,134 @@ +<!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd"[]> + +<!-- AspectJ v1.5.0 Tests --> + +<suite> + + <comment>this one is ok - too simple - could be removed..</comment> + <ajc-test dir="java5/ataspectj" + pr="" title="@Aspect class extending @Aspect class"> + <compile files="ataspectj/misuse/Test005.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + </compile> + </ajc-test> + + <comment>just a warning - might be skept if further optimized in Aj5Attributes..</comment> + <ajc-test dir="java5/ataspectj" + pr="" title="class with @Before extending @Aspect class"> + <compile files="ataspectj/misuse/Test006.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="warning" line="11" text="Found @AspectJ annotations in a non @Aspect type 'ataspectj.misuse.Test006B'"/> + </compile> + </ajc-test> + + <comment>a warning. We ignore the pointcut (TBD) - line is enclosing class (TBD Andy do better ?)</comment> + <ajc-test dir="java5/ataspectj" + pr="" title="@Pointcut not returning void"> + <compile files="ataspectj/misuse/Test008.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="warning" line="9" text="Found @Pointcut on a method not returning void 'someCall()I'"/> + </compile> + </ajc-test> + + +<!-- <ajc-test dir="java5/ataspectj"--> +<!-- pr="" title="@Aspect on interface">--> +<!-- <compile files="ataspectj/misuse/Test010.java" options="-1.5 -Xdev:NoAtAspectJProcessing">--> +<!-- <message kind="warning" line="7" text="Found @Aspect on an interface type 'ataspectj.misuse.Test010'"/>--> +<!-- </compile>--> +<!-- </ajc-test>--> + + <comment>line is enclosing class - TBD</comment> + <ajc-test dir="java5/ataspectj" + pr="" title="@Pointcut with garbage string"> + <compile files="ataspectj/misuse/Test014.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="error" line="7" text="Cannot parse @Pointcut 'call%dddd"/> + </compile> + </ajc-test> + + <comment>line is enclosing class - TBD</comment> + <ajc-test dir="java5/ataspectj" + pr="" title="@Pointcut with throws clause"> + <compile files="ataspectj/misuse/Test016.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="warning" line="7" text="Found @Pointcut on a method throwing exception 'someCall()V'"/> + </compile> + </ajc-test> + + <comment>very dirty hack - can't get this location to work properly so added match all error..</comment> + <ajc-test dir="java5/ataspectj" + pr="" title="@AfterReturning with wrong number of args"> + <compile files="ataspectj/misuse/Test019.java" options="-1.5 -Xdev:NoAtAspectJProcessing -Xlint:ignore"> + <message kind="error" line="1" text="the parameter x is not bound"/> + <message kind="error"/> + </compile> + </ajc-test> + + <comment>line number is enclosing type</comment> + <ajc-test dir="java5/ataspectj" + pr="" title="@Before on non-public method"> + <compile files="ataspectj/misuse/Test020.java" options="-1.5 -Xdev:NoAtAspectJProcessing -Xlint:ignore"> + <message kind="error" line="7" text="Found @AspectJ annotation on a non public advice 'someCall()V'"/> + </compile> + </ajc-test> + + <comment>line number is enclosing type</comment> + <ajc-test dir="java5/ataspectj" + pr="" title="@Before on method not returning void"> + <compile files="ataspectj/misuse/Test021.java" options="-1.5 -Xdev:NoAtAspectJProcessing -Xlint:ignore"> + <message kind="error" line="7" text="Found @AspectJ annotation on a non around advice not returning void 'someCall()I'"/> + </compile> + </ajc-test> + +<!-- +ALEX: todo + <ajc-test dir="java5/ataspectj/coverage" + pr="" title="@Pointcut with wrong number of args"> + <compile files="ataspectj/misuse/Test022.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="error" line="8" text="int x is not declared in the pointcut parameters"/> + </compile> + </ajc-test> + + <ajc-test dir="java5/ataspectj/coverage" + pr="" title="@Around given an extension of ProceedingJoinPoint"> + <compile files="ataspectj/misuse/Test031.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="error" line="9" text="Is this an error?"/> + </compile> + </ajc-test> + + <ajc-test dir="java5/ataspectj/coverage" + pr="" title="calling @Before advice explicitly as a method"> + <compile files="ataspectj/misuse/Test032.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="error" line="14" text="Advice should never be called explicitly"/> + </compile> + </ajc-test> + + <ajc-test dir="java5/ataspectj/coverage" + pr="" title="@Before on Interface method"> + <compile files="ataspectj/misuse/Test033.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="error" line="4" text="The annotation @Before is disallowed for this location"/> + </compile> + </ajc-test> + + <ajc-test dir="java5/ataspectj/coverage" + pr="" title="@Before and @After on one method"> + <compile files="ataspectj/misuse/Test035.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + <message kind="error" line="7" text="A method may only be declared as advice once"/> + </compile> + </ajc-test> + + <ajc-test dir="java5/ataspectj/coverage" + pr="" title="@Before advice with empty string"> + <compile files="ataspectj/misuse/Test037.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + </compile> + </ajc-test> + + <ajc-test dir="java5/ataspectj/coverage" + pr="" title="@Pointcut with an empty string"> + <compile files="ataspectj/misuse/Test039.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + </compile> + </ajc-test> + + <ajc-test dir="java5/ataspectj/coverage" + pr="" title="@Before with AND in string"> + <compile files="ataspectj/misuse/Test040.java" options="-1.5 -Xdev:NoAtAspectJProcessing"> + </compile> + </ajc-test> +--> +</suite> diff --git a/weaver/src/org/aspectj/weaver/ataspectj/Aj5Attributes.java b/weaver/src/org/aspectj/weaver/ataspectj/Aj5Attributes.java index 2eb120ecd..290e32f61 100644 --- a/weaver/src/org/aspectj/weaver/ataspectj/Aj5Attributes.java +++ b/weaver/src/org/aspectj/weaver/ataspectj/Aj5Attributes.java @@ -1,12 +1,13 @@ /******************************************************************************* - * Copyright (c) 2005 Contributors - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the Common Public License v1.0 - * which accompanies this distribution, and is available at - * http://www.eclipse.org/legal/cpl-v10.html + * 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: - * initial implementation Jonas Bonér, Alexandre Vasseur + * initial implementation Alexandre Vasseur *******************************************************************************/ package org.aspectj.weaver.ataspectj; @@ -25,9 +26,11 @@ import org.aspectj.apache.bcel.classfile.Method; import org.aspectj.apache.bcel.classfile.annotation.Annotation; import org.aspectj.apache.bcel.classfile.annotation.ElementNameValuePair; import org.aspectj.apache.bcel.classfile.annotation.RuntimeAnnotations; +import org.aspectj.apache.bcel.classfile.annotation.RuntimeVisibleAnnotations; import org.aspectj.apache.bcel.generic.Type; import org.aspectj.bridge.IMessageHandler; import org.aspectj.bridge.Message; +import org.aspectj.bridge.IMessage; import org.aspectj.lang.JoinPoint; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.weaver.Advice; @@ -50,6 +53,7 @@ import org.aspectj.weaver.patterns.PerSingleton; import org.aspectj.weaver.patterns.PerTypeWithin; import org.aspectj.weaver.patterns.Pointcut; import org.aspectj.weaver.patterns.SimpleScope; +import org.aspectj.weaver.patterns.ParserException; /** * Annotation defined aspect reader. @@ -124,13 +128,13 @@ public class Aj5Attributes { } /** - * Annotations are RV or RIV for now we don't care + * Annotations are RuntimeVisible only. This allow us to not visit RuntimeInvisible ones. * * @param attribute * @return */ public static boolean acceptAttribute(Attribute attribute) { - return (attribute instanceof RuntimeAnnotations); + return (attribute instanceof RuntimeVisibleAnnotations); } /** @@ -145,15 +149,58 @@ public class Aj5Attributes { public static List readAj5ClassAttributes(JavaClass javaClass, ResolvedTypeX type, ISourceContext context,IMessageHandler msgHandler, boolean isCodeStyleAspect) { AjAttributeStruct struct = new AjAttributeStruct(type, context, msgHandler); Attribute[] attributes = javaClass.getAttributes(); + boolean hasAtAspectAnnotation = false; + boolean hasAtPrecedenceAnnotation = false; + for (int i = 0; i < attributes.length; i++) { Attribute attribute = attributes[i]; if (acceptAttribute(attribute)) { RuntimeAnnotations rvs = (RuntimeAnnotations) attribute; - if (!isCodeStyleAspect) handleAspectAnnotation(rvs, struct); - handlePrecedenceAnnotation(rvs, struct); + // we don't need to look for several attribute occurence since it cannot happen as per JSR175 + if (!isCodeStyleAspect) { + hasAtAspectAnnotation = handleAspectAnnotation(rvs, struct); + } + //TODO: below means mix style for @DeclarePrecedence - are we sure we want that ? + hasAtPrecedenceAnnotation = handlePrecedenceAnnotation(rvs, struct); + // there can only be one RuntimeVisible bytecode attribute + break; } } + // basic semantic check + //FIXME AV TBD could be skipped and silently ignore - TBD with Andy + if (hasAtPrecedenceAnnotation && !hasAtAspectAnnotation) { + msgHandler.handleMessage( + new Message( + "Found @DeclarePrecedence on a non @Aspect type '" + type.getName() + "'", + IMessage.WARNING, + null, + type.getSourceLocation() + ) + ); + // bypass what we have read + return EMPTY_LIST; + } + //FIXME turn on when ajcMightHaveAspect fixed +// if (hasAtAspectAnnotation && type.isInterface()) { +// msgHandler.handleMessage( +// new Message( +// "Found @Aspect on an interface type '" + type.getName() + "'", +// IMessage.WARNING, +// null, +// type.getSourceLocation() +// ) +// ); +// // bypass what we have read +// return EMPTY_LIST; +// } + + // the following block will not detect @Pointcut in non @Aspect types for optimization purpose + // FIXME AV TBD with Andy + if (!hasAtAspectAnnotation) { + return EMPTY_LIST; + } + // code style pointcuts are class attributes // we need to gather the @AJ pointcut right now and not at method level annotation extraction time // in order to be able to resolve the pointcut references later on @@ -171,11 +218,12 @@ public class Aj5Attributes { if (acceptAttribute(mattribute)) { RuntimeAnnotations mrvs = (RuntimeAnnotations) mattribute; handlePointcutAnnotation(mrvs, mstruct); + // there can only be one RuntimeVisible bytecode attribute + break; } } struct.ajAttributes.addAll(mstruct.ajAttributes); } - return struct.ajAttributes; } @@ -194,17 +242,66 @@ public class Aj5Attributes { AjAttributeMethodStruct struct = new AjAttributeMethodStruct(method, type, context, msgHandler); Attribute[] attributes = method.getAttributes(); + // we remember if we found one @AJ annotation for minimal semantic error reporting + // the real reporting beeing done thru AJDT and the compiler mapping @AJ to AjAtttribute + // or thru APT + // FIXME AV we could actually skip the whole thing if type is not itself an @Aspect + // but then we would not see any warning. TBD with Andy + boolean hasAtAspectJAnnotation = false; + boolean hasAtAspectJAnnotationMustReturnVoid = false; for (int i = 0; i < attributes.length; i++) { Attribute attribute = attributes[i]; if (acceptAttribute(attribute)) { RuntimeAnnotations rvs = (RuntimeAnnotations) attribute; - handleBeforeAnnotation(rvs, struct, preResolvedPointcut); - handleAfterAnnotation(rvs, struct, preResolvedPointcut); - handleAfterReturningAnnotation(rvs, struct, preResolvedPointcut); - handleAfterThrowingAnnotation(rvs, struct, preResolvedPointcut); - handleAroundAnnotation(rvs, struct, preResolvedPointcut); + hasAtAspectJAnnotationMustReturnVoid = hasAtAspectJAnnotationMustReturnVoid || handleBeforeAnnotation(rvs, struct, preResolvedPointcut); + hasAtAspectJAnnotationMustReturnVoid = hasAtAspectJAnnotationMustReturnVoid || handleAfterAnnotation(rvs, struct, preResolvedPointcut); + hasAtAspectJAnnotationMustReturnVoid = hasAtAspectJAnnotationMustReturnVoid || handleAfterReturningAnnotation(rvs, struct, preResolvedPointcut); + hasAtAspectJAnnotationMustReturnVoid = hasAtAspectJAnnotationMustReturnVoid || handleAfterThrowingAnnotation(rvs, struct, preResolvedPointcut); + hasAtAspectJAnnotation = hasAtAspectJAnnotation || handleAroundAnnotation(rvs, struct, preResolvedPointcut); + // there can only be one RuntimeVisible bytecode attribute + break; } } + hasAtAspectJAnnotation = hasAtAspectJAnnotation || hasAtAspectJAnnotationMustReturnVoid; + + // semantic check - must be in an @Aspect [remove if previous block bypassed in advance] + if (hasAtAspectJAnnotation && !type.isAnnotationStyleAspect()) { + msgHandler.handleMessage( + new Message( + "Found @AspectJ annotations in a non @Aspect type '" + type.getName() + "'", + IMessage.WARNING, + null, + type.getSourceLocation() + ) + ); + ;// go ahead + } + // semantic check - advice must be public + if (hasAtAspectJAnnotation && !struct.method.isPublic()) { + msgHandler.handleMessage( + new Message( + "Found @AspectJ annotation on a non public advice '" + methodToString(struct.method) + "'", + IMessage.ERROR, + null, + type.getSourceLocation() + ) + ); + ;// go ahead + } + // semantic check for non around advice must return void + if (hasAtAspectJAnnotationMustReturnVoid && !Type.VOID.equals(struct.method.getReturnType())) { + msgHandler.handleMessage( + new Message( + "Found @AspectJ annotation on a non around advice not returning void '" + methodToString(struct.method) + "'", + IMessage.ERROR, + null, + type.getSourceLocation() + ) + ); + ;// go ahead + } + + return struct.ajAttributes; } @@ -227,8 +324,9 @@ public class Aj5Attributes { * * @param runtimeAnnotations * @param struct + * @return true if found */ - private static void handleAspectAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeStruct struct) { + private static boolean handleAspectAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeStruct struct) { Annotation aspect = getAnnotation(runtimeAnnotations, "org.aspectj.lang.annotation.Aspect"); if (aspect != null) { ElementNameValuePair aspectPerClause = getAnnotationElement(aspect, "value"); @@ -237,6 +335,7 @@ public class Aj5Attributes { PerClause clause = new PerSingleton(); clause.setLocation(struct.context, -1, -1); struct.ajAttributes.add(new AjAttribute.Aspect(clause)); + return true; } else { String perX = aspectPerClause.getValue().stringifyValue(); final PerClause clause; @@ -247,8 +346,10 @@ public class Aj5Attributes { } clause.setLocation(struct.context, -1, -1); struct.ajAttributes.add(new AjAttribute.Aspect(clause)); + return true; } } + return false; } /** @@ -282,7 +383,7 @@ public class Aj5Attributes { struct.handler.handleMessage( new Message( "cannot read per clause from @Aspect: " + perClause, - null,//TODO + struct.enclosingType.getSourceLocation(), true ) ); @@ -294,8 +395,9 @@ public class Aj5Attributes { * * @param runtimeAnnotations * @param struct + * @return true if found */ - private static void handlePrecedenceAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeStruct struct) { + private static boolean handlePrecedenceAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeStruct struct) { Annotation aspect = getAnnotation(runtimeAnnotations, "org.aspectj.lang.annotation.DeclarePrecedence"); if (aspect != null) { ElementNameValuePair precedence = getAnnotationElement(aspect, "value"); @@ -304,8 +406,10 @@ public class Aj5Attributes { PatternParser parser = new PatternParser(precedencePattern); DeclarePrecedence ajPrecedence = parser.parseDominates(); struct.ajAttributes.add(new AjAttribute.DeclareAttribute(ajPrecedence)); + return true; } } + return false; } /** @@ -313,8 +417,9 @@ public class Aj5Attributes { * * @param runtimeAnnotations * @param struct + * @return true if found */ - private static void handleBeforeAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { + private static boolean handleBeforeAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { Annotation before = getAnnotation(runtimeAnnotations, "org.aspectj.lang.annotation.Before"); if (before != null) { ElementNameValuePair beforeAdvice = getAnnotationElement(before, "value"); @@ -346,8 +451,10 @@ public class Aj5Attributes { struct.context ) ); + return true; } } + return false; } /** @@ -355,8 +462,9 @@ public class Aj5Attributes { * * @param runtimeAnnotations * @param struct + * @return true if found */ - private static void handleAfterAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { + private static boolean handleAfterAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { Annotation after = getAnnotation(runtimeAnnotations, "org.aspectj.lang.annotation.After"); if (after != null) { ElementNameValuePair afterAdvice = getAnnotationElement(after, "value"); @@ -388,8 +496,10 @@ public class Aj5Attributes { struct.context ) ); + return true; } } + return false; } /** @@ -397,8 +507,9 @@ public class Aj5Attributes { * * @param runtimeAnnotations * @param struct + * @return true if found */ - private static void handleAfterReturningAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { + private static boolean handleAfterReturningAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { Annotation after = getAnnotation(runtimeAnnotations, "org.aspectj.lang.annotation.AfterReturning"); if (after != null) { ElementNameValuePair annValue = getAnnotationElement(after, "value"); @@ -448,6 +559,7 @@ public class Aj5Attributes { pc = Pointcut.fromString(pointcut).resolve(binding); } setIgnoreUnboundBindingNames(pc, bindings); + pc.setLocation(struct.enclosingType.getSourceContext(), 0, 0);//TODO method location ? struct.ajAttributes.add(new AjAttribute.AdviceAttribute( AdviceKind.AfterReturning, @@ -458,7 +570,9 @@ public class Aj5Attributes { struct.context ) ); + return true; } + return false; } /** @@ -466,8 +580,9 @@ public class Aj5Attributes { * * @param runtimeAnnotations * @param struct + * @return true if found */ - private static void handleAfterThrowingAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { + private static boolean handleAfterThrowingAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { Annotation after = getAnnotation(runtimeAnnotations, "org.aspectj.lang.annotation.AfterThrowing"); if (after != null) { ElementNameValuePair annValue = getAnnotationElement(after, "value"); @@ -527,7 +642,9 @@ public class Aj5Attributes { struct.context ) ); + return true; } + return false; } /** @@ -535,8 +652,9 @@ public class Aj5Attributes { * * @param runtimeAnnotations * @param struct + * @return true if found */ - private static void handleAroundAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { + private static boolean handleAroundAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) { Annotation around = getAnnotation(runtimeAnnotations, "org.aspectj.lang.annotation.Around"); if (around != null) { ElementNameValuePair aroundAdvice = getAnnotationElement(around, "value"); @@ -568,8 +686,10 @@ public class Aj5Attributes { struct.context ) ); + return true; } } + return false; } /** @@ -583,6 +703,34 @@ public class Aj5Attributes { if (pointcut != null) { ElementNameValuePair pointcutExpr = getAnnotationElement(pointcut, "value"); if (pointcutExpr != null) { + + // semantic check: the method must return void + if (!Type.VOID.equals(struct.method.getReturnType())) { + struct.handler.handleMessage( + new Message( + "Found @Pointcut on a method not returning void '" + methodToString(struct.method) + "'", + IMessage.WARNING, + null, + struct.enclosingType.getSourceLocation()//TODO method loc instead how ? + ) + ); + //TODO AV : Andy - should we stop ? + return; + } + // semantic check: the method must not throw anything + if (struct.method.getExceptionTable() != null) { + struct.handler.handleMessage( + new Message( + "Found @Pointcut on a method throwing exception '" + methodToString(struct.method) + "'", + IMessage.WARNING, + null, + struct.enclosingType.getSourceLocation()//TODO method loc instead how ? + ) + ); + //TODO AV : Andy - should we stop ? + return; + } + // this/target/args binding IScope binding = new BindingScope( struct.enclosingType, @@ -596,21 +744,47 @@ public class Aj5Attributes { // 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 - struct.ajAttributes.add(new AjAttribute.PointcutDeclarationAttribute( - new LazyResolvedPointcutDefinition( - struct.enclosingType, - struct.method.getModifiers(), - struct.method.getName(), - argumentTypes, - Pointcut.fromString(pointcutExpr.getValue().stringifyValue()), - binding - ) - )); + try { + struct.ajAttributes.add(new AjAttribute.PointcutDeclarationAttribute( + new LazyResolvedPointcutDefinition( + struct.enclosingType, + struct.method.getModifiers(), + struct.method.getName(), + argumentTypes, + Pointcut.fromString(pointcutExpr.getValue().stringifyValue()), + binding + ) + )); + } catch (ParserException e) { + struct.handler.handleMessage( + new Message( + "Cannot parse @Pointcut '" + pointcutExpr.getValue().stringifyValue() + "'", + IMessage.ERROR, + e, + struct.enclosingType.getSourceLocation()//TODO method loc instead how ? + ) + ); + return; + } } } } /** + * Returns a readable representation of a method. + * Method.toString() is not suitable. + * + * @param method + * @return + */ + private static String methodToString(Method method) { + StringBuffer sb = new StringBuffer(); + sb.append(method.getName()); + sb.append(method.getSignature()); + return sb.toString(); + } + + /** * Build the bindings for a given method (pointcut / advice) * * @param struct @@ -723,14 +897,13 @@ public class Aj5Attributes { /** * Returns the value of a given element of an annotation or null if not found - * Does not handles default value + * Caution: Does not handles default value. * * @param annotation * @param elementName * @return */ private static ElementNameValuePair getAnnotationElement(Annotation annotation, String elementName) { - //FIXME alex does not handles default values which are annotation of elements in the annotation class for (Iterator iterator1 = annotation.getValues().iterator(); iterator1.hasNext();) { ElementNameValuePair element = (ElementNameValuePair) iterator1.next(); if (elementName.equals(element.getNameString())) { @@ -838,16 +1011,18 @@ public class Aj5Attributes { private Pointcut m_lazyPointcut = null; - public LazyResolvedPointcutDefinition(TypeX declaringType, int modifiers, String name, TypeX[] parameterTypes, + public LazyResolvedPointcutDefinition(ResolvedTypeX declaringType, int modifiers, String name, TypeX[] parameterTypes, Pointcut pointcut, IScope binding) { super(declaringType, modifiers, name, parameterTypes, null); m_pointcutUnresolved = pointcut; m_binding = binding; + m_pointcutUnresolved.setLocation(declaringType.getSourceContext(), 0, 0); } public Pointcut getPointcut() { if (m_lazyPointcut == null) { m_lazyPointcut = m_pointcutUnresolved.resolve(m_binding); + m_lazyPointcut.copyLocationFrom(m_pointcutUnresolved); } return m_lazyPointcut; } |