more changes to make if point cut generated names stable

12 files changed, 229 insertions, 52 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java
index c5320f08a..e9aca3644 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java
@@ -61,6 +61,8 @@ public class AdviceDeclaration extends AjMethodDeclaration {
 
 	public AdviceKind kind; // set during parsing, referenced by Proceed and AsmElementFormatter
 	private int extraArgumentFlags = 0;
+	
+	public int adviceSequenceNumberInType;
 
 	public MethodBinding proceedMethodBinding; // set during this.resolveStaments, referenced by Proceed
 	public List proceedCalls = new ArrayList(2); // populated during Proceed.findEnclosingAround
@@ -323,7 +325,7 @@ public class AdviceDeclaration extends AjMethodDeclaration {
 	// override, Called by ClassScope.postParse
 	public void postParse(TypeDeclaration typeDec) {
 		AspectDeclaration aspectDecl = (AspectDeclaration) typeDec;
-		int adviceSequenceNumberInType = aspectDecl.adviceCounter++;
+		adviceSequenceNumberInType = aspectDecl.adviceCounter++;
 
 		StringBuffer stringifiedPointcut = new StringBuffer(30);
 		pointcutDesignator.print(0, stringifiedPointcut);
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/IfPseudoToken.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/IfPseudoToken.java
index 821b8b3be..7d7842c6d 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/IfPseudoToken.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/IfPseudoToken.java
@@ -34,6 +34,7 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeReference;
 import org.aspectj.org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
 import org.aspectj.org.eclipse.jdt.internal.compiler.parser.Parser;
+import org.aspectj.weaver.BCException;
 import org.aspectj.weaver.Member;
 import org.aspectj.weaver.ResolvedMemberImpl;
 import org.aspectj.weaver.UnresolvedType;
@@ -82,17 +83,17 @@ public class IfPseudoToken extends PseudoToken {
 	/**
 	 * enclosingDec is either AdviceDeclaration or PointcutDeclaration
 	 */
-	public void postParse(TypeDeclaration typeDec,
-			MethodDeclaration enclosingDec) {
+	public int postParse(TypeDeclaration typeDec,
+			MethodDeclaration enclosingDec, int counter) {
 		// typeDec.scope.problemReporter().signalError(sourceStart, sourceEnd,
 		// "if pcd is not implemented in 1.1alpha1");
 		// XXX need to implement correctly
 		if (pointcut == null)
-			return;
+			return 0;
 
-		testMethod = makeIfMethod(enclosingDec.compilationResult, enclosingDec,
-				typeDec);
+		testMethod = makeIfMethod(enclosingDec.compilationResult, enclosingDec, typeDec, counter);
 		AstUtil.addMethodDeclaration(typeDec, testMethod);
+		return 1;
 	}
 
 	private final static char[] CodeGenerationHint = "CodeGenerationHint".toCharArray();
@@ -100,11 +101,9 @@ public class IfPseudoToken extends PseudoToken {
 	private final static char[] IfNameSuffix = "ifNameSuffix".toCharArray();
 	
 	// XXX todo: make sure that errors in Arguments only get displayed once
-	private MethodDeclaration makeIfMethod(CompilationResult result,
-			MethodDeclaration enclosingDec, TypeDeclaration containingTypeDec) {
+	private MethodDeclaration makeIfMethod(CompilationResult result, MethodDeclaration enclosingDec, TypeDeclaration containingTypeDec, int counter) {
 		MethodDeclaration ret = new IfMethodDeclaration(result, pointcut);
-		ret.modifiers = ClassFileConstants.AccStatic
-				| ClassFileConstants.AccFinal | ClassFileConstants.AccPublic;
+		ret.modifiers = ClassFileConstants.AccStatic | ClassFileConstants.AccFinal | ClassFileConstants.AccPublic;
 		ret.returnType = AstUtil.makeTypeReference(TypeBinding.BOOLEAN);
 		
 		String nameSuffix = null;
@@ -144,29 +143,41 @@ public class IfPseudoToken extends PseudoToken {
 		StringBuffer ifSelector = new StringBuffer();
 		ifSelector.append("ajc$if$");
 		if (nameSuffix == null || nameSuffix.length()==0) {
-			ifSelector.append(Integer.toHexString(expr.sourceStart));
+			boolean computedName = false;
+			try {
+				// possibly even better logic for more reliable name:
+				if (enclosingDec instanceof AdviceDeclaration) {
+					// name is ajc$if$<adviceSequenceNumber>[$<ifnumberinPcd>]$<hashcodeOfIfExpressionInHex>
+					ifSelector.append(((AdviceDeclaration)enclosingDec).adviceSequenceNumberInType).append("$");
+					if (counter!=0) {
+						ifSelector.append(counter);
+						ifSelector.append("$");
+					}
+					ifSelector.append(Integer.toHexString(expr.toString().hashCode()));
+					computedName = true;
+				} else if (enclosingDec instanceof PointcutDeclaration) {
+					if (counter!=0) {
+						ifSelector.append(counter);
+						ifSelector.append("$");
+					}
+					StringBuilder toHash = new StringBuilder(((PointcutDeclaration) enclosingDec).getPointcutText());
+					toHash.append(expr.toString());
+					// name is pointcut selector then $if[$<ifnumberinpcd>]$<hashcodeofpointcuttextandexpressiontext>
+					ifSelector.append(Integer.toHexString(toHash.toString().hashCode()));
+					computedName = true;
+				}
+			} catch (Throwable t) {
+				throw new IllegalStateException(t);
+				// let it build a name the 'old way'
+			}
+			if (!computedName) {
+				ifSelector.append(Integer.toHexString(expr.sourceStart));
+			}
 		} else {
 			ifSelector.append(nameSuffix);
 		}
 
-		// possibly even better logic for more reliable name:
-		// if (enclosingDec instanceof AdviceDeclaration) {
-		// // name is
-		// ajc$if$<adviceSequenceNumber>$<hashcodeOfIfExpressionInHex>
-		// ifSelector.append("ajc$if$");
-		// ifSelector.append(((AdviceDeclaration)
-		// enclosingDec).adviceSequenceNumberInType);
-		// ifSelector.append("$").append(Integer.toHexString(expr.toString().hashCode()));
-		// } else if (enclosingDec instanceof PointcutDeclaration) {
-		// // name is pointcut selector then $if$<hashcodeOfIfExpressionInHex>
-		// ifSelector.append(((PointcutDeclaration) enclosingDec).selector);
-		// ifSelector.append("$if$");
-		// ifSelector.append(Integer.toHexString(expr.toString().hashCode()));
-		// } else {
-		// throw new BCException("Unexpected enclosing declaration of " +
-		// enclosingDec + " for if pointcut designator");
-		// }
-		// hashcode of expression
+		
 		ret.selector = ifSelector.toString().toCharArray();
 		ret.arguments = makeArguments(enclosingDec, containingTypeDec);
 		ret.statements = new Statement[] { new ReturnStatement(expr,
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PointcutDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PointcutDeclaration.java
index a95a3e7f9..f10492047 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PointcutDeclaration.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PointcutDeclaration.java
@@ -146,7 +146,7 @@ public class PointcutDeclaration extends AjMethodDeclaration {
 		generateSyntheticPointcutMethod = true;
 	}
 
-	private String getPointcutText() {
+	public String getPointcutText() {
 		String text = getPointcut().toString();
 		if (text.indexOf("BindingTypePattern") == -1)
 			return text;
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoToken.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoToken.java
index 0d75f5f98..ac679ad1a 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoToken.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoToken.java
@@ -80,8 +80,9 @@ public class PseudoToken extends ASTNode implements IToken {
 		return "unknown";
 	}
 	
-	public void postParse(TypeDeclaration typeDec, MethodDeclaration enclosingDec) {
+	public int postParse(TypeDeclaration typeDec, MethodDeclaration enclosingDec, int tokenNumber) {
 		// nothing to do typically
+		return 0;
 	}
 
 	/* (non-Javadoc)
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoTokens.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoTokens.java
index c71d2fbb1..4777d0faf 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoTokens.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoTokens.java
@@ -152,8 +152,9 @@ public class PseudoTokens extends ASTNode {
 	
 	
 	public void postParse(TypeDeclaration typeDec, MethodDeclaration enclosingDec) {
+		int counter = 0; // Counter can be used by postParse as a value to compute uniqueness (if required)
 		for (int i=0, len=tokens.length; i < len; i++) {
-			tokens[i].postParse(typeDec, enclosingDec);
+			counter+=tokens[i].postParse(typeDec, enclosingDec, counter);
 		}
 	}
 	
diff --git a/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java b/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java
index 9ecd5d885..29ada2727 100644
--- a/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java
+++ b/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java
@@ -319,12 +319,19 @@ public abstract class XMLBasedAjcTestCase extends AjcTestCase {
 	}
 
 	protected Method getMethodStartsWith(JavaClass jc, String prefix) {
+		return getMethodStartsWith(jc,prefix,1);
+	}
+	
+	protected Method getMethodStartsWith(JavaClass jc, String prefix, int whichone) {
 		Method[] meths = jc.getMethods();
 		for (int i = 0; i < meths.length; i++) {
 			Method method = meths[i];
 			System.out.println(method);
 			if (method.getName().startsWith(prefix)) {
-				return method;
+				whichone--;
+				if (whichone==0) {
+					return method;
+				}
 			}
 		}
 		return null;
diff --git a/tests/bugs172/pr398246/Code5.java b/tests/bugs172/pr398246/Code5.java
new file mode 100644
index 000000000..50512d496
--- /dev/null
+++ b/tests/bugs172/pr398246/Code5.java
@@ -0,0 +1,20 @@
+import java.lang.annotation.*;
+
+public class Code5 {
+  public static boolean isTrue = true;
+
+  public void m() {
+  }
+  public static void main(String []argv) {
+    new Code5().m();
+  }
+}
+
+aspect X {
+
+  pointcut p(): execution(* Code*.*(..)) && if(Code5.isTrue);
+
+  before(): p() {
+    System.out.println("advice");
+  }
+}
diff --git a/tests/bugs172/pr398246/Code5a.java b/tests/bugs172/pr398246/Code5a.java
new file mode 100644
index 000000000..0f5562640
--- /dev/null
+++ b/tests/bugs172/pr398246/Code5a.java
@@ -0,0 +1,25 @@
+import java.lang.annotation.*;
+
+public class Code5a {
+  public static boolean isTrue = true;
+
+  public void m() {
+  }
+  public static void main(String []argv) {
+    new Code5a().m();
+  }
+}
+
+
+// more white space, on purpose
+
+
+
+aspect X2 {
+
+
+  pointcut p(): execution(* Code*.*(..)) && if(Code5.isTrue);
+  before(): p() {
+    System.out.println("advice");
+  }
+}
diff --git a/tests/bugs172/pr398246/Code6.java b/tests/bugs172/pr398246/Code6.java
new file mode 100644
index 000000000..e032eb7c9
--- /dev/null
+++ b/tests/bugs172/pr398246/Code6.java
@@ -0,0 +1,22 @@
+import java.lang.annotation.*;
+
+public class Code6 {
+  public static boolean isTrue = true;
+  public static boolean isTrue2 = true;
+
+  public void m() {
+  }
+
+  public static void main(String []argv) {
+    new Code6().m();
+  }
+}
+
+aspect X {
+
+  pointcut p(): execution(* Code*.*(..)) && if(Code6.isTrue) && if(Code6.isTrue2);
+
+  before(): p() {
+    System.out.println("advice");
+  }
+}
diff --git a/tests/bugs172/pr398246/Code7.java b/tests/bugs172/pr398246/Code7.java
new file mode 100644
index 000000000..e8d47a5a1
--- /dev/null
+++ b/tests/bugs172/pr398246/Code7.java
@@ -0,0 +1,20 @@
+import java.lang.annotation.*;
+
+public class Code7 {
+  public static boolean isTrue = true;
+  public static boolean isTrue2 = true;
+
+  public void m() {
+  }
+
+  public static void main(String []argv) {
+    new Code7().m();
+  }
+}
+
+aspect X {
+
+ before(): execution(* Code*.*(..)) && if(Code7.isTrue) && if(Code7.isTrue2) {
+    System.out.println("advice");
+  }
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java b/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java
index eea4030d0..fd9454164 100644
--- a/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java
@@ -23,36 +23,69 @@ import org.aspectj.testing.XMLBasedAjcTestCase;
  */
 public class Ajc172Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
 
-	public void testIfPointcutNames_pr398246() throws Exception{
+	public void testIfPointcutNames_pr398246() throws Exception {
 		runTest("if pointcut names");
-		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"X");
-		Method m = getMethodStartsWith(jc,"ajc$if");
-		assertEquals("ajc$if$andy",m.getName());
+		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X");
+		Method m = getMethodStartsWith(jc, "ajc$if");
+		assertEquals("ajc$if$andy", m.getName());
 	}
-	
+
 	public void testIfPointcutNames_pr398246_2() throws Exception {
 		runTest("if pointcut names 2");
-		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"X");
-		Method m = getMethodStartsWith(jc,"ajc$if");
-		assertEquals("ajc$if$fred",m.getName());
+		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X");
+		Method m = getMethodStartsWith(jc, "ajc$if");
+		assertEquals("ajc$if$fred", m.getName());
 	}
-	
+
 	// fully qualified annotation name is used
 	public void testIfPointcutNames_pr398246_3() throws Exception {
 		runTest("if pointcut names 3");
-		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"X");
-		Method m = getMethodStartsWith(jc,"ajc$if");
-		assertEquals("ajc$if$barney",m.getName());
+		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X");
+		Method m = getMethodStartsWith(jc, "ajc$if");
+		assertEquals("ajc$if$barney", m.getName());
 	}
-	
-	// compiling a class later than the initial build - does it pick up the right if clause name?
-	public void testIfPointcutNames_pr398246_4() throws Exception{
+
+	// compiling a class later than the initial build - does it pick up the
+	// right if clause name?
+	public void testIfPointcutNames_pr398246_4() throws Exception {
 		runTest("if pointcut names 4");
-		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"X");
-		Method m = getMethodStartsWith(jc,"ajc$if");
-		assertEquals("ajc$if$sid",m.getName());
+		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X");
+		Method m = getMethodStartsWith(jc, "ajc$if");
+		assertEquals("ajc$if$sid", m.getName());
+	}
+
+	// new style generated names
+	public void testIfPointcutNames_pr398246_5() throws Exception {
+		runTest("if pointcut names 5");
+		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X");
+		Method m = getMethodStartsWith(jc, "ajc$if");
+		assertEquals("ajc$if$ac0cb804", m.getName());
+
+		jc = getClassFrom(ajc.getSandboxDirectory(), "X2");
+		m = getMethodStartsWith(jc, "ajc$if");
+		assertEquals("ajc$if$ac0cb804", m.getName());
 	}
-	
+
+	// new style generated names - multiple ifs in one pointcut
+	public void testIfPointcutNames_pr398246_6() throws Exception {
+		runTest("if pointcut names 6");
+		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X");
+		Method m = getMethodStartsWith(jc, "ajc$if",1);
+		assertEquals("ajc$if$aac93da8", m.getName());
+		m = getMethodStartsWith(jc, "ajc$if",2);
+		assertEquals("ajc$if$1$ae5e778a", m.getName());
+	}
+
+	// new style generated names - multiple ifs in one advice
+	public void testIfPointcutNames_pr398246_7() throws Exception {
+		runTest("if pointcut names 7");
+		JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X");
+		Method m = getMethodStartsWith(jc, "ajc$if",1);
+		assertEquals("ajc$if$1$ac0607c", m.getName());
+		m = getMethodStartsWith(jc, "ajc$if",2);
+		assertEquals("ajc$if$1$1$4d4baf36", m.getName());
+	}
+
 	public void testOptionalAspects_pr398588() {
 		runTest("optional aspects");
 	}
diff --git a/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml b/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml
index 7dc6ce11b..606ae311e 100644
--- a/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml
+++ b/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml
@@ -84,6 +84,41 @@
       	</stdout>
       </run>
     </ajc-test>
+    
+    <ajc-test dir="bugs172/pr398246" title="if pointcut names 5">
+      <compile files="Code5.java Code5a.java" options="-1.5">
+      </compile>
+      <run class="Code5" options="-1.5">
+      	<stdout>
+      	<line text="advice"/>
+      	<line text="advice"/>
+      	<line text="advice"/>
+      	<line text="advice"/>
+      	</stdout>
+      </run>
+    </ajc-test>
+    
+    <ajc-test dir="bugs172/pr398246" title="if pointcut names 6">
+      <compile files="Code6.java" options="-1.5">
+      </compile>
+      <run class="Code6" options="-1.5">
+      	<stdout>      	
+      	<line text="advice"/>
+      	<line text="advice"/>
+      	</stdout>
+      </run>
+    </ajc-test>
+    
+     <ajc-test dir="bugs172/pr398246" title="if pointcut names 7">
+      <compile files="Code7.java" options="-1.5">
+      </compile>
+      <run class="Code7" options="-1.5">
+      	<stdout>
+      	<line text="advice"/>
+      	<line text="advice"/>
+      	</stdout>
+      </run>
+    </ajc-test>
   
 	<ajc-test dir="bugs172/pr389750" title="inconsistent class file">
       <compile files="Code.aj" options="-1.5">