fix for 120356 - better feedback for DEOW when using @AJ. (from helen)

diff --git a/tests/bugs151/pr120356/A.java b/tests/bugs151/pr120356/A.java
new file mode 100644 (file)
index 0000000..6282056
--- /dev/null
+++ b/tests/bugs151/pr120356/A.java
@@ -0,0 +1,15 @@
+
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.DeclareWarning;
+import org.aspectj.lang.annotation.DeclareError;
+
+@Aspect
+public class A {
+
+       @DeclareWarning("execution(* C.warningMethod())")
+    static final String warning = "warning";
+
+    @DeclareError("execution(* C.badMethod())")
+    static final String error = "error";
+       
+}

diff --git a/tests/bugs151/pr120356/C.java b/tests/bugs151/pr120356/C.java
new file mode 100644 (file)
index 0000000..91ef6ab
--- /dev/null
+++ b/tests/bugs151/pr120356/C.java
@@ -0,0 +1,11 @@
+
+
+public class C {
+
+       public void warningMethod() {
+       }
+       
+       public void badMethod() {
+       }
+       
+}

diff --git a/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java b/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java
index c2080edb965a59be63cb0e0ba6be4bbce03fb847..7e209d0b582c00b9f2d5967dc645485085b3b231 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java
@@ -11,12 +11,14 @@
 package org.aspectj.systemtest.ajc151;
 
 import java.io.File;
+import java.util.List;
 
 import junit.framework.Test;
 
 import org.aspectj.asm.AsmManager;
 import org.aspectj.asm.IHierarchy;
 import org.aspectj.asm.IProgramElement;
+import org.aspectj.asm.internal.Relationship;
 import org.aspectj.systemtest.ajc150.GenericsTests;
 import org.aspectj.testing.XMLBasedAjcTestCase;
 
@@ -86,6 +88,67 @@ public class Ajc151Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
          runTest("warning when inherited pointcut not made concrete"); 
   }
   
+  public void testAtAspectDEOWInStructureModel_pr120356() {
+         //AsmManager.setReporting("c:/debug.txt",true,true,true,true);
+         runTest("@AJ deow appear correctly when structure model is generated");  
+         IHierarchy top = AsmManager.getDefault().getHierarchy();
+          
+         // get the IProgramElements corresponding to the @DeclareWarning statement
+         // and the method it matches.
+         IProgramElement warningMethodIPE = top.findElementForLabel(top.getRoot(),
+                         IProgramElement.Kind.METHOD,"warningMethod()");                
+         assertNotNull("Couldn't find 'warningMethod()' element in the tree",warningMethodIPE);
+         IProgramElement atDeclareWarningIPE = top.findElementForLabel(top.getRoot(),
+                         IProgramElement.Kind.FIELD,"warning");
+         assertNotNull("Couldn't find @DeclareWarning element in the tree",atDeclareWarningIPE);
+
+         // check that the method has a matches declare relationship with @DeclareWarning
+         List matches = AsmManager.getDefault().getRelationshipMap().get(warningMethodIPE);    
+         assertNotNull("warningMethod should have some relationships but does not",matches);
+         assertTrue("warningMethod should have one relationships but has " + matches.size(),matches.size()==1);
+         List matchesTargets = ((Relationship)matches.get(0)).getTargets();
+         assertTrue("warningMethod should have one targets but has" + matchesTargets.size(),matchesTargets.size()==1);
+         IProgramElement target = AsmManager.getDefault().getHierarchy().findElementForHandle((String)matchesTargets.get(0));
+         assertEquals("target of relationship should be the @DeclareWarning 'warning' but is IPE with label "
+                         + target.toLabelString(),atDeclareWarningIPE,target);
+         
+         // check that the @DeclareWarning has a matches relationship with the warningMethod
+         List matchedBy = AsmManager.getDefault().getRelationshipMap().get(atDeclareWarningIPE);
+         assertNotNull("@DeclareWarning should have some relationships but does not",matchedBy);
+         assertTrue("@DeclareWarning should have one relationship but has " + matchedBy.size(), matchedBy.size() == 1);
+         List matchedByTargets = ((Relationship)matchedBy.get(0)).getTargets();
+         assertTrue("@DeclareWarning 'matched by' relationship should have one target " +
+                       "but has " + matchedByTargets.size(), matchedByTargets.size() == 1);
+         IProgramElement matchedByTarget = AsmManager.getDefault().getHierarchy().findElementForHandle((String)matchedByTargets.get(0));
+         assertEquals("target of relationship should be the warningMethod but is IPE with label "
+                         + matchedByTarget.toLabelString(),warningMethodIPE,matchedByTarget);
+         
+         // get the IProgramElements corresponding to the @DeclareError statement
+         // and the method it matches.
+         IProgramElement errorMethodIPE = top.findElementForLabel(top.getRoot(),
+                         IProgramElement.Kind.METHOD,"badMethod()");            
+         assertNotNull("Couldn't find 'badMethod()' element in the tree",errorMethodIPE);
+         IProgramElement atDeclarErrorIPE = top.findElementForLabel(top.getRoot(),
+                         IProgramElement.Kind.FIELD,"error");
+         assertNotNull("Couldn't find @DeclareError element in the tree",atDeclarErrorIPE);
+
+         // check that the @DeclareError has a matches relationship with the badMethod
+         List matchedByE = AsmManager.getDefault().getRelationshipMap().get(atDeclarErrorIPE);
+         assertNotNull("@DeclareError should have some relationships but does not",matchedByE);
+         assertTrue("@DeclareError should have one relationship but has " + matchedByE.size(), matchedByE.size() == 1);
+         List matchedByTargetsE = ((Relationship)matchedByE.get(0)).getTargets();
+         assertTrue("@DeclareError 'matched by' relationship should have one target " +
+                       "but has " + matchedByTargetsE.size(), matchedByTargetsE.size() == 1);
+         IProgramElement matchedByTargetE = AsmManager.getDefault().getHierarchy().findElementForHandle((String)matchedByTargetsE.get(0));
+         assertEquals("target of relationship should be the badMethod but is IPE with label "
+                         + matchedByTargetE.toLabelString(),errorMethodIPE,matchedByTargetE);
+
+  }
+  
+  public void testAtAspectNoNPEWithDEOWWithoutStructureModel_pr120356() {
+         runTest("@AJ no NPE with deow when structure model isn't generated"); 
+  }
+  
   /*
    * Load-time weaving bugs and enhancements
    */

diff --git a/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml b/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml
index 5898a17a4a414ef02b40057b2dac584688721d6a..e3b67929abf0891f522c3c307be0a058679cc151 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml
+++ b/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml
@@ -142,6 +142,20 @@
         <compile files="Pr126316.aj" options="-1.5"/>
     </ajc-test>
     
+    <ajc-test dir="bugs151/pr120356" title="@AJ deow appear correctly when structure model is generated">
+        <compile files="C.java, A.java" options="-1.5, -emacssym">
+         <message kind="error" line="8" text="error"/>
+          <message kind="warning" line="5" text="warning"/>
+        </compile>
+    </ajc-test> 
+
+    <ajc-test dir="bugs151/pr120356" title="@AJ no NPE with deow when structure model isn't generated">
+        <compile files="C.java, A.java" options="-1.5">
+         <message kind="error" line="8" text="error"/>
+          <message kind="warning" line="5" text="warning"/>
+        </compile>
+    </ajc-test>
+
     <!-- New features down here... when they arent big enough to have their own test file -->
     
     <ajc-test dir="features151/ptw" title="exposing withintype">

diff --git a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
index 47805fe7b15df1c4be9e2e794e83f13f7429d4a1..b037a7bd1fd23fa8ae627adc73bfb9ee2df019c0 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
+++ b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
@@ -33,6 +33,9 @@ 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.asm.AsmManager;
+import org.aspectj.asm.IHierarchy;
+import org.aspectj.asm.IProgramElement;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.IMessageHandler;
 import org.aspectj.bridge.ISourceLocation;
@@ -1247,7 +1250,7 @@ public class AtAjAttributes {
                 } else {
                     pc .resolve(binding);
                     DeclareErrorOrWarning deow = new DeclareErrorOrWarning(true, pc, struct.field.getConstantValue().toString());
-                    deow.setLocation(struct.context, -1, -1);
+                    setDeclareErrorOrWarningLocation(deow,struct);
                     struct.ajAttributes.add(new AjAttribute.DeclareAttribute(deow));
                     hasError = true;
                 }
@@ -1274,7 +1277,7 @@ public class AtAjAttributes {
                 } else {
                     pc.resolve(binding);
                     DeclareErrorOrWarning deow = new DeclareErrorOrWarning(false, pc, struct.field.getConstantValue().toString());
-                    deow.setLocation(struct.context, -1, -1);
+                    setDeclareErrorOrWarningLocation(deow,struct);
                     struct.ajAttributes.add(new AjAttribute.DeclareAttribute(deow));
                     return hasWarning = true;
                 }
@@ -1283,6 +1286,40 @@ public class AtAjAttributes {
         return hasError || hasWarning;
     }
 
+    /**
+     * Sets the location for the declare error / warning using the corresponding 
+     * IProgramElement in the structure model. This will only fix bug 120356 if
+     * compiled with -emacssym, however, it does mean that the cross references 
+     * view in AJDT will show the correct information.
+     * 
+     * Other possibilities for fix: 
+     *  1. using the information in ajcDeclareSoft (if this is set correctly) 
+     *     which will fix the problem if compiled with ajc but not if compiled 
+     *     with javac.
+     *  2. creating an AjAttribute called FieldDeclarationLineNumberAttribute 
+     *     (much like MethodDeclarationLineNumberAttribute) which we can ask 
+     *     for the offset. This will fix bug 120356 both when compiled with ajc 
+     *     and javac.
+     * 
+     * @param deow
+     * @param struct
+     */
+    private static void setDeclareErrorOrWarningLocation(DeclareErrorOrWarning deow, AjAttributeFieldStruct struct) {
+        IHierarchy top = AsmManager.getDefault().getHierarchy();
+        if (top.getRoot() != null) {
+               IProgramElement ipe = top.findElementForLabel(top.getRoot(),
+                         IProgramElement.Kind.FIELD,struct.field.getName());
+               if (ipe != null && ipe.getSourceLocation() != null) {
+                       ISourceLocation sourceLocation = ipe.getSourceLocation();
+                       int start = sourceLocation.getOffset();
+                       int end = start + struct.field.getName().length();
+                       deow.setLocation(struct.context,start,end);
+                       return;
+               }
+               }
+        deow.setLocation(struct.context, -1, -1);                                                                                              
+    }
+    
     /**
      * Returns a readable representation of a method.
      * Method.toString() is not suitable.