Fix for pr91267 (Generics): NPE at EclipseFactory.java:143 when using generic methods in aspects

6 files changed, 76 insertions, 5 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java
index ef960113e..395094b25 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java
@@ -136,8 +136,17 @@ public class EclipseFactory {
 		return ret;
 	}	
 	
-	
 	private static String getName(TypeBinding binding) {
+		if (binding instanceof TypeVariableBinding) {
+			// The first bound may be null - so default to object?
+			TypeVariableBinding tvb = (TypeVariableBinding)binding;
+			if (tvb.firstBound!=null) {
+				return getName(tvb.firstBound);
+			} else {
+				return getName(tvb.superclass);
+			}
+		}
+		
 		if (binding instanceof ReferenceBinding) {
 			return new String(
 				CharOperation.concatWith(((ReferenceBinding)binding).compoundName, '.'));
@@ -154,6 +163,16 @@ public class EclipseFactory {
 
 
 
+	/**
+	 * Some generics notes:
+	 * 
+	 * Andy 6-May-05
+	 * We were having trouble with parameterized types in a couple of places - due to TypeVariableBindings.  When we
+	 * see a TypeVariableBinding now we default to either the firstBound if it is specified or java.lang.Object.  Not
+	 * sure when/if this gets us unstuck?  It does mean we forget that it is a type variable when going back
+	 * the other way from the TypeX and that would seem a bad thing - but I've yet to see the reason we need to
+	 * remember the type variable.
+	 */
 	//??? going back and forth between strings and bindings is a waste of cycles
 	public static TypeX fromBinding(TypeBinding binding) {
 		if (binding instanceof HelperInterfaceBinding) {
@@ -166,7 +185,12 @@ public class EclipseFactory {
 		if (binding instanceof TypeVariableBinding) {
 			// this is a type variable...
 			TypeVariableBinding tvb = (TypeVariableBinding) binding;
-			return TypeX.forName(getName(tvb.firstBound)); // XXX needs more investigation as to whether this is correct in all cases
+			// This code causes us to forget its a TVB which we will need when going back the other way...
+			if (tvb.firstBound!=null) {
+			  return TypeX.forName(getName(tvb.firstBound)); // XXX needs more investigation as to whether this is correct in all cases
+			} else {
+			  return TypeX.forName(getName(tvb.superclass));
+			}
 		}
 		// FIXME asc/amc cope properly with RawTypeBindings
 		if (binding instanceof ParameterizedTypeBinding && !(binding instanceof RawTypeBinding)) {
diff --git a/tests/java5/generics/bugs/pr91267/TestBug1.aj b/tests/java5/generics/bugs/pr91267/TestBug1.aj
new file mode 100644
index 000000000..b861d2155
--- /dev/null
+++ b/tests/java5/generics/bugs/pr91267/TestBug1.aj
@@ -0,0 +1,15 @@
+import java.util.*;
+
+public aspect TestBug1 {
+    static <T> void addToEnv(Map<String,T> env, String key, T value) {
+	env.put(key, value);
+    }
+
+    public static void main(String[] argv) {
+      Map<String,Integer> msi = new HashMap<String,Integer>();
+      addToEnv(msi,"andy",new Integer(42));
+
+      if (msi.get("andy")!=42) throw new RuntimeException("Failed to add");
+    }
+}
+
diff --git a/tests/java5/generics/bugs/pr91267/TestBug2.aj b/tests/java5/generics/bugs/pr91267/TestBug2.aj
new file mode 100644
index 000000000..840f769ee
--- /dev/null
+++ b/tests/java5/generics/bugs/pr91267/TestBug2.aj
@@ -0,0 +1,13 @@
+import java.util.*;
+
+public aspect TestBug2 {
+    static <T> T lookupEnv(Map<String,T> env, String key) {
+	return env.get(key);
+    }
+
+    public static void main(String[] argv) {
+      Map<String,Integer> msi = new HashMap<String,Integer>();
+      msi.put("andy",42);
+      if (lookupEnv(msi,"andy")!=42) throw new RuntimeException("Failed to lookup");
+    }
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java b/tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java
index 332f0dd8e..96557dfe5 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java
@@ -19,4 +19,12 @@ public class GenericsTests extends XMLBasedAjcTestCase {
 	public void testITDReturningParameterizedType() {
 		runTest("ITD with parameterized type");
 	}
+	
+	public void testPR91267_1() {
+		runTest("NPE using generic methods in aspects 1");
+	}
+
+	public void testPR91267_2() {
+		runTest("NPE using generic methods in aspects 2");
+	}
 }
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
index 8001d474a..8dca8410f 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
@@ -2154,5 +2154,15 @@
         <compile files="Test3.java" options="-1.5"/>
         <run class="Test3"/>
     </ajc-test>
+    
+    <ajc-test dir="java5/generics/bugs/pr91267" title="NPE using generic methods in aspects 1" vm="1.5">
+        <compile files="TestBug1.aj" options="-1.5"/>
+        <run class="TestBug1"/>
+    </ajc-test>
+    
+     <ajc-test dir="java5/generics/bugs/pr91267" title="NPE using generic methods in aspects 2" vm="1.5">
+        <compile files="TestBug2.aj" options="-1.5"/>
+        <run class="TestBug2"/>
+    </ajc-test>
  
 </suite>
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/TypeX.java b/weaver/src/org/aspectj/weaver/TypeX.java
index cd9e352f3..f3f078c37 100644
--- a/weaver/src/org/aspectj/weaver/TypeX.java
+++ b/weaver/src/org/aspectj/weaver/TypeX.java
@@ -92,6 +92,7 @@ public class TypeX implements AnnotatedElement {
     public static TypeX forName(String name) {
         return forSignature(nameToSignature(name));
     }
+	
 
     /** Constructs a TypeX for each java language type name in an incoming array.
      * 
@@ -121,10 +122,10 @@ public class TypeX implements AnnotatedElement {
 		StringBuffer sigAddition = new StringBuffer();
 		sigAddition.append("<");
 		for (int i = 0; i < ret.typeParameters.length; i++) {
-			sigAddition.append(ret.typeParameters[i].signature);
-			sigAddition.append(">");
-			sigAddition.append(";");
+			sigAddition.append(ret.typeParameters[i].signature);			
 		}
+		sigAddition.append(">");
+		sigAddition.append(";");
 		ret.signature = ret.signature + sigAddition.toString();
 		return ret;
     }