diff options
20 files changed, 755 insertions, 74 deletions
diff --git a/aspectj5rt/java5-src/org/aspectj/lang/annotation/Pointcut.java b/aspectj5rt/java5-src/org/aspectj/lang/annotation/Pointcut.java index ec4b3461e..6fae1e8b9 100644 --- a/aspectj5rt/java5-src/org/aspectj/lang/annotation/Pointcut.java +++ b/aspectj5rt/java5-src/org/aspectj/lang/annotation/Pointcut.java @@ -27,8 +27,9 @@ public @interface Pointcut { /** * The pointcut expression + * We allow "" as default for abstract pointcut */ - String value(); + String value() default ""; /** * When compiling without debug info, or when interpreting pointcuts at runtime, diff --git a/docs/adk15ProgGuideDB/ltw.xml b/docs/adk15ProgGuideDB/ltw.xml index 5731e4e7b..f1c79c04d 100644 --- a/docs/adk15ProgGuideDB/ltw.xml +++ b/docs/adk15ProgGuideDB/ltw.xml @@ -1,4 +1,24 @@ <chapter id="ltw" xreflabel="Load-Time Weaving"> + + <!-- + + + + DO NOT MODIFY THIS ONE + Wes is in the process of polishing docs + Do changes in the devGuideDB/ltw.xml instead + + + + + + + + + + --> + + <title>Load-Time Weaving</title> <sect1 id="ltw-introduction"> diff --git a/docs/devGuideDB/ltw.xml b/docs/devGuideDB/ltw.xml index b0abbf37a..09ac6520c 100644 --- a/docs/devGuideDB/ltw.xml +++ b/docs/devGuideDB/ltw.xml @@ -39,8 +39,8 @@ need to specify the <literal>-Xreweavable</literal> compiler option when building them. This causes AspectJ to save additional state in the class files that is used to support subsequent reweaving. </para> - <para><!-- FIXME AV -->As per AspectJ 1.5.0 M3 aspects (code style or annotation style) are - reweavable by default, and weaved classes may be as well in 1.5.0 final.</para> + <para>As per AspectJ 1.5.0 M3 aspects (code style or annotation style) are + reweavable by default, and weaved classes are reweavable by default as well as per AspectJ 1.5.0 M4.</para> </sect2> </sect1> @@ -206,12 +206,7 @@ useful way of externalizing configuration for infrastructure and auxiliary aspects where the pointcut definitions themselves can be considered part of the configuration of the service. - </para> - - <para> - <emphasis> - Note: concrete-aspect is not available in AspectJ 1.5 M3. - </emphasis> + Refer to the next section for more details. </para> <para> @@ -272,6 +267,91 @@ each concrete aspect included in the JAR. </para> </sect2> + <sect2 id="concrete-aspect" xreflabel="concrete-aspect"> + <title>Using Concrete Aspects</title> + <para> + It is possible to concretize an abstract aspect by the mean of the <literal>META-INF/aop.xml</literal> + file. This is usefull to define abstract pointcuts at deployment time. + Consider the following: + </para> + <programlisting><![CDATA[ + package mypack; + + @Aspect + public abstract class AbstractAspect { + + // abstract pointcut: no expression is defined + @Pointcut + abstract void scope(); + + @Before("scope() && execution(* *..doSome(..))") + public void before(JoinPoint jp) { + .... + } + } + ]]></programlisting> + <para> + This aspect is equivalent to the following in code style: + </para> + <programlisting><![CDATA[ + package mypack; + + public abstract aspect AbstractAspect { + + // abstract pointcut: no expression is defined + abstract pointcut scope(); + + before() : scope() && execution(* *..doSome(..)) { + .... + } + } + ]]></programlisting> + <para> + This aspect (in either of its style) is a good candidate for concretization through <literal>META-INF/aop.xml</literal>. + It defines the abstract pointcut <literal>within()</literal>. It is important to remember that + concretization in this case must obey to the following rules: + <itemizedlist> + <listitem><para>The parent aspect must be abstract. It can be an @AspectJ or a + regular code style aspect.</para></listitem> + <listitem><para>Only simple abstract pointcut can be concretized ie pointcut that don't expose + state (through <literal>args(), this(), target(), if()</literal>). In @AspectJ syntax + as illustrated in this sample, this means the method that hosts the pointcut is abstract, + has no arguments, and returns void.</para></listitem> + <listitem><para>Concretization must defines all such abstract pointcuts ie it is not possible + to have <literal>concrete-aspect</literal> inter dependancies.</para></listitem> + <listitem><para>Concretization can only concretize pointcuts ie there cannot be abstract method + left in the aspect.</para></listitem> + </itemizedlist> + If you have requirements for more complex aspect inheritance, you should consider regular aspect + inheritance instead of concretization through XML. + Given that the following XML is valid: + </para> + <programlisting><![CDATA[ + <aspectj> + <conrete-aspect name="mypack.__My__AbstractAspect" extends="mypack.AbstractAspect"> + <pointcut name="scope" expression="within(yourpackage..*)"/> + </concrete-aspect> + </aspectj> + ]]></programlisting> + <para> + It is important to remember that the <literal>name</literal> attribute in the XML directive + <literal>concrete-aspect</literal> defines the fully qualified name that will be given to the + concrete aspect. It must then be a valid class name. This one will indeed be generated on the fly by the weaver internals. You must + then ensure that there won't be name collision. Also note that the concrete aspect will be + defined at the classloader level for which the aop.xml is visible. This implies that if you need + to use the <literal>aspectof</literal> methods to access the aspect instance(s) (depending on the perclause + of the aspect it extends) you have to use the helper API <literal>org.aspectj.lang.Aspects.aspectOf(..)</literal> + as in: + </para> + <programlisting><![CDATA[ + // exception handling omitted + Class myConcreteAspectClass = Class.forName("mypack.__My__AbstractAspect"); + + // here we are using a singleton aspect + AbstractAspect concreteInstance = Aspects.aspectOf(myConcreteAspectClass); + ]]></programlisting> + </sect2> + <!-- TODO someone implement that --> <!-- <sect2 id="configuring-load-time-weaving-with-properties-files" xreflabel="configuring-load-time-weaving-with-properties-files"> diff --git a/lib/test/aspectjrt.jar b/lib/test/aspectjrt.jar Binary files differindex 4a6eea4b9..8302072b1 100644 --- a/lib/test/aspectjrt.jar +++ b/lib/test/aspectjrt.jar diff --git a/loadtime/.classpath b/loadtime/.classpath index 5a9f37bcd..8ac0f898e 100644 --- a/loadtime/.classpath +++ b/loadtime/.classpath @@ -7,6 +7,7 @@ <classpathentry combineaccessrules="false" kind="src" path="/bridge"/> <classpathentry combineaccessrules="false" kind="src" path="/util"/> <classpathentry combineaccessrules="false" kind="src" path="/weaver"/> + <classpathentry combineaccessrules="false" kind="src" path="/bcel-builder"/> <classpathentry kind="lib" path="/lib/ext/jrockit/managementapi-jrockit81.jar"/> <classpathentry sourcepath="/lib/junit/junit-src.jar" kind="lib" path="/lib/junit/junit.jar"/> <classpathentry kind="lib" path="/lib/ant/lib/xml-apis.jar"/> diff --git a/loadtime/src/aspectj_1_5_0.dtd b/loadtime/src/aspectj_1_5_0.dtd index 710c0dac8..060962745 100644 --- a/loadtime/src/aspectj_1_5_0.dtd +++ b/loadtime/src/aspectj_1_5_0.dtd @@ -130,6 +130,7 @@ concrete-aspect <!ATTLIST concrete-aspect name CDATA #REQUIRED extends CDATA #REQUIRED + precedence CDATA #IMPLIED > <!--***************************************************************************************************************************** pointcut diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java index 547b14170..ebfdce180 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java @@ -71,7 +71,6 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { * @param bytes */ public void acceptClass(String name, byte[] bytes) { - //TODO av make dump configurable try { if (shouldDump(name.replace('/', '.'))) { Aj.dump(name, bytes); @@ -79,6 +78,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { } catch (Throwable throwable) { throwable.printStackTrace(); } + Aj.defineClass(loader, name, bytes);// could be done lazily using the hook } }; @@ -113,6 +113,23 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { messageHandler = bcelWorld.getMessageHandler(); // after adding aspects weaver.prepareForWeave(); +// // weave and flush what was registered so far +// for (Iterator iterator = m_codeGens.iterator(); iterator.hasNext();) { +// ConcreteAspectCodeGen concreteAspectCodeGen = (ConcreteAspectCodeGen) iterator.next(); +// byte[] partiallyWoven = concreteAspectCodeGen.getBytes(this); +// this.generatedClassHandler.acceptClass( +// concreteAspectCodeGen.m_concreteAspect.name, +// partiallyWoven +// ); +// ResolvedType aspect = weaver.addLibraryAspect(concreteAspectCodeGen.m_concreteAspect.name); +// //generate key for SC +// String aspectCode = readAspect(concreteAspectCodeGen.m_concreteAspect.name, loader); +// if(namespace==null){ +// namespace=new StringBuffer(aspectCode); +// }else{ +// namespace = namespace.append(";"+aspectCode); +// } +// } } } @@ -257,16 +274,21 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { //TODO: the exclude aspect allow to exclude aspect defined upper in the CL hierarchy - is it what we want ?? // if not, review the getResource so that we track which resource is defined by which CL - //it aspectClassNames + //iterate aspectClassNames //exclude if in any of the exclude list for (Iterator iterator = definitions.iterator(); iterator.hasNext();) { Definition definition = (Definition) iterator.next(); for (Iterator aspects = definition.getAspectClassNames().iterator(); aspects.hasNext();) { String aspectClassName = (String) aspects.next(); if (acceptAspect(aspectClassName)) { - weaver.addLibraryAspect(aspectClassName); - - //generate key for SC + ResolvedType aspect = weaver.addLibraryAspect(aspectClassName); + if (aspect.isAbstract()) { + // this is a warning + weaver.getWorld().getMessageHandler().handleMessage( + new Message("Abstract aspect registered in aop.xml, use a <concrete-aspect> element instead", IMessage.WARNING, null, null) + ); + } + //generate key for SC String aspectCode = readAspect(aspectClassName, loader); if(namespace==null){ namespace=new StringBuffer(aspectCode); @@ -277,9 +299,35 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { } } - //it concreteAspects - //exclude if in any of the exclude list - //TODO + //iterate concreteAspects + //exclude if in any of the exclude list - note that the user defined name matters for that to happen + for (Iterator iterator = definitions.iterator(); iterator.hasNext();) { + Definition definition = (Definition) iterator.next(); + for (Iterator aspects = definition.getConcreteAspects().iterator(); aspects.hasNext();) { + Definition.ConcreteAspect concreteAspect = (Definition.ConcreteAspect) aspects.next(); + if (acceptAspect(concreteAspect.name)) { + ConcreteAspectCodeGen gen = new ConcreteAspectCodeGen(concreteAspect, weaver.getWorld()); + if (!gen.validate()) { + weaver.getWorld().getMessageHandler().handleMessage( + new Message("Concrete-aspect '"+concreteAspect.name+"' could not be registered", IMessage.ERROR, null, null) + ); + break; + } + this.generatedClassHandler.acceptClass( + concreteAspect.name, + gen.getBytes() + ); + ResolvedType aspect = weaver.addLibraryAspect(concreteAspect.name); + //generate key for SC + String aspectCode = readAspect(concreteAspect.name, loader); + if(namespace==null){ + namespace=new StringBuffer(aspectCode); + }else{ + namespace = namespace.append(";"+aspectCode); + } + } + } + } } /** diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java new file mode 100644 index 000000000..b90ddeda0 --- /dev/null +++ b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java @@ -0,0 +1,320 @@ +/******************************************************************************* + * 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: + * Alexandre Vasseur initial implementation + *******************************************************************************/ +package org.aspectj.weaver.loadtime; + +import org.aspectj.apache.bcel.Constants; +import org.aspectj.apache.bcel.classfile.JavaClass; +import org.aspectj.apache.bcel.generic.InstructionConstants; +import org.aspectj.apache.bcel.generic.InstructionList; +import org.aspectj.apache.bcel.generic.ObjectType; +import org.aspectj.apache.bcel.generic.Type; +import org.aspectj.apache.bcel.generic.annotation.AnnotationGen; +import org.aspectj.apache.bcel.generic.annotation.ElementNameValuePairGen; +import org.aspectj.apache.bcel.generic.annotation.ElementValueGen; +import org.aspectj.apache.bcel.generic.annotation.SimpleElementValueGen; +import org.aspectj.bridge.IMessage; +import org.aspectj.bridge.Message; +import org.aspectj.weaver.AnnotationX; +import org.aspectj.weaver.ResolvedMember; +import org.aspectj.weaver.ResolvedType; +import org.aspectj.weaver.UnresolvedType; +import org.aspectj.weaver.World; +import org.aspectj.weaver.bcel.BcelPerClauseAspectAdder; +import org.aspectj.weaver.bcel.BcelWorld; +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 java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + +/** + * Generates bytecode for concrete-aspect + * <p/> + * The concrete aspect is @AspectJ code generated. As it is build during aop.xml definitions registration + * we perform the type munging for perclause ie aspectOf artifact directly, instead of waiting for it + * to go thru the weaver (that we are in the middle of configuring). + * + * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> + */ +public class ConcreteAspectCodeGen { + + private final static String[] EMPTY_STRINGS = new String[0]; + private final static Type[] EMPTY_TYPES = new Type[0]; + + /** + * Concrete aspect definition we build for + */ + private final Definition.ConcreteAspect m_concreteAspect; + + /** + * World for which we build for + */ + private final World m_world; + + /** + * Set to true when all is checks are verified + */ + private boolean m_isValid = false; + + /** + * The parent aspect, not concretized + */ + private ResolvedType m_parent; + + /** + * Aspect perClause, used for direct munging of aspectOf artifacts + */ + private PerClause m_perClause; + + /** + * Create a new compiler for a concrete aspect + * + * @param concreteAspect + * @param world + */ + ConcreteAspectCodeGen(Definition.ConcreteAspect concreteAspect, World world) { + m_concreteAspect = concreteAspect; + m_world = world; + } + + /** + * Checks that concrete aspect is valid + * + * @return true if ok, false otherwise + */ + public boolean validate() { + if (!(m_world instanceof BcelWorld)) { + reportError("Internal error: world must be of type BcelWorld"); + return false; + } + + m_parent = m_world.resolve(m_concreteAspect.extend, true); + // handle inner classes + if (m_parent.equals(ResolvedType.MISSING)) { + // fallback on inner class lookup mechanism + String fixedName = m_concreteAspect.extend; + int hasDot = fixedName.lastIndexOf('.'); + while (hasDot > 0) { + char[] fixedNameChars = fixedName.toCharArray(); + fixedNameChars[hasDot] = '$'; + fixedName = new String(fixedNameChars); + hasDot = fixedName.lastIndexOf('.'); + m_parent = m_world.resolve(UnresolvedType.forName(fixedName), true); + if (!m_parent.equals(ResolvedType.MISSING)) { + break; + } + } + } + if (m_parent.isMissing()) { + reportError("Cannot find m_parent aspect for: " + stringify()); + return false; + } + + // extends must be abstract + if (!m_parent.isAbstract()) { + reportError("Attempt to concretize a non-abstract aspect: " + stringify()); + return false; + } + + // m_parent must be aspect + if (!m_parent.isAspect()) { + reportError("Attempt to concretize a non aspect: " + stringify()); + 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(); + while (methods.hasNext()) { + ResolvedMember method = (ResolvedMember) methods.next(); + if (method.isAbstract()) { + if ("()V".equals(method.getSignature())) { + elligibleAbstractions.add(method.getName()); + } else { + reportError("Abstract member '" + method.getName() + "' cannot be concretized as a pointcut (illegal signature): " + stringify()); + return false; + } + } + } + List pointcutNames = new ArrayList(); + for (Iterator it = m_concreteAspect.pointcuts.iterator(); it.hasNext();) { + Definition.Pointcut abstractPc = (Definition.Pointcut) it.next(); + pointcutNames.add(abstractPc.name); + } + for (Iterator it = elligibleAbstractions.iterator(); it.hasNext();) { + String elligiblePc = (String) it.next(); + if (!pointcutNames.contains(elligiblePc)) { + reportError("Abstract pointcut '" + elligiblePc + "' not configured: " + stringify()); + return false; + } + } + + m_perClause = m_parent.getPerClause(); + m_isValid = true; + return m_isValid; + } + + /** + * Rebuild the XML snip that defines this concrete aspect, for log error purpose + * + * @return string repr. + */ + private String stringify() { + StringBuffer sb = new StringBuffer("<concrete-aspect name='"); + sb.append(m_concreteAspect.name); + sb.append("' extends='"); + sb.append(m_concreteAspect.extend); + sb.append("'/> in aop.xml"); + return sb.toString(); + } + + /** + * Build the bytecode for the concrete aspect + * + * @return concrete aspect bytecode + */ + public byte[] getBytes() { + if (!m_isValid) { + throw new RuntimeException("Must validate first"); + } + + //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() {} + // } + + // @Aspect public class ... + LazyClassGen cg = new LazyClassGen( + m_concreteAspect.name.replace('.', '/'), + m_parent.getName(), + null,//TODO AV - we could point to the aop.xml that defines it and use JSR-45 + Modifier.PUBLIC + Constants.ACC_SUPER, + EMPTY_STRINGS, + m_world + ); + AnnotationGen ag = new AnnotationGen( + new ObjectType("org/aspectj/lang/annotation/Aspect"), + Collections.EMPTY_LIST, + true, + cg.getConstantPoolGen() + ); + cg.addAnnotation(ag.getAnnotation()); + if (m_concreteAspect.precedence != null) { + SimpleElementValueGen svg = new SimpleElementValueGen( + ElementValueGen.STRING, + cg.getConstantPoolGen(), + m_concreteAspect.precedence + ); + List elems = new ArrayList(); + elems.add(new ElementNameValuePairGen("value", svg, cg.getConstantPoolGen())); + AnnotationGen agprec = new AnnotationGen( + new ObjectType("org/aspectj/lang/annotation/DeclarePrecedence"), + elems, + true, + cg.getConstantPoolGen() + ); + cg.addAnnotation(agprec.getAnnotation()); + } + + // default constructor + LazyMethodGen init = new LazyMethodGen( + Modifier.PUBLIC, + Type.VOID, + "<init>", + EMPTY_TYPES, + EMPTY_STRINGS, + cg + ); + InstructionList cbody = init.getBody(); + cbody.append(InstructionConstants.ALOAD_0); + cbody.append(cg.getFactory().createInvoke( + m_parent.getName().replace('.', '/'), + "<init>", + Type.VOID, + EMPTY_TYPES, + Constants.INVOKESPECIAL + )); + cbody.append(InstructionConstants.RETURN); + cg.addMethodGen(init); + + for (Iterator it = m_concreteAspect.pointcuts.iterator(); it.hasNext();) { + Definition.Pointcut abstractPc = (Definition.Pointcut) it.next(); + + LazyMethodGen mg = new LazyMethodGen( + Modifier.PUBLIC, + Type.VOID, + abstractPc.name, + EMPTY_TYPES, + EMPTY_STRINGS, + cg + ); + SimpleElementValueGen svg = new SimpleElementValueGen( + ElementValueGen.STRING, + cg.getConstantPoolGen(), + abstractPc.expression + ); + List elems = new ArrayList(); + elems.add(new ElementNameValuePairGen("value", svg, cg.getConstantPoolGen())); + AnnotationGen mag = new AnnotationGen( + new ObjectType("org/aspectj/lang/annotation/Pointcut"), + elems, + true, + cg.getConstantPoolGen() + ); + AnnotationX max = new AnnotationX(mag.getAnnotation(), m_world); + mg.addAnnotation(max); + + InstructionList body = mg.getBody(); + body.append(InstructionConstants.RETURN); + + cg.addMethodGen(mg); + } + + // handle the perClause + BcelPerClauseAspectAdder perClauseMunger = new BcelPerClauseAspectAdder( + ResolvedType.forName(m_concreteAspect.name).resolve(m_world), + m_perClause.getKind() + ); + perClauseMunger.forceMunge(cg); + + //TODO AV - unsafe cast + // register the fresh new class into the world repository as it does not exist on the classpath anywhere + JavaClass jc = cg.getJavaClass((BcelWorld) m_world); + ((BcelWorld) m_world).addSourceObjectType(jc); + + return jc.getBytes(); + } + + /** + * Error reporting + * + * @param message + */ + private void reportError(String message) { + m_world.getMessageHandler().handleMessage(new Message(message, IMessage.ERROR, null, null)); + } +} diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java index a80a4a918..8c7035b5e 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java @@ -42,7 +42,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W */ public WeavingURLClassLoader (ClassLoader parent) { this(getURLs(getClassPath()),getURLs(getAspectPath()),parent); -// System.err.println("? WeavingURLClassLoader.<init>(" + parent + ")"); +// System.err.println("? WeavingURLClassLoader.<init>(" + m_parent + ")"); } public WeavingURLClassLoader (URL[] urls, ClassLoader parent) { @@ -55,7 +55,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W // System.err.println("? WeavingURLClassLoader.<init>() classURLs=" + classURLs.length + ", aspectURLs=" + aspectURLs.length); this.aspectURLs = aspectURLs; - /* If either we nor our parent is using an ASPECT_PATH use a new-style + /* If either we nor our m_parent is using an ASPECT_PATH use a new-style * adaptor */ if (this.aspectURLs.length > 0 || parent instanceof WeavingClassLoader) { diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java index 73677e60f..87c2f82e2 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java @@ -74,20 +74,26 @@ public class Definition { } public static class ConcreteAspect { - String name; - String extend; - List pointcuts; + public final String name; + public final String extend; + public final String precedence; + public final List pointcuts; public ConcreteAspect(String name, String extend) { + this(name, extend, null); + } + + public ConcreteAspect(String name, String extend, String precedence) { this.name = name; this.extend = extend; + this.precedence = precedence; this.pointcuts = new ArrayList(); } } public static class Pointcut { - String name; - String expression; + public final String name; + public final String expression; public Pointcut(String name, String expression) { this.name = name; this.expression = expression; diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java index 9dbbfa18f..1aeccaf1d 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java @@ -56,6 +56,7 @@ public class DocumentParser extends DefaultHandler { private final static String CONCRETE_ASPECT_ELEMENT = "concrete-aspect"; private final static String NAME_ATTRIBUTE = "name"; private final static String EXTEND_ATTRIBUTE = "extends"; + private final static String PRECEDENCE_ATTRIBUTE = "precedence"; private final static String POINTCUT_ELEMENT = "pointcut"; private final static String WITHIN_ATTRIBUTE = "within"; private final static String EXPRESSION_ATTRIBUTE = "expression"; @@ -149,8 +150,13 @@ public class DocumentParser extends DefaultHandler { } else if (CONCRETE_ASPECT_ELEMENT.equals(qName)) { String name = attributes.getValue(NAME_ATTRIBUTE); String extend = attributes.getValue(EXTEND_ATTRIBUTE); + String precedence = attributes.getValue(PRECEDENCE_ATTRIBUTE); if (!isNull(name) && !isNull(extend)) { - m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend); + if (isNull(precedence)) { + m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend); + } else { + m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend, precedence); + } m_definition.getConcreteAspects().add(m_lastConcreteAspect); } } else if (POINTCUT_ELEMENT.equals(qName) && m_lastConcreteAspect != null) { diff --git a/tests/java5/ataspectj/ataspectj/ConcreteAspectTest.aj b/tests/java5/ataspectj/ataspectj/ConcreteAspectTest.aj new file mode 100644 index 000000000..d3bcd2136 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/ConcreteAspectTest.aj @@ -0,0 +1,62 @@ +/******************************************************************************* + * 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: + * Alexandre Vasseur initial implementation + *******************************************************************************/ +package ataspectj; + +import junit.framework.TestCase; +import junit.framework.Test; +import junit.framework.TestSuite; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.aspectj.lang.annotation.Pointcut; + +/** + * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> + */ +public class ConcreteAspectTest extends TestCase { + + static int I; + + void target() { + I++; + } + + // abstract aspect + // pc() is undefined hence always false, and advice not applied + // this aspect is illegal as is in aop.xml + // ones must use a concrete-aspect + static abstract aspect ConcreteAspect { + + abstract pointcut pc(); + // must be abstract + // for concrete-aspect, must further be no-arg, void + // but can be more complex for non-xml inheritance + + before() : pc() { + I++; + } + } + + public void testConcrete() { + I = 0; + target(); + assertEquals(2, I); + } + + public static void main(String[] args) { + TestHelper.runAndThrowOnFailure(suite()); + } + + public static Test suite() { + return new TestSuite(ConcreteAspectTest.class); + } + +} diff --git a/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java b/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java new file mode 100644 index 000000000..180ec4f2b --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java @@ -0,0 +1,72 @@ +/******************************************************************************* + * 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: + * Alexandre Vasseur initial implementation + *******************************************************************************/ +package ataspectj; + +import junit.framework.TestCase; +import junit.framework.Test; +import junit.framework.TestSuite; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.aspectj.lang.annotation.Pointcut; + +/** + * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> + */ +public class ConcreteAtAspectTest extends TestCase { + + static int I; + + void target() { + I++; + } + + // abstract aspect + // pc() is undefined hence always false, and advice not applied + // this aspect is illegal as is in aop.xml + // ones must use a concrete-aspect + @Aspect + abstract static class ConcreteAtAspect { + + @Pointcut() + abstract void pc(); + // must be abstract + // for concrete-aspect, must further be no-arg, void + // but can be more complex for non-xml inheritance + + @Before("pc()") + public void before() { + I++; + } + } + + // legal abstract aspect resolved with inheritance + @Aspect + static class ConcreteAtAspectSub extends ConcreteAtAspect { + @Pointcut("execution(* ataspectj.ConcreteAtAspectTest.target())") + void pc() {} + } + + public void testConcrete() { + I = 0; + target(); + assertEquals(3, I); + } + + public static void main(String[] args) { + TestHelper.runAndThrowOnFailure(suite()); + } + + public static Test suite() { + return new TestSuite(ConcreteAtAspectTest.class); + } + +} diff --git a/tests/java5/ataspectj/ataspectj/aop-concreteaspect.xml b/tests/java5/ataspectj/ataspectj/aop-concreteaspect.xml new file mode 100644 index 000000000..a9f40938d --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/aop-concreteaspect.xml @@ -0,0 +1,9 @@ +<?xml version="1.0"?> +<aspectj> + <weaver options="-XmessageHandlerClass:ataspectj.TestHelper -1.5"/> + <aspects> + <concrete-aspect name="ataspectj.Foo" extends="ataspectj.ConcreteAspectTest.ConcreteAspect"> + <pointcut name="pc" expression="execution(* ataspectj.ConcreteAspectTest.target())"/> + </concrete-aspect> + </aspects> +</aspectj> diff --git a/tests/java5/ataspectj/ataspectj/aop-concreteataspect.xml b/tests/java5/ataspectj/ataspectj/aop-concreteataspect.xml new file mode 100644 index 000000000..9b38b3d64 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/aop-concreteataspect.xml @@ -0,0 +1,10 @@ +<?xml version="1.0"?> +<aspectj> + <weaver options="-XmessageHandlerClass:ataspectj.TestHelper -1.5"/> + <aspects> + <concrete-aspect name="ataspectj.Foo" extends="ataspectj.ConcreteAtAspectTest.ConcreteAtAspect"> + <pointcut name="pc" expression="execution(* ataspectj.ConcreteAtAspectTest.target())"/> + </concrete-aspect> + <aspect name="ataspectj.ConcreteAtAspectTest.ConcreteAtAspectSub"/> + </aspects> +</aspectj> diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java index 79e3dc307..d0a88a27e 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java @@ -94,4 +94,11 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase { runTest("Compile time aspects declared to ltw weaver"); } + public void testConcreteAtAspect() { + runTest("Concrete@Aspect"); + } + + public void testConcreteAspect() { + runTest("ConcreteAspect"); + } } diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml index a316e6799..fd2b922f1 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml @@ -131,4 +131,20 @@ <ant file="ajc-ant.xml" target="ltw.oldAspectsDeclared" verbose="true"/> </ajc-test> + <ajc-test dir="java5/ataspectj" title="Concrete@Aspect"> + <compile + files="ataspectj/ConcreteAtAspectTest.java,ataspectj/TestHelper.java" + options="-1.5 -Xdev:NoAtAspectJProcessing -XnoWeave" + /> + <run class="ataspectj.ConcreteAtAspectTest" ltw="ataspectj/aop-concreteataspect.xml"/> + </ajc-test> + + <ajc-test dir="java5/ataspectj" title="ConcreteAspect"> + <compile + files="ataspectj/ConcreteAspectTest.aj,ataspectj/TestHelper.java" + options="-1.5 -Xdev:NoAtAspectJProcessing -XnoWeave" + /> + <run class="ataspectj.ConcreteAspectTest" ltw="ataspectj/aop-concreteaspect.xml"/> + </ajc-test> + </suite>
\ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java index 1160ac791..f3846c53a 100644 --- a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java +++ b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java @@ -1051,57 +1051,73 @@ public class AtAjAttributes { Annotation pointcut = getAnnotation(runtimeAnnotations, AjcMemberMaker.POINTCUT_ANNOTATION); if (pointcut != null) { ElementNameValuePair pointcutExpr = getAnnotationElement(pointcut, VALUE); - if (pointcutExpr != null) { - // semantic check: the method must return void, or be "public static boolean" for if() support - if (!(Type.VOID.equals(struct.method.getReturnType()) - || (Type.BOOLEAN.equals(struct.method.getReturnType()) && struct.method.isStatic() && struct.method.isPublic()))) { - reportWarning("Found @Pointcut on a method not returning 'void' or not 'public static boolean'", struct); - ;//no need to stop - } - // semantic check: the method must not throw anything - if (struct.method.getExceptionTable() != null) { - reportWarning("Found @Pointcut on a method throwing exception", struct); - ;// no need to stop - } - - // this/target/args binding - final IScope binding; - try { - binding = new BindingScope( - struct.enclosingType, - struct.context, - extractBindings(struct) - ); - } catch (UnreadableDebugInfoException e) { - return; - } + // semantic check: the method must return void, or be "public static boolean" for if() support + if (!(Type.VOID.equals(struct.method.getReturnType()) + || (Type.BOOLEAN.equals(struct.method.getReturnType()) && struct.method.isStatic() && struct.method.isPublic()))) { + reportWarning("Found @Pointcut on a method not returning 'void' or not 'public static boolean'", struct); + ;//no need to stop + } - UnresolvedType[] argumentTypes = new UnresolvedType[struct.method.getArgumentTypes().length]; - for (int i = 0; i < argumentTypes.length; i++) { - argumentTypes[i] = UnresolvedType.forSignature(struct.method.getArgumentTypes()[i].getSignature()); - } + // semantic check: the method must not throw anything + if (struct.method.getExceptionTable() != null) { + reportWarning("Found @Pointcut on a method throwing exception", struct); + ;// no need to stop + } - // 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, true); - if (pc == null) return;//parse error - // do not resolve binding now but lazily - pc.setLocation(struct.context, -1, -1);//FIXME AVASM !! bMethod is null here.. - struct.ajAttributes.add( - new AjAttribute.PointcutDeclarationAttribute( - new LazyResolvedPointcutDefinition( - struct.enclosingType, - struct.method.getModifiers(), - struct.method.getName(), - argumentTypes, - UnresolvedType.forSignature(struct.method.getReturnType().getSignature()), - pc, - binding - ) - ) + // this/target/args binding + final IScope binding; + try { + binding = new BindingScope( + struct.enclosingType, + struct.context, + extractBindings(struct) ); + } catch (UnreadableDebugInfoException e) { + return; + } + + UnresolvedType[] argumentTypes = new UnresolvedType[struct.method.getArgumentTypes().length]; + for (int i = 0; i < argumentTypes.length; i++) { + argumentTypes[i] = UnresolvedType.forSignature(struct.method.getArgumentTypes()[i].getSignature()); + } + + Pointcut pc = null; + if (struct.method.isAbstract()) { + if ((pointcutExpr != null && isNullOrEmpty(pointcutExpr.getValue().stringifyValue())) + || pointcutExpr == null) { + // abstract pointcut + // leave pc = null + } else { + reportError("Found defined @Pointcut on an abstract method", struct); + return;//stop + } + } else { + if (pointcutExpr != null) { + // 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 + pc = parsePointcut(pointcutExpr.getValue().stringifyValue(), struct, true); + if (pc == null) return;//parse error + pc.setLocation(struct.context, -1, -1);//FIXME AVASM !! bMethod is null here.. + } else { + reportError("Found undefined @Pointcut on a non-abstract method", struct); + return; + } } + // do not resolve binding now but lazily + struct.ajAttributes.add( + new AjAttribute.PointcutDeclarationAttribute( + new LazyResolvedPointcutDefinition( + struct.enclosingType, + struct.method.getModifiers(), + struct.method.getName(), + argumentTypes, + UnresolvedType.forSignature(struct.method.getReturnType().getSignature()), + pc,//can be null for abstract pointcut + binding + ) + ) + ); } } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java b/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java index 242e8b76d..79427eb4c 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java @@ -85,6 +85,10 @@ public class BcelPerClauseAspectAdder extends BcelTypeMunger { return false; } + return forceMunge(gen); + } + + public boolean forceMunge(LazyClassGen gen) { generatePerClauseMembers(gen); if (kind == PerClause.SINGLETON) { diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java index cf4c29927..6ab999ec3 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java @@ -141,8 +141,9 @@ public class BcelWeaver implements IWeaver { * The type is resolved to support DOT for static inner classes as well as DOLLAR * * @param aspectName + * @return aspect */ - public void addLibraryAspect(String aspectName) { + public ResolvedType addLibraryAspect(String aspectName) { // 1 - resolve as is ResolvedType type = world.resolve(UnresolvedType.forName(aspectName), true); if (type.equals(ResolvedType.MISSING)) { @@ -167,8 +168,9 @@ public class BcelWeaver implements IWeaver { //TODO AV - happens to reach that a lot of time: for each type flagged reweavable X for each aspect in the weaverstate //=> mainly for nothing for LTW - pbly for something in incremental build... xcutSet.addOrReplaceAspect(type); - } else { - // FIXME : Alex: better warning upon no such aspect from aop.xml + return type; + } else { + // FIXME AV - better warning upon no such aspect from aop.xml throw new RuntimeException("Cannot register non aspect: " + type.getName() + " , " + aspectName); } } |