last of the dreaded jdtlikehandleprovider changes - 141730#22

asm/src/org/aspectj/asm/internal/JDTLikeHandleProvider.java

@@ -131,6 +131,12 @@ public class JDTLikeHandleProvider implements IElementHandleProvider {
 			}		
 		} else if (ipe.getKind().equals(IProgramElement.Kind.INITIALIZER)) {	
 			return String.valueOf(++initializerCounter).toCharArray();
+		} else if (ipe.getKind().equals(IProgramElement.Kind.CODE)) {
+			int index = CharOperation.lastIndexOf('!',byteCodeName);
+			if (index != -1) {
+				return convertCount(CharOperation.subarray(byteCodeName,
+						index+1,byteCodeName.length));
+			}
 		}
 		return empty;
 	}

tests/features153/jdtlikehandleprovider/Get.aj

@@ -0,0 +1,20 @@
+aspect A1 {
+
+	pointcut getPcd() : get(int C1.x);
+	
+	before() : getPcd() {
+	}
+	
+}
+
+class C1 {
+	
+	int x = 0;
+	
+	public void method1() {
+		int y = x;
+		System.out.println("y " + y);
+		int z = x;
+		System.out.println("z " + z);		
+	}
+}

tests/features153/jdtlikehandleprovider/Handler.aj

@@ -0,0 +1,30 @@
+import java.io.FileNotFoundException;
+
+aspect A {
+
+	pointcut handlerPointcut() : handler(FileNotFoundException);
+	
+	before() : handlerPointcut() {	
+	}
+	
+}
+
+class C {
+	
+	public void method() {
+		try {
+			exceptionMethod();
+		} catch (FileNotFoundException e) {
+			e.printStackTrace();
+		}
+		try {
+			exceptionMethod();
+		} catch (FileNotFoundException e) {
+			e.printStackTrace();
+		}
+	}
+	
+	
+	public void exceptionMethod() throws FileNotFoundException {
+	}
+}

tests/features153/jdtlikehandleprovider/Set.aj

@@ -0,0 +1,19 @@
+aspect A1 {
+
+	pointcut setPcd() : set(int C1.x);
+	
+	before() : setPcd() {
+	}
+	
+}
+
+class C1 {
+	
+	int x = 0;
+	
+	public void method() {
+		x = 1;
+		x = 2;
+	}
+
+}

tests/features153/jdtlikehandleprovider/TwoDiffMethodCalls.aj

@@ -0,0 +1,25 @@
+aspect A {
+
+	pointcut tracedPrint(String s): call(void java.io.PrintStream.println(*)) &&
+		args(s) && !within(A);
+		
+	before(String s): tracedPrint(s) {
+		System.out.println("got you: " + s + " ;)");
+	}	
+		
+	after(String s): tracedPrint(s) {
+		System.out.println("hehe, finished: " + s + " :(");
+	}
+}
+
+class Main {
+
+	public static void main(String[] args) {
+		System.out.println("start");
+	}
+
+	public void method(String[] s) {
+		System.out.println("end");		
+	}
+	
+}

tests/features153/jdtlikehandleprovider/TwoMethodCalls.aj

@@ -0,0 +1,23 @@
+aspect A {
+
+	pointcut tracedPrint(String s): call(void java.io.PrintStream.println(*)) &&
+		args(s) && !within(A);
+		
+	before(String s): tracedPrint(s) {
+		System.out.println("got you: " + s + " ;)");
+	}	
+		
+	after(String s): tracedPrint(s) {
+		System.out.println("hehe, finished: " + s + " :(");
+	}
+}
+
+class Main {
+
+	public static void main(String[] args) {
+		System.out.println("start");
+		System.out.println("end");
+	}
+	
+}
+

tests/src/org/aspectj/systemtest/ajc153/JDTLikeHandleProviderTests.java

@@ -240,6 +240,81 @@ public class JDTLikeHandleProviderTests extends XMLBasedAjcTestCase {
 				"*DeclareWarnings.aj}DeclareWarnings`declare warning!10");
 
 	}
+
+	// these two handles are the same unless we have added a counter
+	// on the end
+	public void testIPEsWithSameNameHaveUniqueHandles_methodCall() {
+		runTest("ipes with same name have unique handles - method-call");	
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String handle1 = "*TwoMethodCalls.aj[Main~main~\\[QString;?method-call(" +
+				"void java.io.PrintStream.println(java.lang.String))";
+		assertNotNull("expected to find node with handle " + handle1 
+				+ ", but did not",top.getElement(handle1));
+
+		String handle2 = "*TwoMethodCalls.aj[Main~main~\\[QString;?method-call(" +
+				"void java.io.PrintStream.println(java.lang.String))!2";
+		assertNotNull("expected to find node with handle " + handle2 
+				+ ", but did not",top.getElement(handle2));
+		
+		String handle3 = "*TwoMethodCalls.aj[Main~main~\\[QString;?method-call(" +
+				"void java.io.PrintStream.println(java.lang.String))!3";
+		assertNull("expected not to find node with handle " + handle3 
+				+ ", but found one",top.getElement(handle3));
+	}
+	
+	// these two handles should be different anyway so second one
+	// shouldn't have the "2"
+	public void testIPEsWithDiffNamesDontHaveCounter_methodCall() {
+		runTest("ipes with different names do not have counter - method-call");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String handle1 = "*TwoDiffMethodCalls.aj[Main~main~\\[QString;?method-call(" +
+				"void java.io.PrintStream.println(java.lang.String))";
+		assertNotNull("expected to find node with handle " + handle1 
+				+ ", but did not",top.getElement(handle1));
+
+		String handle2 = "*TwoDiffMethodCalls.aj[Main~method~\\[QString;?method-call(" +
+				"void java.io.PrintStream.println(java.lang.String))";
+		assertNotNull("expected to find node with handle " + handle2 
+				+ ", but did not",top.getElement(handle2));
+	}
+	
+	public void testIPEsWithSameNameHaveUniqueHandles_handler() {
+		runTest("ipes with same name have unique handles - handler");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String handle1 = "*Handler.aj[C~method?exception-handler(void C." +
+				"<catch>(java.io.FileNotFoundException))";
+		assertNotNull("expected to find node with handle " + handle1 
+				+ ", but did not",top.getElement(handle1));
+
+		String handle2 = "*Handler.aj[C~method?exception-handler(void C." +
+				"<catch>(java.io.FileNotFoundException))!2";
+		assertNotNull("expected to find node with handle " + handle2 
+				+ ", but did not",top.getElement(handle2));
+	}
+	
+	public void testIPEsWithSameNameHaveUniqueHandles_get() {
+		runTest("ipes with same name have unique handles - get");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String handle1 = "*Get.aj[C1~method1?field-get(int C1.x)";
+		assertNotNull("expected to find node with handle " + handle1 
+				+ ", but did not",top.getElement(handle1));
+
+		String handle2 = "*Get.aj[C1~method1?field-get(int C1.x)!2";
+		assertNotNull("expected to find node with handle " + handle2 
+				+ ", but did not",top.getElement(handle2));
+	}
+	
+	public void testIPEsWithSameNameHaveUniqueHandles_set() {
+		runTest("ipes with same name have unique handles - set");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String handle1 = "*Set.aj[C1~method?field-set(int C1.x)";
+		assertNotNull("expected to find node with handle " + handle1 
+				+ ", but did not",top.getElement(handle1));
+
+		String handle2 = "*Set.aj[C1~method?field-set(int C1.x)!2";
+		assertNotNull("expected to find node with handle " + handle2 
+				+ ", but did not",top.getElement(handle2));
+	}
 	
 	//---------- following tests ensure we produce the same handles as jdt -----//
 	//---------- (apart from the prefix)

tests/src/org/aspectj/systemtest/ajc153/jdtlikehandleprovider.xml

@@ -208,4 +208,24 @@
       <compile files="Java5Class.java" options="-emacssym -1.5"/>
     </ajc-test>
 
+    <ajc-test dir="features153/jdtlikehandleprovider" title="ipes with same name have unique handles - method-call">
+      <compile files="TwoMethodCalls.aj" options="-emacssym"/>
+    </ajc-test>
+
+    <ajc-test dir="features153/jdtlikehandleprovider" title="ipes with different names do not have counter - method-call">
+      <compile files="TwoDiffMethodCalls.aj" options="-emacssym"/>
+    </ajc-test>
+
+    <ajc-test dir="features153/jdtlikehandleprovider" title="ipes with same name have unique handles - handler">
+      <compile files="Handler.aj" options="-emacssym"/>
+    </ajc-test>
+
+    <ajc-test dir="features153/jdtlikehandleprovider" title="ipes with same name have unique handles - get">
+      <compile files="Get.aj" options="-emacssym"/>
+    </ajc-test>
+
+    <ajc-test dir="features153/jdtlikehandleprovider" title="ipes with same name have unique handles - set">
+      <compile files="Set.aj" options="-emacssym"/>
+    </ajc-test>
+
 </suite>

tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java

@@ -1494,7 +1494,12 @@ public class MultiProjectIncrementalTests extends AbstractMultiProjectIncrementa
 				warnings.isEmpty());
 		alter("PR152589","inc1");
 		build("PR152589");
-		checkWasFullBuild();
+		if (AsmManager.getDefault().getHandleProvider().dependsOnLocation())
+			  checkWasFullBuild(); // the line number has changed... but nothing structural about the code
+			else 
+			  checkWasntFullBuild(); // the line number has changed... but nothing structural about the code
+
+//		checkWasFullBuild();
 		warnings = MyTaskListManager.getWarningMessages();
 		assertTrue("There should be no warnings after adding a whitespace:\n"
 				+warnings,warnings.isEmpty());	
@@ -1698,4 +1703,4 @@ public class MultiProjectIncrementalTests extends AbstractMultiProjectIncrementa
 		return new File(projDir,"bin"+File.separator+filename);
 	}
 	
-}
+}

weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java

@@ -243,15 +243,33 @@ public class AsmRelationshipProvider {
 		return true;
 	}
 	
-	
+	/**
+	 * Finds or creates a code IProgramElement for the given shadow. 
+	 * 
+	 * The byteCodeName of the created node is set to 'shadowSig.getName() + "!" + counter',
+	 * eg "println!3". The counter is the occurence count of children within
+	 * the enclosingNode which have the same name. So, for example, if a method 
+	 * contains two System.out.println statements, the first one will have 
+	 * byteCodeName 'println!1' and the second will have byteCodeName 'println!2'.
+	 * This is to ensure the two nodes have unique handles when the handles 
+	 * do not depend on sourcelocations. 
+	 * 
+	 * Currently the shadows are examined in the sequence they appear in the 
+	 * source file. This means that the counters are consistent over incremental 
+	 * builds. All aspects are compiled up front and any new aspect created will force
+	 * a full build. Moreover, if the body of the enclosingShadow is changed, then
+	 * the model for this is rebuilt from scratch. 
+	 */
 	private IProgramElement findOrCreateCodeNode(IProgramElement enclosingNode, Member shadowSig, Shadow shadow)
 	{
 		for (Iterator it = enclosingNode.getChildren().iterator(); it.hasNext(); ) {
 			IProgramElement node = (IProgramElement)it.next();
-			if (shadowSig.getName().equals(node.getBytecodeName()) &&
-				shadowSig.getSignature().equals(node.getBytecodeSignature()) &&
-				sourceLinesMatch(node.getSourceLocation(),shadow.getSourceLocation()))
-			{
+			int excl = node.getBytecodeName().lastIndexOf('!');
+			if (((excl != -1 
+					&& shadowSig.getName().equals(node.getBytecodeName().substring(0,excl)))
+					|| shadowSig.getName().equals(node.getBytecodeName()))
+					&& shadowSig.getSignature().equals(node.getBytecodeSignature()) 
+					&& sourceLinesMatch(node.getSourceLocation(),shadow.getSourceLocation())){
 				return node;
 			}
 		}
@@ -265,8 +283,18 @@ public class AsmRelationshipProvider {
 			shadow.toString(),
 			IProgramElement.Kind.CODE,
 			peLoc,0,null,null);
-				
-		peNode.setBytecodeName(shadowSig.getName());
+		
+		// check to see if the enclosing shadow already has children with the 
+		// same name. If so we want to add a counter to the byteCodeName otherwise
+		// we wont get unique handles
+		int numberOfChildrenWithThisName = 0;
+    	for (Iterator it = enclosingNode.getChildren().iterator(); it.hasNext(); ) {
+    		IProgramElement child = (IProgramElement)it.next();
+			if (child.getName().equals(shadow.toString())) {
+				numberOfChildrenWithThisName++;
+			}
+    	}
+		peNode.setBytecodeName(shadowSig.getName() + "!" + String.valueOf(numberOfChildrenWithThisName+1));
 		peNode.setBytecodeSignature(shadowSig.getSignature());
 		enclosingNode.addChild(peNode);
 		return peNode;