testcode and fixes for pr99191: thanks to Helen.

diff --git a/tests/bugs150/pr99191/pr99191_1.java b/tests/bugs150/pr99191/pr99191_1.java
new file mode 100644 (file)
index 0000000..f716b27
--- /dev/null
+++ b/tests/bugs150/pr99191/pr99191_1.java
@@ -0,0 +1,10 @@
+@interface Annotation{}
+aspect B {
+       
+    declare @field : int C.noSuchField : @Annotation; // should be an error
+       declare @field : int B.noSuchField : @Annotation; // should be an error
+    
+}
+
+class C {
+}

diff --git a/tests/bugs150/pr99191/pr99191_2.java b/tests/bugs150/pr99191/pr99191_2.java
new file mode 100644 (file)
index 0000000..aca5b53
--- /dev/null
+++ b/tests/bugs150/pr99191/pr99191_2.java
@@ -0,0 +1,14 @@
+@interface Annotation{}
+aspect B {
+       declare @field : int C.anotherField : @Annotation; // should be woven
+       declare @field : int someField : @Annotation; // shouldn't have any errors
+       declare @field : int C.aField : @Annotation; // shouldn't have any errors
+}
+
+class C {
+       @Annotation int aField = 1;
+}
+
+aspect D {
+       public int C.anotherField;
+}

diff --git a/tests/bugs150/pr99191/pr99191_3.java b/tests/bugs150/pr99191/pr99191_3.java
new file mode 100644 (file)
index 0000000..40bbbca
--- /dev/null
+++ b/tests/bugs150/pr99191/pr99191_3.java
@@ -0,0 +1,10 @@
+@interface Annotation{}
+aspect B {
+       
+       declare @method : public * C.noSuchMethod(..) : @Annotation; // should be an error
+       declare @method : * B.noSuchMethod(..) : @Annotation; // should be an error
+    
+}
+
+class C {
+}

diff --git a/tests/bugs150/pr99191/pr99191_4.java b/tests/bugs150/pr99191/pr99191_4.java
new file mode 100644 (file)
index 0000000..600e804
--- /dev/null
+++ b/tests/bugs150/pr99191/pr99191_4.java
@@ -0,0 +1,16 @@
+@interface Annotation{}
+aspect B {
+       declare @method : public void C.anotherMethod(..) : @Annotation; // shouldn't have any errors
+       declare @method : * someMethod(..) : @Annotation; // shouldn't have any errors
+       declare @method : public void C.amethod(..) : @Annotation; // already get a warning for this, don't want an error saying method doesn't exist
+}
+
+class C {
+       @Annotation public void amethod() {             
+       }
+}
+
+aspect D {
+       public void C.anotherMethod() {
+       }
+}

diff --git a/tests/bugs150/pr99191/pr99191_5.java b/tests/bugs150/pr99191/pr99191_5.java
new file mode 100644 (file)
index 0000000..71a6973
--- /dev/null
+++ b/tests/bugs150/pr99191/pr99191_5.java
@@ -0,0 +1,10 @@
+@interface Annotation{}
+aspect B {
+       
+       declare @constructor : C.new(String) : @Annotation; // should be an error
+       declare @constructor : B.new(int) : @Annotation; // should be an error
+    
+}
+
+class C {
+}

diff --git a/tests/bugs150/pr99191/pr99191_6.java b/tests/bugs150/pr99191/pr99191_6.java
new file mode 100644 (file)
index 0000000..4f176bb
--- /dev/null
+++ b/tests/bugs150/pr99191/pr99191_6.java
@@ -0,0 +1,15 @@
+@interface Annotation{}
+aspect B {
+       declare @constructor : C.new(String) : @Annotation; // shouldn't have any errors
+       declare @constructor : *.new(int) : @Annotation; // shouldn't have any errors
+       declare @constructor : *.new(int) : @Annotation; // already get a warning for this, don't want an error saying method doesn't exist
+}
+
+class C {
+       @Annotation public C(int i) {
+       }
+}
+
+aspect D {
+       public C.new(String s){}
+}

diff --git a/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
index fc05dad53662fa5ef0a016a4cac8c45d4fff327b..1829956cf118780532fd7df06c7bbd32ad2dfc1b 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
@@ -64,6 +64,13 @@ public class Ajc150Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   public void testIncompatibleClassChangeError_pr113630_1() {runTest("IncompatibleClassChangeError - errorscenario");}
   public void testIncompatibleClassChangeError_pr113630_2() {runTest("IncompatibleClassChangeError - workingscenario");}
   
+
+  public void testDeclareAnnotationOnNonExistentType_pr99191_1() { runTest("declare annotation on non existent type - 1");}
+  public void testDeclareAnnotationOnNonExistentType_pr99191_2() { runTest("declare annotation on non existent type - 2");}  
+  public void testDeclareAnnotationOnNonExistentType_pr99191_3() { runTest("declare annotation on non existent type - 3");}  
+  public void testDeclareAnnotationOnNonExistentType_pr99191_4() { runTest("declare annotation on non existent type - 4");}  
+  public void testDeclareAnnotationOnNonExistentType_pr99191_5() { runTest("declare annotation on non existent type - 5");}  
+  
   public void testBadGenericSigAttribute_pr110927() { 
        runTest("cant create signature attribute");
        Signature sig = GenericsTests.getClassSignature(ajc,"I");

diff --git a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
index b476f9e39073a27777a3e4b03131297f976f3eb6..78e4d931872e4e7151611b0c9c31360564514e22 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
@@ -10,6 +10,48 @@
     <ajc-test dir="bugs150" title="Problem with constructor ITDs">
         <compile files="pr112783.aj" options="-1.5"/>
     </ajc-test>
+    
+    
+    <ajc-test dir="bugs150/pr99191" title="declare annotation on non existent type - 1">
+        <compile files="pr99191_1.java" options="-1.5">
+               <message kind="error" line="4" text="The field 'int C.noSuchField' does not exist"/>
+                       <message kind="error" line="5" text="The field 'int B.noSuchField' does not exist"/>
+        </compile>
+    </ajc-test> 
+
+    <!-- Currently a warning doesn't occur if the annotation is already on the field
+         (see bug 113029). If this is fixed, need to add check for this warning to this
+         test as in test "declare annotation on non existent type - 4" -->
+    <ajc-test dir="bugs150/pr99191" title="declare annotation on non existent type - 2">
+        <compile files="pr99191_2.java" options="-1.5">
+        </compile>
+    </ajc-test> 
+
+    <ajc-test dir="bugs150/pr99191" title="declare annotation on non existent type - 3">
+        <compile files="pr99191_3.java" options="-1.5">
+               <message kind="error" line="4" text="The method 'public * C.noSuchMethod(..)' does not exist"/>
+                       <message kind="error" line="5" text="The method '* B.noSuchMethod(..)' does not exist"/>
+        </compile>
+    </ajc-test> 
+
+    <ajc-test dir="bugs150/pr99191" title="declare annotation on non existent type - 4">
+        <compile files="pr99191_4.java" options="-1.5">
+               <message kind="warning" text="void C.amethod() - already has an annotation of type Annotation, cannot add a second instance [Xlint:elementAlreadyAnnotated]"/>
+        </compile>
+    </ajc-test> 
+
+    <ajc-test dir="bugs150/pr99191" title="declare annotation on non existent type - 5">
+        <compile files="pr99191_5.java" options="-1.5">
+               <message kind="error" line="4" text="The method 'C.new(java.lang.String)' does not exist"/>
+                       <message kind="error" line="5" text="The method 'B.new(int)' does not exist"/>
+        </compile>
+    </ajc-test> 
+
+    <ajc-test dir="bugs150/pr99191" title="declare annotation on non existent type - 6">
+        <compile files="pr99191_6.java" options="-1.5">
+               <message kind="warning" text="void C.&lt;init&gt;(int) - already has an annotation of type Annotation, cannot add a second instance [Xlint:elementAlreadyAnnotated]"/>
+        </compile>
+    </ajc-test>
  
     <ajc-test dir="bugs150/pr113630/case1" title="IncompatibleClassChangeError - errorscenario">
         <compile files="Bean.java,BeanTestCase.java,javaBean.java,propertyChanger.java,PropertySupportAspect5.aj" options="-1.5">

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
index f4cb5ab3230d219a163e1f2d2923d6771ad5d030..179ec31b646144370e06ba7757bea754348b317e 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
@@ -56,6 +56,7 @@ import org.aspectj.apache.bcel.generic.Type;
 import org.aspectj.apache.bcel.generic.annotation.AnnotationGen;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
+import org.aspectj.bridge.Message;
 import org.aspectj.bridge.WeaveMessage;
 import org.aspectj.bridge.context.CompilationAndWeavingContext;
 import org.aspectj.bridge.context.ContextToken;
@@ -82,6 +83,7 @@ import org.aspectj.weaver.WeaverMessages;
 import org.aspectj.weaver.WeaverMetrics;
 import org.aspectj.weaver.WeaverStateInfo;
 import org.aspectj.weaver.patterns.DeclareAnnotation;
+import org.aspectj.weaver.patterns.ExactTypePattern;
 
 class BcelClassWeaver implements IClassWeaver {
     
@@ -467,6 +469,8 @@ class BcelClassWeaver implements IClassWeaver {
                List decaMs = getMatchingSubset(allDecams,clazz.getType());             
                if (decaMs.isEmpty()) return false; // nothing to do
                if (!members.isEmpty()) {
+                 Set unusedDecams = new HashSet();
+                 unusedDecams.addAll(decaMs);
           for (int memberCounter = 0;memberCounter<members.size();memberCounter++) {
             LazyMethodGen mg = (LazyMethodGen)members.get(memberCounter);
             if (!mg.getName().startsWith(NameMangler.PREFIX)) {
@@ -479,7 +483,12 @@ class BcelClassWeaver implements IClassWeaver {
                        DeclareAnnotation decaM = (DeclareAnnotation) iter.next();
                        
                        if (decaM.matches(mg.getMemberView(),world)) {
-                               if (doesAlreadyHaveAnnotation(mg.getMemberView(),decaM,reportedProblems)) continue; // skip this one...
+                               if (doesAlreadyHaveAnnotation(mg.getMemberView(),decaM,reportedProblems)) {
+                                       // remove the declare @method since don't want an error when 
+                                       // the annotation is already there
+                                       unusedDecams.remove(decaM);
+                                       continue; // skip this one...
+                               }
                                
                                Annotation a = decaM.getAnnotationX().getBcelAnnotation();
                                AnnotationGen ag = new AnnotationGen(a,clazz.getConstantPoolGen(),true);
@@ -494,6 +503,8 @@ class BcelClassWeaver implements IClassWeaver {
                                reportMethodCtorWeavingMessage(clazz, mg.getMemberView(), decaM,mg.getDeclarationLineNumber());
                                isChanged = true;
                                modificationOccured = true;
+                                               // remove the declare @method since have matched against it
+                               unusedDecams.remove(decaM);                             
                        } else {
                                if (!decaM.isStarredAnnotationPattern()) 
                                        worthRetrying.add(decaM); // an annotation is specified that might be put on by a subsequent decaf
@@ -508,18 +519,26 @@ class BcelClassWeaver implements IClassWeaver {
                        for (Iterator iter = worthRetrying.iterator(); iter.hasNext();) {
                                DeclareAnnotation decaM = (DeclareAnnotation) iter.next();
                                if (decaM.matches(mg.getMemberView(),world)) {
-                                       if (doesAlreadyHaveAnnotation(mg.getMemberView(),decaM,reportedProblems)) continue; // skip this one...
+                                       if (doesAlreadyHaveAnnotation(mg.getMemberView(),decaM,reportedProblems)) {
+                                               // remove the declare @method since don't want an error when 
+                                               // the annotation is already there
+                                               unusedDecams.remove(decaM);
+                                               continue; // skip this one...
+                                       }
                                        mg.addAnnotation(decaM.getAnnotationX());
                                        AsmRelationshipProvider.getDefault().addDeclareAnnotationRelationship(decaM.getSourceLocation(),clazz.getName(),mg.getMethod());
                                        isChanged = true;
                                        modificationOccured = true;
                                        forRemoval.add(decaM);
+                                               // remove the declare @method since have matched against it
+                                       unusedDecams.remove(decaM);
                                }
                        }
                        worthRetrying.removeAll(forRemoval);
                }
             }
           }
+                 checkUnusedDeclareAtTypes(unusedDecams, false);
         }
                return isChanged;
     }
@@ -791,7 +810,8 @@ class BcelClassWeaver implements IClassWeaver {
                if (decaFs.isEmpty()) return false; // nothing more to do
                Field[] fields = clazz.getFieldGens();
                if (fields!=null) {
-                 
+                 Set unusedDecafs = new HashSet();
+                 unusedDecafs.addAll(decaFs);
           for (int fieldCounter = 0;fieldCounter<fields.length;fieldCounter++) {
             BcelField aBcelField = new BcelField(clazz.getBcelObjectType(),fields[fieldCounter]);
                        if (!aBcelField.getName().startsWith(NameMangler.PREFIX)) {                             
@@ -808,6 +828,9 @@ class BcelClassWeaver implements IClassWeaver {
                                        
                                        if (!dontAddTwice(decaF,dontAddMeTwice)){
                                                if (doesAlreadyHaveAnnotation(aBcelField,decaF,reportedProblems)){
+                                       // remove the declare @field since don't want an error when 
+                                       // the annotation is already there
+                                       unusedDecafs.remove(decaF);
                                                        continue;
                                                }
                                                
@@ -833,6 +856,8 @@ class BcelClassWeaver implements IClassWeaver {
                                        reportFieldAnnotationWeavingMessage(clazz, fields, fieldCounter, decaF);                
                                        isChanged = true;
                                        modificationOccured = true;
+                                       // remove the declare @field since have matched against it
+                               unusedDecafs.remove(decaF); 
                                } else {
                                        if (!decaF.isStarredAnnotationPattern()) 
                                                worthRetrying.add(decaF); // an annotation is specified that might be put on by a subsequent decaf
@@ -848,22 +873,80 @@ class BcelClassWeaver implements IClassWeaver {
                                DeclareAnnotation decaF = (DeclareAnnotation) iter.next();
                                if (decaF.matches(aBcelField,world)) {
                                        // below code is for recursive things
-                                       if (doesAlreadyHaveAnnotation(aBcelField,decaF,reportedProblems)) continue; // skip this one...
+                                       if (doesAlreadyHaveAnnotation(aBcelField,decaF,reportedProblems)) {
+                                       // remove the declare @field since don't want an error when 
+                                       // the annotation is already there
+                                       unusedDecafs.remove(decaF);
+                                               continue; // skip this one...
+                                       }
                                        aBcelField.addAnnotation(decaF.getAnnotationX());
                                        AsmRelationshipProvider.getDefault().addDeclareAnnotationRelationship(decaF.getSourceLocation(),clazz.getName(),fields[fieldCounter]);
                                        isChanged = true;
                                        modificationOccured = true;
                                        forRemoval.add(decaF);
+                                       // remove the declare @field since have matched against it
+                               unusedDecafs.remove(decaF); 
                                }
                          }
                          worthRetrying.removeAll(forRemoval);
             }
                        }
           }
+                 checkUnusedDeclareAtTypes(unusedDecafs,true);
         }
                return isChanged;
        }
 
+       // bug 99191 - put out an error message if the type doesn't exist
+       /**
+        * Report an error if the reason a "declare @method/ctor/field" was not used was because the member
+        * specified does not exist.  This method is passed some set of declare statements that didn't
+        * match and a flag indicating whether the set contains declare @field or declare @method/ctor
+        * entries.
+        */
+       private void checkUnusedDeclareAtTypes(Set unusedDecaTs, boolean isDeclareAtField) {
+               for (Iterator iter = unusedDecaTs.iterator(); iter.hasNext();) {
+                       DeclareAnnotation declA = (DeclareAnnotation) iter.next();
+                       
+                       // Error if an exact type pattern was specified 
+                       if ((declA.isExactPattern() || 
+                                       (declA.getSignaturePattern().getDeclaringType() instanceof ExactTypePattern))
+                                       && (!declA.getSignaturePattern().getName().isAny()
+                                                       || (declA.getKind() == DeclareAnnotation.AT_CONSTRUCTOR))) {
+                               
+                               // Quickly check if an ITD meets supplies the 'missing' member
+                               boolean itdMatch = false;
+                               List lst = clazz.getType().getInterTypeMungers();
+                               for (Iterator iterator = lst.iterator(); iterator.hasNext() && !itdMatch;) {
+                                       BcelTypeMunger element = (BcelTypeMunger) iterator.next();
+                                       if (element.getMunger() instanceof NewFieldTypeMunger) { 
+                                               NewFieldTypeMunger nftm = (NewFieldTypeMunger)element.getMunger();
+                                               itdMatch = declA.getSignaturePattern().matches(nftm.getSignature(),world,false);
+                                       }else if (element.getMunger() instanceof NewMethodTypeMunger) {
+                                               NewMethodTypeMunger nmtm = (NewMethodTypeMunger)element.getMunger();
+                                               itdMatch = declA.getSignaturePattern().matches(nmtm.getSignature(),world,false);                                                        
+                                       } else if (element.getMunger() instanceof NewConstructorTypeMunger) {
+                                               NewConstructorTypeMunger nctm = (NewConstructorTypeMunger)element.getMunger();
+                                               itdMatch = declA.getSignaturePattern().matches(nctm.getSignature(),world,false);                                                        
+                                       }
+                               }
+                               if (!itdMatch) {
+                                       IMessage message = null;
+                                       if (isDeclareAtField) {
+                                               message = new Message(
+                                                               "The field '"+ declA.getSignaturePattern().toString() + 
+                                                               "' does not exist", declA.getSourceLocation() , true);
+                                       } else { 
+                                               message = new Message(
+                                                               "The method '"+ declA.getSignaturePattern().toString() + 
+                                                               "' does not exist", declA.getSourceLocation() , true);
+                                       }
+                                       world.getMessageHandler().handleMessage(message);                                       
+                               }
+                       }
+               }
+       }
+       
        // TAG: WeavingMessage
        private void reportFieldAnnotationWeavingMessage(LazyClassGen clazz, Field[] fields, int fieldCounter, DeclareAnnotation decaF) {
                if (!getWorld().getMessageHandler().isIgnoring(IMessage.WEAVEINFO)){