test and fix for 132130

7 files changed, 97 insertions, 12 deletions

diff --git a/ajde/testsrc/org/aspectj/ajde/AsmDeclarationsTest.java b/ajde/testsrc/org/aspectj/ajde/AsmDeclarationsTest.java
index 1925ea02b..9411c433d 100644
--- a/ajde/testsrc/org/aspectj/ajde/AsmDeclarationsTest.java
+++ b/ajde/testsrc/org/aspectj/ajde/AsmDeclarationsTest.java
@@ -111,7 +111,7 @@ public class AsmDeclarationsTest extends AjdeTestCase {
 		assertNotNull(fieldNode);		
 		assertEquals(fieldNode.toLabelString(), fieldMsg);
 
-		String methodMsg = "Point.check(int, Line)";
+		String methodMsg = "Point.check(int,Line)";
 		IProgramElement methodNode = model.findElementForLabel(aspect, IProgramElement.Kind.INTER_TYPE_METHOD, methodMsg);
 		assertNotNull(methodNode);		
 		assertEquals(methodNode.toLabelString(), methodMsg);
@@ -135,7 +135,7 @@ public class AsmDeclarationsTest extends AjdeTestCase {
 		assertNotNull(ptctNode);		
 		assertEquals(ptctNode.toLabelString(), ptct);		
 
-		String params = "namedWithArgs(int, int)";
+		String params = "namedWithArgs(int,int)";
 		IProgramElement paramsNode = model.findElementForSignature(aspect, IProgramElement.Kind.POINTCUT, params);
 		assertNotNull(paramsNode);		
 		assertEquals(paramsNode.toLabelString(), params);	
@@ -176,7 +176,7 @@ public class AsmDeclarationsTest extends AjdeTestCase {
 		assertNotNull(namedWithOneArgNode);		
 		assertEquals(namedWithOneArgNode.toLabelString(), namedWithOneArg);		
 
-		String afterReturning = "afterReturning(int, int): namedWithArgs..";
+		String afterReturning = "afterReturning(int,int): namedWithArgs..";
 		IProgramElement afterReturningNode = model.findElementForLabel(aspect, IProgramElement.Kind.ADVICE, afterReturning);
 		assertNotNull(afterReturningNode);		
 		assertEquals(afterReturningNode.toLabelString(), afterReturning);
diff --git a/ajde/testsrc/org/aspectj/ajde/AsmRelationshipsTest.java b/ajde/testsrc/org/aspectj/ajde/AsmRelationshipsTest.java
index ce89a6c6a..1b674dc15 100644
--- a/ajde/testsrc/org/aspectj/ajde/AsmRelationshipsTest.java
+++ b/ajde/testsrc/org/aspectj/ajde/AsmRelationshipsTest.java
@@ -82,7 +82,7 @@ public class AsmRelationshipsTest extends AjdeTestCase {
 	public void testInterTypeDeclarations() {		
 		checkInterTypeMapping("InterTypeDecCoverage", "Point", "Point.xxx", "Point", 
 			"declared on", "aspect declarations", IProgramElement.Kind.INTER_TYPE_FIELD);	
-		checkInterTypeMapping("InterTypeDecCoverage", "Point", "Point.check(int, Line)", 
+		checkInterTypeMapping("InterTypeDecCoverage", "Point", "Point.check(int,Line)", 
 			"Point", "declared on", "aspect declarations", IProgramElement.Kind.INTER_TYPE_METHOD);	
 	}
 
diff --git a/ajdoc/testsrc/org/aspectj/tools/ajdoc/CoverageTestCase.java b/ajdoc/testsrc/org/aspectj/tools/ajdoc/CoverageTestCase.java
index f0d86072d..09d6ede30 100644
--- a/ajdoc/testsrc/org/aspectj/tools/ajdoc/CoverageTestCase.java
+++ b/ajdoc/testsrc/org/aspectj/tools/ajdoc/CoverageTestCase.java
@@ -170,7 +170,7 @@ public class CoverageTestCase extends AjdocTestCase {
         
 		String[] strings = { 
 				"after(): named..",
-				"afterReturning(int, int): namedWithArgs..",
+				"afterReturning(int,int): namedWithArgs..",
 				"afterThrowing(): named..",
 				"before(): named..",
 				"around(int): namedWithOneArg..",
diff --git a/asm/src/org/aspectj/asm/internal/ProgramElement.java b/asm/src/org/aspectj/asm/internal/ProgramElement.java
index e153c1624..b78b3bbda 100644
--- a/asm/src/org/aspectj/asm/internal/ProgramElement.java
+++ b/asm/src/org/aspectj/asm/internal/ProgramElement.java
@@ -427,7 +427,7 @@ public class ProgramElement implements IProgramElement {
 						sb.append(arg);
 					}
 				}
-				if (it.hasNext()) sb.append(", ");
+				if (it.hasNext()) sb.append(",");
 			}
 			sb.append(')');
 		}
diff --git a/tests/bugs151/pr132130.aj b/tests/bugs151/pr132130.aj
new file mode 100644
index 000000000..3259d7232
--- /dev/null
+++ b/tests/bugs151/pr132130.aj
@@ -0,0 +1,17 @@
+ aspect basic {
+
+    declare @method : * debit(..) : @Secured(role="supervisor");
+    declare @constructor : BankAccount+.new(..) : @Secured(role="supervisor");
+}
+
+class BankAccount {
+	
+	public BankAccount(String s, int i) {
+	}
+    public void debit(long accId,long amount) {
+    }
+}
+
+@interface Secured {
+    String role();
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java b/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java
index 93ea08893..2c33e7bb6 100644
--- a/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java
@@ -75,13 +75,13 @@ public class Ajc151Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   	  IProgramElement pe = top.findElementForType("pkg","foo");
   	  assertNotNull("Couldn't find 'foo' element in the tree",pe);
   	  // check that the defaults return the fully qualified arg
-  	  assertEquals("foo(int, java.lang.Object)",pe.toLabelString());
-  	  assertEquals("C.foo(int, java.lang.Object)",pe.toLinkLabelString());
-  	  assertEquals("foo(int, java.lang.Object)",pe.toSignatureString());
+  	  assertEquals("foo(int,java.lang.Object)",pe.toLabelString());
+  	  assertEquals("C.foo(int,java.lang.Object)",pe.toLinkLabelString());
+  	  assertEquals("foo(int,java.lang.Object)",pe.toSignatureString());
   	  // check that can get hold of the non qualified args
-  	  assertEquals("foo(int, Object)",pe.toLabelString(false));
-  	  assertEquals("C.foo(int, Object)",pe.toLinkLabelString(false));
-  	  assertEquals("foo(int, Object)",pe.toSignatureString(false));
+  	  assertEquals("foo(int,Object)",pe.toLabelString(false));
+  	  assertEquals("C.foo(int,Object)",pe.toLinkLabelString(false));
+  	  assertEquals("foo(int,Object)",pe.toSignatureString(false));
 
   	  IProgramElement pe2 = top.findElementForType("pkg","printParameters");
   	  assertNotNull("Couldn't find 'printParameters' element in the tree",pe2);
@@ -192,6 +192,70 @@ public class Ajc151Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   			  + target.toLabelString(),constructor,target);
   }
   
+  
+  public void testDeclareAnnotationAppearsInStructureModel_pr132130() {
+	  //AsmManager.setReporting("c:/debug.txt",true,true,true,true);
+	  runTest("declare annotation appears in structure model when in same file");
+	  IHierarchy top = AsmManager.getDefault().getHierarchy();
+	  
+  	  // get the IProgramElements corresponding to the different code entries
+  	  IProgramElement decam = top.findElementForLabel(top.getRoot(),
+  			  IProgramElement.Kind.DECLARE_ANNOTATION_AT_METHOD,"declare @method: * debit(..) : @Secured(role = \"supervisor\")");  	   	 
+  	  assertNotNull("Couldn't find 'declare @method' element in the tree",decam);
+  	  IProgramElement method = top.findElementForLabel(top.getRoot(),
+  			  IProgramElement.Kind.METHOD,"debit(long,long)");
+  	  assertNotNull("Couldn't find the 'debit(long,long)' method element in the tree",method);
+  	  IProgramElement decac = top.findElementForLabel(top.getRoot(),
+  			  IProgramElement.Kind.DECLARE_ANNOTATION_AT_CONSTRUCTOR,"declare @constructor: BankAccount+.new(..) : @Secured(role = \"supervisor\")");  	   	 
+  	  assertNotNull("Couldn't find 'declare @constructor' element in the tree",decac);
+  	  IProgramElement ctr = top.findElementForLabel(top.getRoot(),
+  			  IProgramElement.Kind.CONSTRUCTOR,"BankAccount(String,int)");
+  	  assertNotNull("Couldn't find the 'BankAccount(String,int)' constructor element in the tree",ctr);
+
+  	  
+  	  // check that decam has a annotates relationship with the debit method
+  	  List matches = AsmManager.getDefault().getRelationshipMap().get(decam);	
+  	  assertNotNull("'declare @method' should have some relationships but does not",matches);
+  	  assertTrue("'declare @method' should have one relationships but has " + matches.size(),matches.size()==1);
+  	  List matchesTargets = ((Relationship)matches.get(0)).getTargets();
+  	  assertTrue("'declare @method' should have one targets but has" + matchesTargets.size(),matchesTargets.size()==1);
+  	  IProgramElement target = AsmManager.getDefault().getHierarchy().findElementForHandle((String)matchesTargets.get(0));
+  	  assertEquals("target of relationship should be the 'debit(long,long)' method but is IPE with label "
+  			  + target.toLabelString(),method,target);
+  	  
+  	  // check that the debit method has an annotated by relationship with the declare @method
+  	  matches = AsmManager.getDefault().getRelationshipMap().get(method);	
+  	  assertNotNull("'debit(long,long)' should have some relationships but does not",matches);
+  	  assertTrue("'debit(long,long)' should have one relationships but has " + matches.size(),matches.size()==1);
+  	  matchesTargets = ((Relationship)matches.get(0)).getTargets();
+  	  assertTrue("'debit(long,long)' should have one targets but has" + matchesTargets.size(),matchesTargets.size()==1);
+  	  target = AsmManager.getDefault().getHierarchy().findElementForHandle((String)matchesTargets.get(0));
+  	  assertEquals("target of relationship should be the 'declare @method' ipe but is IPE with label "
+  			  + target.toLabelString(),decam,target);
+
+  	  // check that decac has a annotates relationship with the constructor
+  	  matches = AsmManager.getDefault().getRelationshipMap().get(decac);	
+  	  assertNotNull("'declare @method' should have some relationships but does not",matches);
+  	  assertTrue("'declare @method' should have one relationships but has " + matches.size(),matches.size()==1);
+  	  matchesTargets = ((Relationship)matches.get(0)).getTargets();
+  	  assertTrue("'declare @method' should have one targets but has" + matchesTargets.size(),matchesTargets.size()==1);
+  	  target = AsmManager.getDefault().getHierarchy().findElementForHandle((String)matchesTargets.get(0));
+  	  assertEquals("target of relationship should be the 'debit(long, long)' method but is IPE with label "
+  			  + target.toLabelString(),ctr,target);
+  	  
+  	  // check that the constructor has an annotated by relationship with the declare @constructor
+  	  matches = AsmManager.getDefault().getRelationshipMap().get(ctr);	
+  	  assertNotNull("'debit(long, long)' should have some relationships but does not",matches);
+  	  assertTrue("'debit(long, long)' should have one relationships but has " + matches.size(),matches.size()==1);
+  	  matchesTargets = ((Relationship)matches.get(0)).getTargets();
+  	  assertTrue("'debit(long, long)' should have one targets but has" + matchesTargets.size(),matchesTargets.size()==1);
+  	  target = AsmManager.getDefault().getHierarchy().findElementForHandle((String)matchesTargets.get(0));
+  	  assertEquals("target of relationship should be the 'declare @method' ipe but is IPE with label "
+  			  + target.toLabelString(),decac,target);
+
+
+  }
+  
   /*
    * @AspectJ bugs and enhancements
    */
diff --git a/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml b/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml
index 0c03c2b0b..5d83c75f5 100644
--- a/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml
+++ b/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml
@@ -256,6 +256,10 @@
         <compile files="pr131932.aj" options="-1.5 -emacssym"/>
     </ajc-test>
 
+    <ajc-test dir="bugs151" title="declare annotation appears in structure model when in same file">
+        <compile files="pr132130.aj" options="-1.5 -emacssym"/>
+    </ajc-test>
+
     <!-- New features down here... when they arent big enough to have their own test file -->
     
     <ajc-test dir="features151/ptw" title="exposing withintype">