fix for pr107953, @AfterThrowing with no formal

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
index 68502b43c4bc5c967e7dde448f64caa972a25edf..2d178579730cc52b5da4c2eaa18426b08b2dbbe9 100644 (file)
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java
@@ -260,6 +260,8 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor {
         * 1) Advice must be public
         * 2) Advice must have a void return type if not around advice
         * 3) Advice must not have any other @AspectJ annotations
+        * 4) After throwing advice must declare the thrown formal
+        * 5) After returning advice must declare the returning formal
         */
        private void validateAdvice(MethodDeclaration methodDeclaration) {
                
@@ -290,6 +292,42 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor {
                if (ajAnnotations.adviceKind != AdviceKind.Around) {
                        ensureVoidReturnType(methodDeclaration);
                }         
+               
+               if (ajAnnotations.adviceKind == AdviceKind.AfterThrowing) {
+                       int[] throwingLocation = new int[2];
+                       String thrownFormal = getStringLiteralFor("throwing",ajAnnotations.adviceAnnotation,throwingLocation);
+                       if (thrownFormal != null) {
+                               Argument[] arguments = methodDeclaration.arguments;
+                               if (arguments != null && arguments.length > 0) {
+                                       Argument lastArgument = arguments[arguments.length - 1];
+                                       if (!thrownFormal.equals(new String(lastArgument.name))) {
+                                               methodDeclaration.scope.problemReporter()
+                                                       .signalError(methodDeclaration.sourceStart,methodDeclaration.sourceEnd,"throwing formal '" + thrownFormal + "' must be declared as the last parameter in the advice signature");
+                                       }
+                               } else {
+                                       methodDeclaration.scope.problemReporter()
+                                       .signalError(methodDeclaration.sourceStart,methodDeclaration.sourceEnd,"throwing formal '" + thrownFormal + "' must be declared as the last parameter in the advice signature");
+                               }
+                       }
+               }
+               
+               if (ajAnnotations.adviceKind == AdviceKind.AfterReturning) {
+                       int[] throwingLocation = new int[2];
+                       String returningFormal = getStringLiteralFor("returning",ajAnnotations.adviceAnnotation,throwingLocation);
+                       if (returningFormal != null) {
+                               Argument[] arguments = methodDeclaration.arguments;
+                               if (arguments != null && arguments.length > 0) {
+                                       Argument lastArgument = arguments[arguments.length - 1];
+                                       if (!returningFormal.equals(new String(lastArgument.name))) {
+                                               methodDeclaration.scope.problemReporter()
+                                                       .signalError(methodDeclaration.sourceStart,methodDeclaration.sourceEnd,"returning formal '" + returningFormal + "' must be declared as the last parameter in the advice signature");
+                                       }
+                               } else {
+                                       methodDeclaration.scope.problemReporter()
+                                       .signalError(methodDeclaration.sourceStart,methodDeclaration.sourceEnd,"returning formal '" + returningFormal + "' must be declared as the last parameter in the advice signature");
+                               }
+                       }
+               }
 
                resolveAndSetPointcut(methodDeclaration, ajAnnotations.adviceAnnotation);
                

diff --git a/weaver/src/org/aspectj/weaver/WeaverMessages.java b/weaver/src/org/aspectj/weaver/WeaverMessages.java
index b49155f4976bffdf7058529222c5f58889bece7c..40ef2e9a3c4da6716538fd1d97854ca405d838a6 100644 (file)
--- a/weaver/src/org/aspectj/weaver/WeaverMessages.java
+++ b/weaver/src/org/aspectj/weaver/WeaverMessages.java
@@ -148,6 +148,10 @@ public class WeaverMessages {
        
        public static final String HAS_MEMBER_NOT_ENABLED="hasMemberNotEnabled";
        
+       // @AspectJ
+       public static final String RETURNING_FORMAL_NOT_DECLARED_IN_ADVICE = "returningFormalNotDeclaredInAdvice";
+       public static final String THROWN_FORMAL_NOT_DECLARED_IN_ADVICE = "thrownFormalNotDeclaredInAdvice";
+       
        public static String format(String key) {
                return bundle.getString(key);
        }

diff --git a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
index ef8c26497f6fd2172ff17b55827d04cc51ebdc7b..2896d9dc650068697efbd927ebcb64e34e563947 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
+++ b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
@@ -46,6 +46,7 @@ import org.aspectj.weaver.ReferenceType;
 import org.aspectj.weaver.ResolvedPointcutDefinition;
 import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.UnresolvedType;
+import org.aspectj.weaver.WeaverMessages;
 import org.aspectj.weaver.patterns.AndPointcut;
 import org.aspectj.weaver.patterns.DeclareErrorOrWarning;
 import org.aspectj.weaver.patterns.DeclarePrecedence;
@@ -341,26 +342,43 @@ public class AtAjAttributes {
         boolean hasAtAspectJAnnotationMustReturnVoid = false;
         for (int i = 0; i < attributes.length; i++) {
             Attribute attribute = attributes[i];
-            if (acceptAttribute(attribute)) {
-                RuntimeAnnotations rvs = (RuntimeAnnotations) attribute;
-                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;
-            }
+            try {
+                               if (acceptAttribute(attribute)) {
+                                   RuntimeAnnotations rvs = (RuntimeAnnotations) attribute;
+                                   hasAtAspectJAnnotationMustReturnVoid = hasAtAspectJAnnotationMustReturnVoid || handleBeforeAnnotation(
+                                           rvs, struct, preResolvedPointcut
+                                   );
+                                   hasAtAspectJAnnotationMustReturnVoid = hasAtAspectJAnnotationMustReturnVoid || handleAfterAnnotation(
+                                           rvs, struct, preResolvedPointcut
+                                   );
+                                   hasAtAspectJAnnotationMustReturnVoid = hasAtAspectJAnnotationMustReturnVoid || handleAfterReturningAnnotation(
+                                           rvs, struct, preResolvedPointcut, bMethod
+                                   );
+                                   hasAtAspectJAnnotationMustReturnVoid = hasAtAspectJAnnotationMustReturnVoid || handleAfterThrowingAnnotation(
+                                           rvs, struct, preResolvedPointcut, bMethod
+                                   );
+                                   hasAtAspectJAnnotation = hasAtAspectJAnnotation || handleAroundAnnotation(
+                                           rvs, struct, preResolvedPointcut
+                                   );
+                                   // there can only be one RuntimeVisible bytecode attribute
+                                   break;
+                               }
+                       } catch (ReturningFormalNotDeclaredInAdviceSignatureException e) {
+                               msgHandler.handleMessage(
+                                       new Message(
+                                                       WeaverMessages.format(WeaverMessages.RETURNING_FORMAL_NOT_DECLARED_IN_ADVICE,e.getFormalName()),
+                                                       IMessage.ERROR,
+                                                       null,
+                                                       bMethod.getSourceLocation())
+                               );
+                       } catch (ThrownFormalNotDeclaredInAdviceSignatureException e) {
+                               msgHandler.handleMessage(
+                                               new Message(
+                                                               WeaverMessages.format(WeaverMessages.THROWN_FORMAL_NOT_DECLARED_IN_ADVICE,e.getFormalName()),
+                                                               IMessage.ERROR,
+                                                               null,
+                                                               bMethod.getSourceLocation())
+                                       );                      }
         }
         hasAtAspectJAnnotation = hasAtAspectJAnnotation || hasAtAspectJAnnotationMustReturnVoid;
 
@@ -402,8 +420,7 @@ public class AtAjAttributes {
             );
             ;// go ahead
         }
-
-
+        
         return struct.ajAttributes;
     }
 
@@ -664,7 +681,13 @@ public class AtAjAttributes {
      * @param struct
      * @return true if found
      */
-    private static boolean handleAfterReturningAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) {
+    private static boolean handleAfterReturningAnnotation(
+               RuntimeAnnotations runtimeAnnotations, 
+               AjAttributeMethodStruct struct, 
+               ResolvedPointcutDefinition preResolvedPointcut,
+               BcelMethod owningMethod) 
+    throws ReturningFormalNotDeclaredInAdviceSignatureException
+    {
         Annotation after = getAnnotation(runtimeAnnotations, AjcMemberMaker.AFTERRETURNING_ANNOTATION);
         if (after != null) {
             ElementNameValuePair annValue = getAnnotationElement(after, VALUE);
@@ -689,8 +712,16 @@ public class AtAjAttributes {
             }
             if (annReturned != null) {
                 returned = annReturned.getValue().stringifyValue();
-                if (isNullOrEmpty(returned))
+                if (isNullOrEmpty(returned)) {
                     returned = null;
+                } else {
+                       // check that thrownFormal exists as the last parameter in the advice
+                       String[] pNames = owningMethod.getParameterNames();
+                       if (pNames == null || pNames.length == 0 || !pNames[pNames.length -1].equals(returned)) {
+                               throw new ReturningFormalNotDeclaredInAdviceSignatureException(returned);
+                       }
+                       
+                }
             }
 
             // this/target/args binding
@@ -748,16 +779,22 @@ public class AtAjAttributes {
      * @param struct
      * @return true if found
      */
-    private static boolean handleAfterThrowingAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeMethodStruct struct, ResolvedPointcutDefinition preResolvedPointcut) {
+    private static boolean handleAfterThrowingAnnotation(
+               RuntimeAnnotations runtimeAnnotations, 
+               AjAttributeMethodStruct struct, 
+               ResolvedPointcutDefinition preResolvedPointcut, 
+               BcelMethod owningMethod)
+    throws ThrownFormalNotDeclaredInAdviceSignatureException
+    {
         Annotation after = getAnnotation(runtimeAnnotations, AjcMemberMaker.AFTERTHROWING_ANNOTATION);
         if (after != null) {
             ElementNameValuePair annValue = getAnnotationElement(after, VALUE);
             ElementNameValuePair annPointcut = getAnnotationElement(after, POINTCUT);
-            ElementNameValuePair annThrowned = getAnnotationElement(after, THROWING);
+            ElementNameValuePair annThrown = getAnnotationElement(after, THROWING);
 
             // extract the pointcut and throwned type/binding - do some checks
             String pointcut = null;
-            String throwned = null;
+            String thrownFormal = null;
             if ((annValue != null && annPointcut != null) || (annValue == null && annPointcut == null)) {
                 reportError("@AfterThrowing: either 'value' or 'poincut' must be provided, not both", struct);
                 return false;
@@ -771,17 +808,24 @@ public class AtAjAttributes {
                 reportError("@AfterThrowing: either 'value' or 'poincut' must be provided, not both", struct);
                 return false;
             }
-            if (annThrowned != null) {
-                throwned = annThrowned.getValue().stringifyValue();
-                if (isNullOrEmpty(throwned))
-                    throwned = null;
+            if (annThrown != null) {
+                thrownFormal = annThrown.getValue().stringifyValue();
+                if (isNullOrEmpty(thrownFormal)) {
+                    thrownFormal = null;
+                } else {
+                       // check that thrownFormal exists as the last parameter in the advice
+                       String[] pNames = owningMethod.getParameterNames();
+                       if (pNames == null || pNames.length == 0 || !pNames[pNames.length -1].equals(thrownFormal)) {
+                               throw new ThrownFormalNotDeclaredInAdviceSignatureException(thrownFormal);
+                       }
+                }
             }
 
             // this/target/args binding
             // exclude the throwned binding from the pointcut binding since it is an extraArg binding
             FormalBinding[] bindings = new org.aspectj.weaver.patterns.FormalBinding[0];
             try {
-                bindings = (throwned == null ? extractBindings(struct) : extractBindings(struct, throwned));
+                bindings = (thrownFormal == null ? extractBindings(struct) : extractBindings(struct, thrownFormal));
             } catch (UnreadableDebugInfoException unreadableDebugInfoException) {
                 return false;
             }
@@ -795,7 +839,7 @@ public class AtAjAttributes {
             int extraArgument = extractExtraArgument(struct.method);
 
             // return binding
-            if (throwned != null) {
+            if (thrownFormal != null) {
                 extraArgument |= Advice.ExtraArgument;
             }
 
@@ -1457,4 +1501,26 @@ public class AtAjAttributes {
             return node;
         }
     }
+    
+    static class ThrownFormalNotDeclaredInAdviceSignatureException extends Exception {
+       
+       private String formalName;
+       
+       public ThrownFormalNotDeclaredInAdviceSignatureException(String formalName) {
+               this.formalName = formalName;
+       }
+       
+       public String getFormalName() { return formalName; }
+    }
+    
+    static class ReturningFormalNotDeclaredInAdviceSignatureException extends Exception {
+       
+       private String formalName;
+       
+       public ReturningFormalNotDeclaredInAdviceSignatureException(String formalName) {
+               this.formalName = formalName;
+       }
+       
+       public String getFormalName() { return formalName; }
+    }
 }

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelMethod.java b/weaver/src/org/aspectj/weaver/bcel/BcelMethod.java
index 89f6d67eae307fd8b21a8aeaeddb83abb52a95fb..7bdf3af71f37d283f499871755e6a8257d2a0892 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/BcelMethod.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelMethod.java
@@ -68,8 +68,8 @@ final class BcelMethod extends ResolvedMemberImpl {
                this.sourceContext = declaringType.getResolvedTypeX().getSourceContext();
                this.world = declaringType.getResolvedTypeX().getWorld();
                this.bcelObjectType = declaringType;
-               unpackAjAttributes(world);
                unpackJavaAttributes();
+               unpackAjAttributes(world);
        }
 
        // ----

diff --git a/weaver/src/org/aspectj/weaver/weaver-messages.properties b/weaver/src/org/aspectj/weaver/weaver-messages.properties
index 56e94c5aee3977aab80441d823ab424f67bf8523..d896687d0917707d7e32d596b2547aa1d035cbae 100644 (file)
--- a/weaver/src/org/aspectj/weaver/weaver-messages.properties
+++ b/weaver/src/org/aspectj/weaver/weaver-messages.properties
@@ -157,4 +157,8 @@ noParameterizedDeclaringTypesInExecution=can't use parameterized type patterns f
 noParameterizedDeclaringTypesInCall=can't use parameterized type patterns for the declaring type of a call pointcut expression (use the raw type instead)
 noRawTypePointcutReferences=cannot use a raw type reference to refer to a pointcut in a generic type (use a parameterized reference instead)
 
-hasMemberNotEnabled=the type pattern {0} can only be used when the -XhasMember option is set
\ No newline at end of file
+hasMemberNotEnabled=the type pattern {0} can only be used when the -XhasMember option is set
+
+# @AspectJ
+returningFormalNotDeclaredInAdvice=the last parameter of this advice must be named ''{0}'' to bind the returning value
+thrownFormalNotDeclaredInAdvice=the last parameter of this advice must be named ''{0}'' and be of a subtype of Throwable
\ No newline at end of file