tests and fixes for jdtlikehandleprovider, bug 159896

tests/features153/jdtlikehandleprovider/AdviceAndInjar.aj

@@ -0,0 +1,17 @@
+public aspect AdviceAndInjar {
+	
+	pointcut p() : call(* *.*(..)) && !within(AdviceAndInjar);
+
+	before() : p() {
+	}
+	
+	after() : p() {
+	}
+	
+	pointcut p1() : execution(* *.*(..)) && !within(AdviceAndInjar);
+	
+	Object around() : p1() {
+		return proceed();
+	}
+
+}

tests/features153/jdtlikehandleprovider/AspectOnAspectPath.aj

@@ -0,0 +1,17 @@
+package pkg;
+
+public aspect AspectOnAspectPath {
+
+	pointcut p() : call(* *.*(..)) && !within(AspectOnAspectPath);
+	declare warning: p() : "blah";
+	
+	before() : p() {
+	}
+	
+	pointcut exec() : execution(* *.*(..)) && !within(AspectOnAspectPath);
+	declare warning : exec() : "blah2";
+	before() : exec() {
+		
+	}
+	
+}

tests/features153/jdtlikehandleprovider/ClassForAspectpath.java

@@ -0,0 +1,10 @@
+public class ClassForAspectpath {
+	
+	public void foo() {
+		bar();
+	}
+	
+	public void bar() {
+		
+	}
+}

tests/features153/jdtlikehandleprovider/DeclareAndInjar.aj

@@ -0,0 +1,9 @@
+public aspect DeclareAndInjar {
+	
+	pointcut p() : call(* *.*(..)) && !within(DeclareAndInjar);
+	declare warning: p() : "warning";
+
+	pointcut exec() : execution(* *.*(..)) && !within(DeclareAndInjar);
+	declare error : exec() : "error";
+
+}

tests/features153/jdtlikehandleprovider/README.txt

@@ -0,0 +1,7 @@
+To regenerate the jars in this directory:
+
+1. ajc AspectOnAspectPath.aj -outjar jarForAspectPath.jar
+
+2. ajc DeclareAndInjar.aj -outjar declare.jar
+
+3. ajc AdviceAndInjar.aj -outjar advice.jar

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

@@ -316,6 +316,67 @@ public class JDTLikeHandleProviderTests extends XMLBasedAjcTestCase {
 				+ ", but did not",top.getElement(handle2));
 	}
 	
+	public void testTwoPiecesOfBeforeAdviceInInjarAspectHaveUniqueHandles_pr159896() {
+		runTest("advice with same name in injar aspect should have unique handles");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String handle1 = top.findElementForLabel(top.getRoot(),
+				IProgramElement.Kind.ADVICE,"before(): p..").getHandleIdentifier();
+		String handle2 = top.findElementForLabel(top.getRoot(),
+				IProgramElement.Kind.ADVICE,"before(): exec..").getHandleIdentifier();
+		assertFalse("expected the two advice nodes to have unique handles but" +
+				" did not", handle1.equals(handle2));
+	}
+
+	public void testTwoDeclareWarningsInInjarAspectHaveUniqueHandles_pr159896() {
+		runTest("declare warnings in injar aspect should have unique handles");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String handle1 = top.findElementForLabel(top.getRoot(),
+				IProgramElement.Kind.DECLARE_WARNING,"declare warning: \"blah\"").getHandleIdentifier();
+		String handle2 = top.findElementForLabel(top.getRoot(),
+				IProgramElement.Kind.DECLARE_WARNING,"declare warning: \"blah2\"").getHandleIdentifier();
+		assertFalse("expected the two declare warning nodes to have unique handles but" +
+				" did not", handle1.equals(handle2));
+	}
+	
+	// if have one declare warning and one declare error statement within an injar
+	// aspect, neither of them should have a counter (i.e. "!2") at the end of 
+	// their handle
+	public void testOnlyIncrementSameDeclareTypeFromInjar_pr159896() {
+		runTest("dont increment counter for different declares");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String warning = top.findElementForLabel(top.getRoot(), 
+				IProgramElement.Kind.DECLARE_WARNING,"declare warning: \"warning\"").getHandleIdentifier();
+		assertTrue("shouldn't have incremented counter for declare warning handle " +
+				"because only one declare warning statement", 
+				warning.indexOf("!0") == -1 && warning.indexOf("!2") == -1);
+		String error = top.findElementForLabel(top.getRoot(), 
+				IProgramElement.Kind.DECLARE_ERROR,"declare error: \"error\"").getHandleIdentifier();
+		assertTrue("shouldn't have incremented counter for declare error handle " +
+				"because only one declare error statement", 
+				error.indexOf("!0") == -1 && error.indexOf("!2") == -1);
+	}
+	
+	public void testOnlyIncrementSameAdviceKindFromInjar_pr159896() {
+		runTest("dont increment counter for different advice kinds");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		String before = top.findElementForLabel(top.getRoot(), 
+				IProgramElement.Kind.ADVICE,"before(): p..").getHandleIdentifier();
+		assertTrue("shouldn't have incremented counter for before handle " +
+				"because only one before advice", 
+				before.indexOf("!0") == -1 && before.indexOf("!2") == -1 && before.indexOf("!3") == -1 );
+		String after = top.findElementForLabel(top.getRoot(), 
+				IProgramElement.Kind.ADVICE,"after(): p..").getHandleIdentifier();
+		assertTrue("shouldn't have incremented counter for after handle " +
+				"because only one after advice", 
+				after.indexOf("!0") == -1 && after.indexOf("!2") == -1 && after.indexOf("!3") == -1 );
+		String around = top.findElementForLabel(top.getRoot(), 
+				IProgramElement.Kind.ADVICE,"around(): p1..").getHandleIdentifier();
+		assertTrue("shouldn't have incremented counter for around handle " +
+				"because only one around advice", 
+				around.indexOf("!0") == -1 && around.indexOf("!2") == -1 && around.indexOf("!3") == -1 );
+		
+	}
+	
 	//---------- following tests ensure we produce the same handles as jdt -----//
 	//---------- (apart from the prefix)
 	

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

@@ -228,4 +228,29 @@
       <compile files="Set.aj" options="-emacssym"/>
     </ajc-test>
 
+    <ajc-test dir="features153/jdtlikehandleprovider" title="advice with same name in injar aspect should have unique handles">
+      <compile files="ClassForAspectpath.java" options="-emacssym" aspectpath="jarForAspectPath.jar">
+        <message kind="warning" text="blah"/>
+        <message kind="warning" text="blah2"/>
+      </compile>
+    </ajc-test>
+
+    <ajc-test dir="features153/jdtlikehandleprovider" title="declare warnings in injar aspect should have unique handles">
+      <compile files="ClassForAspectpath.java" options="-emacssym" aspectpath="jarForAspectPath.jar">
+        <message kind="warning" text="blah"/>
+        <message kind="warning" text="blah2"/>
+      </compile>
+    </ajc-test>
+
+    <ajc-test dir="features153/jdtlikehandleprovider" title="dont increment counter for different declares">
+      <compile files="ClassForAspectpath.java" options="-emacssym" aspectpath="declare.jar">
+        <message kind="warning" text="warning"/>
+        <message kind="error" text="error"/>
+      </compile>
+    </ajc-test>
+
+    <ajc-test dir="features153/jdtlikehandleprovider" title="dont increment counter for different advice kinds">
+      <compile files="ClassForAspectpath.java" options="-emacssym" aspectpath="advice.jar"/>
+    </ajc-test>
+
 </suite>

weaver/src/org/aspectj/weaver/ShadowMunger.java

@@ -273,31 +273,65 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH
     }
     
     private void addChildNodes(IProgramElement parent, Collection children) {
+    	int afterCtr = 1;
+    	int aroundCtr = 1;
+    	int beforeCtr = 1;
+    	int deCtr = 1;
+    	int dwCtr = 1;
     	for (Iterator iter = children.iterator(); iter.hasNext();) {
 			Object element = (Object) iter.next();
 			if (element instanceof DeclareErrorOrWarning) {
 				DeclareErrorOrWarning decl = (DeclareErrorOrWarning)element;
-			   	IProgramElement deowNode = new ProgramElement(
-		    			decl.isError() ? "declare error" : "declare warning",
-		    			decl.isError() ? IProgramElement.Kind.DECLARE_ERROR : IProgramElement.Kind.DECLARE_WARNING,
-		    			getBinarySourceLocation(decl.getSourceLocation()),
-		    			decl.getDeclaringType().getModifiers(),
-		    			null,null); 
-		    	deowNode.setDetails("\"" + AsmRelationshipUtils.genDeclareMessage(decl.getMessage()) + "\"");
-		    	parent.addChild(deowNode);
+				int counter = 0;
+				if (decl.isError()) {
+					counter = deCtr++;
+				} else {
+					counter = dwCtr++;
+				}
+		    	parent.addChild(createDeclareErrorOrWarningChild(decl,counter));
 			} else if (element instanceof BcelAdvice) {
 				BcelAdvice advice = (BcelAdvice)element;
-				IProgramElement adviceNode = new ProgramElement(
-		    			advice.kind.getName(),
-		    			IProgramElement.Kind.ADVICE,
-		    			getBinarySourceLocation(advice.getSourceLocation()),
-		    			advice.signature.getModifiers(),null,Collections.EMPTY_LIST);
-		    	adviceNode.setDetails(AsmRelationshipUtils.genPointcutDetails(advice.getPointcut()));
-		    	parent.addChild(adviceNode);
+				int counter = 0;
+				if (advice.getKind().equals(AdviceKind.Before)) {
+					counter = beforeCtr++;
+				} else if (advice.getKind().equals(AdviceKind.Around)){
+					counter = aroundCtr++;
+				} else {
+					counter = afterCtr++;
+				}
+		    	parent.addChild(createAdviceChild(advice,counter));
 			}
 		}
     }
     
+    private IProgramElement createDeclareErrorOrWarningChild(
+    		DeclareErrorOrWarning decl, int count) {
+	   	IProgramElement deowNode = new ProgramElement(
+    			decl.getName(),
+    			decl.isError() ? IProgramElement.Kind.DECLARE_ERROR : IProgramElement.Kind.DECLARE_WARNING,
+    			getBinarySourceLocation(decl.getSourceLocation()),
+    			decl.getDeclaringType().getModifiers(),
+    			null,null); 
+    	deowNode.setDetails("\"" + AsmRelationshipUtils.genDeclareMessage(decl.getMessage()) + "\"");
+    	if (count != -1) {
+    		deowNode.setBytecodeName(decl.getName() + "_" + count);
+    	}
+    	return deowNode;
+    }
+    
+    private IProgramElement createAdviceChild(BcelAdvice advice, int counter ) {
+		IProgramElement adviceNode = new ProgramElement(
+    			advice.kind.getName(),
+    			IProgramElement.Kind.ADVICE,
+    			getBinarySourceLocation(advice.getSourceLocation()),
+    			advice.signature.getModifiers(),null,Collections.EMPTY_LIST);
+    	adviceNode.setDetails(AsmRelationshipUtils.genPointcutDetails(advice.getPointcut()));
+    	if (counter != 1) {
+			adviceNode.setBytecodeName(advice.getKind().getName() + "$" + counter + "$");
+		}
+    	return adviceNode;
+    }
+    
     /**
      * Returns the binarySourceLocation for the given sourcelocation. This
      * isn't cached because it's used when faulting in the binary nodes

weaver/src/org/aspectj/weaver/patterns/DeclareErrorOrWarning.java

@@ -31,6 +31,9 @@ public class DeclareErrorOrWarning extends Declare {
 		this.message = message;
 	}
 	
+	/**
+	 * returns "declare warning: <message>" or "declare error: <message>"
+	 */
 	public String toString() {
 		StringBuffer buf = new StringBuffer();
 		buf.append("declare ");
@@ -111,4 +114,15 @@ public class DeclareErrorOrWarning extends Declare {
 	public String getNameSuffix() {
 		return "eow";
 	}
+	
+	/**
+	 * returns "declare warning" or "declare error"
+	 */
+	public String getName() {
+		StringBuffer buf = new StringBuffer();
+		buf.append("declare ");
+		if (isError) buf.append("error");
+		else buf.append("warning");	
+		return buf.toString();
+	}
 }