testcode and fixes for pr99191: thanks to Helen.

9 files changed, 211 insertions, 4 deletions

diff --git a/tests/bugs150/pr99191/pr99191_1.java b/tests/bugs150/pr99191/pr99191_1.java
new file mode 100644
index 000000000..f716b2748
--- /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
index 000000000..aca5b53f2
--- /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
index 000000000..40bbbcad0
--- /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
index 000000000..600e804ce
--- /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
index 000000000..71a697316
--- /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
index 000000000..4f176bb66
--- /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 fc05dad53..1829956cf 100644
--- 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 b476f9e39..78e4d9318 100644
--- 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 f4cb5ab32..179ec31b6 100644
--- 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)){