398588 fix

13 files changed, 145 insertions, 35 deletions

diff --git a/tests/bugs172/pr398588/Anno.java b/tests/bugs172/pr398588/Anno.java
new file mode 100644
index 000000000..f298cf761
--- /dev/null
+++ b/tests/bugs172/pr398588/Anno.java
@@ -0,0 +1,7 @@
+package a.b.c;
+
+import java.lang.annotation.*;
+
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Anno {
+}
diff --git a/tests/bugs172/pr398588/Anno2.java b/tests/bugs172/pr398588/Anno2.java
new file mode 100644
index 000000000..16abf0957
--- /dev/null
+++ b/tests/bugs172/pr398588/Anno2.java
@@ -0,0 +1,7 @@
+package a.b.c;
+
+import java.lang.annotation.*;
+
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Anno2 {
+}
diff --git a/tests/bugs172/pr398588/AspectA.java b/tests/bugs172/pr398588/AspectA.java
new file mode 100644
index 000000000..95919fa8f
--- /dev/null
+++ b/tests/bugs172/pr398588/AspectA.java
@@ -0,0 +1,5 @@
+import a.b.c.Anno;
+
+public aspect AspectA {
+  before(): execution(@Anno * *(..)) {System.out.println("A");}
+}
diff --git a/tests/bugs172/pr398588/AspectB.java b/tests/bugs172/pr398588/AspectB.java
new file mode 100644
index 000000000..6f6ac8e62
--- /dev/null
+++ b/tests/bugs172/pr398588/AspectB.java
@@ -0,0 +1,3 @@
+public aspect AspectB {
+  before():staticinitialization(!Aspect*) { System.out.println("staticinitialization");}
+}
diff --git a/tests/bugs172/pr398588/AspectC.java b/tests/bugs172/pr398588/AspectC.java
new file mode 100644
index 000000000..143cd974e
--- /dev/null
+++ b/tests/bugs172/pr398588/AspectC.java
@@ -0,0 +1,5 @@
+import a.b.c.Blah;
+
+public aspect AspectC {
+  before(): execution(@Blah * *(..)) {System.out.println("C");}
+}
diff --git a/tests/bugs172/pr398588/AspectD.java b/tests/bugs172/pr398588/AspectD.java
new file mode 100644
index 000000000..5c14845ee
--- /dev/null
+++ b/tests/bugs172/pr398588/AspectD.java
@@ -0,0 +1,5 @@
+import a.b.c.Anno2;
+
+public aspect AspectD {
+  before(): execution(@Anno2 * *(..)) {System.out.println("D");}
+}
diff --git a/tests/bugs172/pr398588/Blah.java b/tests/bugs172/pr398588/Blah.java
new file mode 100644
index 000000000..6288a2734
--- /dev/null
+++ b/tests/bugs172/pr398588/Blah.java
@@ -0,0 +1,6 @@
+package a.b.c;
+
+import java.lang.annotation.*;
+
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Blah {}
diff --git a/tests/bugs172/pr398588/Code.java b/tests/bugs172/pr398588/Code.java
new file mode 100644
index 000000000..a11192728
--- /dev/null
+++ b/tests/bugs172/pr398588/Code.java
@@ -0,0 +1,10 @@
+import java.lang.annotation.*;
+import a.b.c.Blah;
+
+@Blah
+public class Code {
+  @Blah
+  public static void main(String[] argv) {
+    System.out.println("abcde");
+  }
+}
diff --git a/tests/bugs172/pr398588/Code2.java b/tests/bugs172/pr398588/Code2.java
new file mode 100644
index 000000000..bb48df62a
--- /dev/null
+++ b/tests/bugs172/pr398588/Code2.java
@@ -0,0 +1,11 @@
+import java.lang.annotation.*;
+import a.b.c.*;
+
+@Anno
+public class Code2 {
+  @Anno
+  public static void main(String[] argv) {
+    System.out.println("abcde");
+  }
+}
+
diff --git a/tests/bugs172/pr398588/META-INF/aop.xml b/tests/bugs172/pr398588/META-INF/aop.xml
new file mode 100644
index 000000000..a1157bb15
--- /dev/null
+++ b/tests/bugs172/pr398588/META-INF/aop.xml
@@ -0,0 +1,9 @@
+<aspectj>
+  <aspects>
+    <aspect name="AspectA" requires="a.b.c.Anno"/>
+    <aspect name="AspectB"/>
+    <aspect name="AspectC" requires="a.b.c.Blah"/>
+    <aspect name="AspectD" requires="a.b.c.Anno2"/>
+  </aspects>
+  <weaver options="-showWeaveInfo -verbose"/>
+</aspectj>
diff --git a/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java b/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java
index 88094cba6..292efcbee 100644
--- a/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java
@@ -18,13 +18,17 @@ import org.aspectj.testing.XMLBasedAjcTestCase;
 
 /**
  * @author Andy Clement
- */ 
+ */
 public class Ajc172Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
-	
+
+	public void testOptionalAspects_pr398588() {
+		runTest("optional aspects");
+	}
+
 	public void testInconsistentClassFile_pr389750() {
 		runTest("inconsistent class file");
 	}
-	
+
 	public void testInconsistentClassFile_pr389750_2() {
 		runTest("inconsistent class file 2");
 	}
@@ -44,13 +48,16 @@ public class Ajc172Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
 	public void testAnnotationValueError_pr389752_2() {
 		runTest("annotation value error 2");
 	}
-	
-	// this needs some cleverness to fix... the annotation value is parsed as a string and then not checked
-	// to see if the user is accidentally supplying, for example, an enum value. Due to the use of strings, it
-	// is hard to check.  The verification code might go here: WildAnnotationTypePattern, line 205 (the string case)
-//	public void testAnnotationValueError_pr389752_3() {
-//		runTest("annotation value error 3");
-//	}
+
+	// this needs some cleverness to fix... the annotation value is parsed as a
+	// string and then not checked
+	// to see if the user is accidentally supplying, for example, an enum value.
+	// Due to the use of strings, it
+	// is hard to check. The verification code might go here:
+	// WildAnnotationTypePattern, line 205 (the string case)
+	// public void testAnnotationValueError_pr389752_3() {
+	// runTest("annotation value error 3");
+	// }
 
 	// ---
 
diff --git a/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml b/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml
index d6a2897ad..81217b60f 100644
--- a/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml
+++ b/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml
@@ -1,7 +1,39 @@
 <!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd"[]>
 
 <suite>
-
+	<ajc-test dir="bugs172/pr398588" title="optional aspects">
+	     <compile files="Anno.java Anno2.java" outjar="anno.jar" options="-1.5"/>
+	     <compile files="AspectA.java" classpath="anno.jar" outjar="aspect.jar" options="-Xlint:ignore -1.5"/>
+	     <compile files="AspectB.java" outjar="aspect2.jar" options="-Xlint:ignore -1.5"/>
+	     <compile files="Code.java Blah.java" classpath="code.jar;aspect.jar;aspect2.jar" options="-1.5"/>
+		 <compile files="AspectC.java" classpath="$sandbox" outjar="aspect3.jar" options="-Xlint:ignore -1.5"/>	     
+	     <compile files="AspectD.java" classpath="anno.jar" outjar="aspect4.jar" options="-Xlint:ignore -1.5"/>
+	     
+	     <file deletefile="anno.jar"/>
+	     <run class="Code" ltw="META-INF/aop.xml" classpath=".;aspect.jar;aspect2.jar;aspectj3.jar;aspect4.jar">
+	     <stderr>
+	     <line text="AspectJ Weaver Version"/>
+	     <line text="register classloader"/>
+	     <line text="using configuration"/>
+	     <line text="register aspect AspectA"/>
+	     <line text="deactivating aspect"/>
+	     <line text="register aspect AspectB"/>     
+	     <line text="register aspect AspectC"/>   
+	     <line text="register aspect AspectD"/>
+	     <line text="deactivating aspect 'AspectD' as it requires type 'a.b.c.Anno2' which cannot be found on the classpath"/>
+	     <line text="Join point 'method-execution(void Code.main(java.lang.String[]))' in Type 'Code' (Code.java:7) advised by before advice from 'AspectC'"/>
+	     <line text="Join point 'staticinitialization(void Code.&lt;clinit&gt;())' in Type 'Code' (Code.java) advised by before advice from 'AspectB'"/>  
+	     <line text="processing reweavable"/>
+	     <line text="processing reweavable"/>
+	     </stderr>
+	     <stdout>
+	     <line text="staticinitialization"/>
+	     <line text="C"/>
+	     <line text="abcde"/>
+	     </stdout>
+	     </run>
+	</ajc-test>
+  
 	<ajc-test dir="bugs172/pr389750" title="inconsistent class file">
       <compile files="Code.aj" options="-1.5">
       </compile>
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
index c46c68985..7bf873bb1 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
@@ -1000,41 +1000,44 @@ public class BcelWorld extends World implements Repository {
 
 	@Override
 	public boolean hasUnsatisfiedDependency(ResolvedType aspectType) {
-		if (!aspectRequiredTypesProcessed) {
-			if (aspectRequiredTypes != null) {
-				List<String> forRemoval = new ArrayList<String>();
-				for (Map.Entry<String, String> entry : aspectRequiredTypes.entrySet()) {
-					ResolvedType rt = this.resolve(UnresolvedType.forName(entry.getValue()));
-					if (!rt.isMissing()) {
-						forRemoval.add(entry.getKey());
-					} else {
-						if (!getMessageHandler().isIgnoring(IMessage.INFO)) {
-							getMessageHandler().handleMessage(
-									MessageUtil.info("deactivating aspect '" + aspectType.getName() + "' as it requires type '"
-											+ rt.getName() + "' which cannot be found on the classpath"));
-						}
+		if (aspectRequiredTypes == null) {
+			// no aspects require anything, so there can be no unsatisfied dependencies
+			return false;
+		}
+		String aspectName = aspectType.getName();
+		if (!aspectRequiredTypesProcessed.contains(aspectName)) {
+			String requiredTypeName = aspectRequiredTypes.get(aspectName);
+			if (requiredTypeName==null) {
+				aspectRequiredTypesProcessed.add(aspectName);
+				return false;
+			} else {
+				ResolvedType rt = resolve(UnresolvedType.forName(requiredTypeName));
+				if (!rt.isMissing()) {
+					aspectRequiredTypesProcessed.add(aspectName);
+					aspectRequiredTypes.remove(aspectName);
+					return false;
+				} else {
+					if (!getMessageHandler().isIgnoring(IMessage.INFO)) {
+						getMessageHandler().handleMessage(
+								MessageUtil.info("deactivating aspect '" + aspectName + "' as it requires type '"
+										+ requiredTypeName + "' which cannot be found on the classpath"));
 					}
-				}
-				for (String key : forRemoval) {
-					aspectRequiredTypes.remove(key);
+					aspectRequiredTypesProcessed.add(aspectName);
+					return true;
 				}
 			}
-			aspectRequiredTypesProcessed = true;
-		}
-		if (aspectRequiredTypes == null) {
-			return false;
 		}
-		return aspectRequiredTypes.containsKey(aspectType.getName());
+		return aspectRequiredTypes.containsKey(aspectName);
 	}
 
-	private boolean aspectRequiredTypesProcessed = false;
+	private List<String> aspectRequiredTypesProcessed = new ArrayList<String>();
 	private Map<String, String> aspectRequiredTypes = null;
 
-	public void addAspectRequires(String name, String requiredType) {
+	public void addAspectRequires(String aspectClassName, String requiredType) {
 		if (aspectRequiredTypes == null) {
 			aspectRequiredTypes = new HashMap<String, String>();
 		}
-		aspectRequiredTypes.put(name, requiredType);
+		aspectRequiredTypes.put(aspectClassName, requiredType);
 	}
 
 	/**