test and fixes for Bug 145963: add injar aspects to the model

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AsmElementFormatter.java

@@ -34,16 +34,14 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.ast.MethodDeclaration;
 import org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeReference;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
 import org.aspectj.weaver.AdviceKind;
+import org.aspectj.weaver.AsmRelationshipUtils;
 import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.UnresolvedType;
-import org.aspectj.weaver.patterns.AndPointcut;
 import org.aspectj.weaver.patterns.DeclareAnnotation;
 import org.aspectj.weaver.patterns.DeclareErrorOrWarning;
 import org.aspectj.weaver.patterns.DeclareParents;
 import org.aspectj.weaver.patterns.DeclarePrecedence;
 import org.aspectj.weaver.patterns.DeclareSoft;
-import org.aspectj.weaver.patterns.OrPointcut;
-import org.aspectj.weaver.patterns.ReferencePointcut;
 import org.aspectj.weaver.patterns.TypePattern;
 import org.aspectj.weaver.patterns.TypePatternList;
 
@@ -52,19 +50,6 @@ import org.aspectj.weaver.patterns.TypePatternList;
  */
 public class AsmElementFormatter {
 
-	public static final String UNDEFINED="<undefined>";
-	public static final String DECLARE_PRECEDENCE = "precedence";
-	public static final String DECLARE_SOFT = "soft";
-	public static final String DECLARE_PARENTS = "parents";
-	public static final String DECLARE_WARNING = "warning";
-	public static final String DECLARE_ERROR = "error";
-	public static final String DECLARE_UNKNONWN = "<unknown declare>";
-	public static final String POINTCUT_ABSTRACT = "<abstract pointcut>";
-	public static final String POINTCUT_ANONYMOUS = "<anonymous pointcut>";
-	public static final String DOUBLE_DOTS = "..";
-	public static final int MAX_MESSAGE_LENGTH = 18;
-	public static final String DEC_LABEL = "declare";
-
 	public void genLabelAndKind(MethodDeclaration methodDeclaration, IProgramElement node) {
 		
 		if (methodDeclaration instanceof AdviceDeclaration) { 
@@ -77,28 +62,9 @@ public class AsmElementFormatter {
 	
 			StringBuffer details = new StringBuffer();
 			if (ad.pointcutDesignator != null) {	
-				if (ad.pointcutDesignator.getPointcut() instanceof ReferencePointcut) {
-					ReferencePointcut rp = (ReferencePointcut)ad.pointcutDesignator.getPointcut();
-					details.append(rp.name).append("..");
-				} else if (ad.pointcutDesignator.getPointcut() instanceof AndPointcut) {
-					AndPointcut ap = (AndPointcut)ad.pointcutDesignator.getPointcut();
-					if (ap.getLeft() instanceof ReferencePointcut) {
-						details.append(ap.getLeft().toString()).append(DOUBLE_DOTS);	
-					} else {
-						details.append(POINTCUT_ANONYMOUS).append(DOUBLE_DOTS);
-					}
-				} else if (ad.pointcutDesignator.getPointcut() instanceof OrPointcut) {
-					OrPointcut op = (OrPointcut)ad.pointcutDesignator.getPointcut();
-					if (op.getLeft() instanceof ReferencePointcut) {
-						details.append(op.getLeft().toString()).append(DOUBLE_DOTS);	
-					} else {
-						details.append(POINTCUT_ANONYMOUS).append(DOUBLE_DOTS);
-					}
-				} else {
-					details.append(POINTCUT_ANONYMOUS);
-				}
+				details.append(AsmRelationshipUtils.genPointcutDetails(ad.pointcutDesignator.getPointcut()));
 			} else {
-				details.append(POINTCUT_ABSTRACT);
+				details.append(AsmRelationshipUtils.POINTCUT_ABSTRACT);
 			} 
 			node.setName(ad.kind.toString());
 			//if (details.length()!=0) 
@@ -113,25 +79,25 @@ public class AsmElementFormatter {
 			
 		} else if (methodDeclaration instanceof DeclareDeclaration) { 
 			DeclareDeclaration declare = (DeclareDeclaration)methodDeclaration;
-			String name = DEC_LABEL + " ";
+			String name = AsmRelationshipUtils.DEC_LABEL + " ";
 			if (declare.declareDecl instanceof DeclareErrorOrWarning) {
 				DeclareErrorOrWarning deow = (DeclareErrorOrWarning)declare.declareDecl;
 				
 				if (deow.isError()) {
 					node.setKind( IProgramElement.Kind.DECLARE_ERROR);
-					name += DECLARE_ERROR;
+					name += AsmRelationshipUtils.DECLARE_ERROR;
 				} else {
 					node.setKind( IProgramElement.Kind.DECLARE_WARNING);
-					name += DECLARE_WARNING;
+					name += AsmRelationshipUtils.DECLARE_WARNING;
 				}
 				node.setName(name) ;
-				node.setDetails("\"" + genDeclareMessage(deow.getMessage()) + "\"");
+				node.setDetails("\"" + AsmRelationshipUtils.genDeclareMessage(deow.getMessage()) + "\"");
 				
 			} else if (declare.declareDecl instanceof DeclareParents) {
 
 				node.setKind( IProgramElement.Kind.DECLARE_PARENTS);
 				DeclareParents dp = (DeclareParents)declare.declareDecl;
-				node.setName(name + DECLARE_PARENTS);
+				node.setName(name + AsmRelationshipUtils.DECLARE_PARENTS);
 				
 				String kindOfDP = null;
 				StringBuffer details = new StringBuffer("");
@@ -161,13 +127,13 @@ public class AsmElementFormatter {
 			} else if (declare.declareDecl instanceof DeclareSoft) {
 				node.setKind( IProgramElement.Kind.DECLARE_SOFT);
 				DeclareSoft ds = (DeclareSoft)declare.declareDecl;
-				node.setName(name + DECLARE_SOFT);
+				node.setName(name + AsmRelationshipUtils.DECLARE_SOFT);
 				node.setDetails(genTypePatternLabel(ds.getException()));
 				
 			} else if (declare.declareDecl instanceof DeclarePrecedence) {
 				node.setKind( IProgramElement.Kind.DECLARE_PRECEDENCE);
 				DeclarePrecedence ds = (DeclarePrecedence)declare.declareDecl;
-				node.setName(name + DECLARE_PRECEDENCE);
+				node.setName(name + AsmRelationshipUtils.DECLARE_PRECEDENCE);
 				node.setDetails(genPrecedenceListLabel(ds.getPatterns()));
 				
 			} else if (declare.declareDecl instanceof DeclareAnnotation) {
@@ -188,7 +154,7 @@ public class AsmElementFormatter {
 			    
 			} else {
 				node.setKind(IProgramElement.Kind.ERROR);
-				node.setName(DECLARE_UNKNONWN);
+				node.setName(AsmRelationshipUtils.DECLARE_UNKNONWN);
 			}
 			
 		} else if (methodDeclaration instanceof InterTypeDeclaration) {
@@ -247,7 +213,7 @@ public class AsmElementFormatter {
 	                                   || "Lorg/aspectj/lang/annotation/Around;".equals(annotationSig)) {
 	                            node.setKind(IProgramElement.Kind.ADVICE);
 	                            //TODO AV - all are considered anonymous - is that ok?
-	                            node.setDetails(POINTCUT_ANONYMOUS);
+	                            node.setDetails(AsmRelationshipUtils.POINTCUT_ANONYMOUS);
 	                            break;
 	                        }
                         }
@@ -348,15 +314,6 @@ public class AsmElementFormatter {
 		return label;
 		
 	}
-
-	public String genDeclareMessage(String message) {
-		int length = message.length();
-		if (length < MAX_MESSAGE_LENGTH) {
-			return message;
-		} else {
-			return message.substring(0, MAX_MESSAGE_LENGTH-1) + "..";
-		}
-	}
 	
 //	// TODO: 
 //	private String translateAdviceName(String label) {
@@ -387,7 +344,8 @@ public class AsmElementFormatter {
 //		}
 //	}
 
-	// !!! move or replace
+
+	//	 !!! move or replace
 	private String translatePointcutName(String name) {
 		int index = name.indexOf("$$")+2;
 		int endIndex = name.lastIndexOf('$');

tests/bugs152/pr145963/AbstractAspect.aj

@@ -0,0 +1,10 @@
+package pkg;
+
+public abstract aspect AbstractAspect {
+
+	public abstract pointcut abstractPCD();
+	
+	before() : abstractPCD() {
+	}
+	
+}

tests/bugs152/pr145963/AspectInDefaultPackage.aj

@@ -0,0 +1,18 @@
+
+public aspect AspectInDefaultPackage {
+
+	public pointcut execM1() : execution(* pack.C.method1(..));
+	public pointcut execM2() : execution(* pack.C.method2(..));
+	
+	before() : execM1() && this(pack.C) {		
+	}
+	
+	before() : execM2() || execM1() {
+	}
+	
+	before() : execution(* pack.C.method1()) {
+	}
+
+	
+	
+}

tests/bugs152/pr145963/C.java

@@ -0,0 +1,12 @@
+package pack;
+
+public class C {
+
+	public void method1() {
+		new C().method2();
+	}
+	
+	public void method2() {
+	}
+	
+}

tests/bugs152/pr145963/ConcreteAspect.aj

@@ -0,0 +1,17 @@
+package pkg;
+
+public aspect ConcreteAspect {
+	
+	public pointcut execM1() : execution(* pack.C.method1(..));
+	public pointcut execM2() : execution(* pack.C.method2(..));
+	
+	before() : execM1() && this(pack.C) {		
+	}
+	
+	before() : execM2() || execM1() {
+	}
+	
+	before() : execution(* pack.C.method1()) {
+	}
+
+}

tests/bugs152/pr145963/README.txt

@@ -0,0 +1,16 @@
+To recreate the jar files create AspectJ projects within Eclipse containing the
+required files, right click and select 'Export > Java > JAR file with AspectJ Support'
+
+jar file					files contained in the jar file
+--------					-------------------------------
+adviceAndDeow.jar			A.aj, Deow.aj, Itd.aj, NewClass.java
+adviceLabels.jar			ConcreteAspect.aj
+aspectInDefaultPackage.jar	AspectInDefaultPackage.aj
+
+
+
+may need classpath entries on some of these: ?
+
+ajc A.aj Deow.aj Itd.aj NewClass.java -outjar adviceAndDeow.jar
+ajc ConcreteAspect.aj -outjar adviceLabels.jar
+ajc AspectInDefaultPackage.aj -outjar aspectInDefaultPackage.jar

tests/bugs152/pr145963/SrcAspect.aj

@@ -0,0 +1,10 @@
+package pkg;
+
+public aspect SrcAspect {
+	
+	pointcut p() : execution(* *.*(..)) && !within(pkg.*);
+	
+	before() : p() {
+	}
+	
+}

tests/src/org/aspectj/systemtest/ajc152/CreatingModelForInjarTests.java

@@ -84,7 +84,7 @@ public class CreatingModelForInjarTests extends org.aspectj.testing.XMLBasedAjcT
 	public void testAdviceInRelMap() {
 		runTest("advice and deow");
 		IHierarchy top = AsmManager.getDefault().getHierarchy();
-		IProgramElement adviceNode = top.findElementForLabel(top.getRoot(), IProgramElement.Kind.ADVICE,"before()");
+		IProgramElement adviceNode = top.findElementForLabel(top.getRoot(), IProgramElement.Kind.ADVICE,"before(): p..");
 		IRelationshipMap relMap = AsmManager.getDefault().getRelationshipMap();
 		List adviceRels = relMap.get(adviceNode);
 		assertFalse("expected before advice to have relationships but did not",adviceRels.isEmpty());
@@ -101,6 +101,47 @@ public class CreatingModelForInjarTests extends org.aspectj.testing.XMLBasedAjcT
 		assertFalse("expected declare warning to have relationships but did not",dwRels.isEmpty());
 	}
 	
+	public void testAdviceLabelsCorrect() {
+		runTest("ensure advice label is correct");
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		
+		IProgramElement node = top.findElementForLabel(top.getRoot(), 
+				IProgramElement.Kind.ADVICE, "before(): execM1()..");
+		assertNotNull("expected to find ipe with label 'before(): execM1()..'" +
+				" but didn't", node);
+		
+		node = top.findElementForLabel(top.getRoot(), 
+				IProgramElement.Kind.ADVICE, "before(): execM2()..");
+		assertNotNull("expected to find ipe with label 'before(): execM2()..'" +
+				" but didn't", node);
+		
+		node = top.findElementForLabel(top.getRoot(), 
+				IProgramElement.Kind.ADVICE, "before(): <anonymous pointcut>");
+		assertNotNull("expected to find ipe with label 'before(): <anonymous pointcut>'" +
+				" but didn't", node);
+	}
+	
+	// ensure that filled in hierarchy only has one entry for
+	// aspect
+	public void testOnlyOneAspectEntry() {
+		runTest("ensure advice label is correct");
+		
+		IProgramElement pkgNode = getPkgNode();
+		assertEquals("expected one child node but found " + 
+				pkgNode.getChildren().size(), 1, pkgNode.getChildren().size());
+		
+	}
+	
+	public void testOnlyOneAspectEntry_inDefaultPackage() {
+		runTest("aspect in default package");
+		// expect there to be two children - 'pack' and 
+		// 'AspectInDefaultPackage.aj (binary)'
+		IProgramElement defaultPkg = AsmManager.getDefault().getHierarchy().getRoot();
+		assertEquals("expected two child node but found " + 
+				defaultPkg.getChildren().size(), 2, defaultPkg.getChildren().size());
+		
+	}
+	
 	// --------------------- Helper methods ---------------------
 	
 	private IProgramElement getPkgNode() {

tests/src/org/aspectj/systemtest/ajc152/injar.xml

@@ -9,5 +9,12 @@
 	  </compile>
     </ajc-test>
 
+    <ajc-test dir="bugs152/pr145963" title="ensure advice label is correct">
+      <compile files="C.java" aspectpath="adviceLabels.jar" options="-emacssym"/>
+    </ajc-test>
     
+    <ajc-test dir="bugs152/pr145963" title="aspect in default package">
+      <compile files="C.java" aspectpath="aspectInDefaultPackage.jar" options="-emacssym"/>
+    </ajc-test>
+
 </suite>

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

@@ -176,6 +176,9 @@ public class AsmRelationshipProvider {
 				return;
 			}
 			
+			if (World.createInjarHierarchy) {
+				munger.createHierarchy();
+			}
 
 			IRelationshipMap mapper = AsmManager.getDefault().getRelationshipMap();
 			IProgramElement targetNode = getNode(AsmManager.getDefault().getHierarchy(), shadow);

weaver/src/org/aspectj/weaver/AsmRelationshipUtils.java

@@ -0,0 +1,84 @@
+/********************************************************************
+ * Copyright (c) 2006 Contributors. All rights reserved. 
+ * This program and the accompanying materials are made available 
+ * under the terms of the Eclipse Public License v1.0 
+ * which accompanies this distribution and is available at 
+ * http://eclipse.org/legal/epl-v10.html 
+ *  
+ * Contributors: IBM Corporation - initial API and implementation 
+ * 				 Helen Hawkins   - initial version
+ *******************************************************************/
+package org.aspectj.weaver;
+
+import org.aspectj.weaver.patterns.AndPointcut;
+import org.aspectj.weaver.patterns.OrPointcut;
+import org.aspectj.weaver.patterns.Pointcut;
+import org.aspectj.weaver.patterns.ReferencePointcut;
+
+/**
+ * Provides utility methods for generating details for IProgramElements
+ * used when creating the model both from source (via AsmElementFormatter.visit(..))
+ * and when filling in the model for binary aspects (via AsmRelationshipProvider
+ * bug 145963)
+ */
+public class AsmRelationshipUtils {
+
+	public static final String UNDEFINED="<undefined>";
+	public static final String DECLARE_PRECEDENCE = "precedence";
+	public static final String DECLARE_SOFT = "soft";
+	public static final String DECLARE_PARENTS = "parents";
+	public static final String DECLARE_WARNING = "warning";
+	public static final String DECLARE_ERROR = "error";
+	public static final String DECLARE_UNKNONWN = "<unknown declare>";
+	public static final String POINTCUT_ABSTRACT = "<abstract pointcut>";
+	public static final String POINTCUT_ANONYMOUS = "<anonymous pointcut>";
+	public static final String DOUBLE_DOTS = "..";
+	public static final int MAX_MESSAGE_LENGTH = 18;
+	public static final String DEC_LABEL = "declare";
+
+	/**
+	 * Generates the declare message used in the details, for example if
+	 * the declare warning statement has message "There should be no printlns" 
+	 * will return 'declare warning: "There should be n.."' 
+	 */
+	public static String genDeclareMessage(String message) {
+		int length = message.length();
+		if (length < MAX_MESSAGE_LENGTH) {
+			return message;
+		} else {
+			return message.substring(0, MAX_MESSAGE_LENGTH-1) + DOUBLE_DOTS;
+		}
+	}
+
+	/**
+	 * Generates the pointcut details for the given pointcut, for example
+	 * an anonymous pointcut will return '<anonymous pointcut>' and 
+	 * a named pointcut called p() will return 'p()..'
+	 */
+	public static String genPointcutDetails(Pointcut pcd) {
+		StringBuffer details = new StringBuffer();
+		if (pcd instanceof ReferencePointcut) {
+			ReferencePointcut rp = (ReferencePointcut)pcd;
+			details.append(rp.name).append(DOUBLE_DOTS);
+		} else if (pcd instanceof AndPointcut) {
+			AndPointcut ap = (AndPointcut)pcd;
+			if (ap.getLeft() instanceof ReferencePointcut) {
+				details.append(ap.getLeft().toString()).append(DOUBLE_DOTS);	
+			} else {
+				details.append(POINTCUT_ANONYMOUS).append(DOUBLE_DOTS);
+			}
+		} else if (pcd instanceof OrPointcut) {
+			OrPointcut op = (OrPointcut)pcd;
+			if (op.getLeft() instanceof ReferencePointcut) {
+				details.append(op.getLeft().toString()).append(DOUBLE_DOTS);	
+			} else {
+				details.append(POINTCUT_ANONYMOUS).append(DOUBLE_DOTS);
+			}
+		} else {
+			details.append(POINTCUT_ANONYMOUS);
+		}
+		return details.toString();
+	}
+	
+	
+}

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

@@ -98,9 +98,6 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH
 		if (null == handle) {
 			ISourceLocation sl = getSourceLocation();
 			if (sl != null) {
-				if (World.createInjarHierarchy) {
-					createHierarchy();
-				} 
 				IProgramElement ipe = AsmManager.getDefault().getHierarchy().findElementForSourceLine(sl);
 				handle = AsmManager.getDefault().getHandleProvider().createHandleIdentifier(ipe);
 			}
@@ -155,6 +152,9 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH
      */
     public abstract ResolvedType getResolvedDeclaringAspect();
     
+    /**
+     * Creates the hierarchy for binary aspects
+     */
     public void createHierarchy() {
     	IProgramElement sourceFileNode = AsmManager.getDefault().getHierarchy().findElementForSourceLine(getSourceLocation());
     	if (!sourceFileNode.getKind().equals(IProgramElement.Kind.FILE_JAVA)) {
@@ -162,6 +162,7 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH
 		}
     	String name = sourceFileNode.getName();
     	sourceFileNode.setName(name + " (binary)");
+    	AsmManager.getDefault().getHandleProvider().createHandleIdentifier(sourceFileNode);
     	
     	ResolvedType aspect = getResolvedDeclaringAspect();
     	
@@ -178,9 +179,29 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH
     	                IProgramElement.Kind.PACKAGE, 
     	                new ArrayList());
     			root.addChild(pkgNode);
+    			pkgNode.addChild(sourceFileNode);
+			} else {
+				for (Iterator iter = pkgNode.getChildren().iterator(); iter.hasNext();) {
+					IProgramElement element = (IProgramElement) iter.next();
+					if (element.getHandleIdentifier().equals(
+							sourceFileNode.getHandleIdentifier())) {
+						// already added the sourcefile so have already
+						// added the structure for this aspect
+						return;
+					}
+				}
+				pkgNode.addChild(sourceFileNode);
 			}
-    		pkgNode.addChild(sourceFileNode);
 		} else {
+			for (Iterator iter = root.getChildren().iterator(); iter.hasNext();) {
+				IProgramElement element = (IProgramElement) iter.next();
+				if (element.getHandleIdentifier().equals(
+						sourceFileNode.getHandleIdentifier())) {
+					// already added the sourcefile so have already
+					// added the structure for this aspect
+					return;
+				}				
+			}
 			root.addChild(sourceFileNode);
 		}
     	
@@ -240,26 +261,19 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH
 		    			getSourceLocation(),
 		    			this.getDeclaringType().getModifiers(),
 		    			null,null); 
-		    	deowNode.setDetails("\"" + genDeclareMessage(decl.getMessage()) + "\"");
+		    	deowNode.setDetails("\"" + AsmRelationshipUtils.genDeclareMessage(decl.getMessage()) + "\"");
 		    	parent.addChild(deowNode);
 			} else if (element instanceof BcelAdvice) {
 				BcelAdvice advice = (BcelAdvice)element;
-		    	parent.addChild(new ProgramElement(
-				advice.kind.getName(),
-				IProgramElement.Kind.ADVICE,
-				getSourceLocation(),
-				advice.signature.getModifiers(),null,Collections.EMPTY_LIST));
+				IProgramElement adviceNode = new ProgramElement(
+		    			advice.kind.getName(),
+		    			IProgramElement.Kind.ADVICE,
+		    			getSourceLocation(),
+		    			advice.signature.getModifiers(),null,Collections.EMPTY_LIST);
+		    	adviceNode.setDetails(AsmRelationshipUtils.genPointcutDetails(advice.getPointcut()));
+		    	parent.addChild(adviceNode);
 			}
 		}
     }
-    
-	// taken from AsmElementFormatter
-	private String genDeclareMessage(String message) {
-		int length = message.length();
-		if (length < 18) {
-			return message;
-		} else {
-			return message.substring(0, 17) + "..";
-		}
-	}
+
 }