test and fix for 145963: faulting in aspects coming in as binary inputs

16 files changed, 347 insertions, 9 deletions

diff --git a/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java b/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java
index 42c60a203..8224c6d9f 100644
--- a/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java
+++ b/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java
@@ -272,8 +272,8 @@ public class AspectJElementHierarchy implements IHierarchy {
 			lastSlash = sourceFilePath.lastIndexOf('/');
 		}
 		String fileName = sourceFilePath.substring(lastSlash+1);
-		IProgramElement fileNode = new ProgramElement(fileName, IProgramElement.Kind.FILE_JAVA, null);
-		fileNode.setSourceLocation(new SourceLocation(new File(sourceFilePath), 1, 1));
+		IProgramElement fileNode = new ProgramElement(fileName, IProgramElement.Kind.FILE_JAVA, new SourceLocation(new File(sourceFilePath), 1, 1),0,null,null);
+		//fileNode.setSourceLocation();
 		fileNode.addChild(NO_STRUCTURE);
 		return fileNode;
 	}
diff --git a/asm/src/org/aspectj/asm/internal/ProgramElement.java b/asm/src/org/aspectj/asm/internal/ProgramElement.java
index acddddfb2..af5a16960 100644
--- a/asm/src/org/aspectj/asm/internal/ProgramElement.java
+++ b/asm/src/org/aspectj/asm/internal/ProgramElement.java
@@ -162,8 +162,10 @@ public class ProgramElement implements IProgramElement {
 		return sourceLocation;
 	}
 
+	// not really sure why we have this setter ... how can we be in the situation where we didn't
+	// know the location when we built the node but we learned it later on?
 	public void setSourceLocation(ISourceLocation sourceLocation) {
-		//this.sourceLocation = sourceLocation;
+//		this.sourceLocation = sourceLocation;
 	}
 
 	public IMessage getMessage() {
diff --git a/tests/bugs152/pr145963/A.aj b/tests/bugs152/pr145963/A.aj
new file mode 100644
index 000000000..657c79b36
--- /dev/null
+++ b/tests/bugs152/pr145963/A.aj
@@ -0,0 +1,10 @@
+package pkg;
+
+public aspect A {
+	
+	pointcut p() : execution(* *.*(..)) && !within(pkg.*);
+	
+	before() : p() {
+	}
+	
+}
diff --git a/tests/bugs152/pr145963/ClassForAspectPath.java b/tests/bugs152/pr145963/ClassForAspectPath.java
new file mode 100644
index 000000000..0494c9c84
--- /dev/null
+++ b/tests/bugs152/pr145963/ClassForAspectPath.java
@@ -0,0 +1,12 @@
+public class ClassForAspectPath {
+
+	public static void main(String[] args) {
+		new ClassForAspectPath().method();
+
+	}
+
+	public void method() {
+		System.out.println("blah");
+	}
+	
+}
diff --git a/tests/bugs152/pr145963/Deow.aj b/tests/bugs152/pr145963/Deow.aj
new file mode 100644
index 000000000..bde434340
--- /dev/null
+++ b/tests/bugs152/pr145963/Deow.aj
@@ -0,0 +1,7 @@
+package pkg;
+
+public aspect Deow {
+
+	declare warning : (get(* System.out) || get(* System.err)) : "There should be no printlns";  
+
+}
diff --git a/tests/bugs152/pr145963/Itd.aj b/tests/bugs152/pr145963/Itd.aj
new file mode 100644
index 000000000..f53963c3b
--- /dev/null
+++ b/tests/bugs152/pr145963/Itd.aj
@@ -0,0 +1,7 @@
+package pkg;
+
+public aspect Itd {
+
+	declare parents : Point extends NewClass;
+	
+}
diff --git a/tests/bugs152/pr145963/NewClass.java b/tests/bugs152/pr145963/NewClass.java
new file mode 100644
index 000000000..187e321c6
--- /dev/null
+++ b/tests/bugs152/pr145963/NewClass.java
@@ -0,0 +1,5 @@
+package pkg;
+
+public class NewClass {
+
+}
diff --git a/tests/bugs152/pr145963/adviceAndDeow.jar b/tests/bugs152/pr145963/adviceAndDeow.jar
new file mode 100644
index 000000000..d9ab8c793
--- /dev/null
+++ b/tests/bugs152/pr145963/adviceAndDeow.jar
diff --git a/tests/src/org/aspectj/systemtest/ajc152/AllTestsAspectJ152.java b/tests/src/org/aspectj/systemtest/ajc152/AllTestsAspectJ152.java
index 05c311f50..1cf8167f7 100644
--- a/tests/src/org/aspectj/systemtest/ajc152/AllTestsAspectJ152.java
+++ b/tests/src/org/aspectj/systemtest/ajc152/AllTestsAspectJ152.java
@@ -19,6 +19,7 @@ public class AllTestsAspectJ152 {
 		TestSuite suite = new TestSuite("AspectJ 1.5.2 tests");
 		//$JUnit-BEGIN$
 		suite.addTest(Ajc152Tests.suite());
+		suite.addTest(CreatingModelForInjarTests.suite());
 		suite.addTest(SynchronizationTests.suite());
 		suite.addTest(SynchronizationTransformTests.suite());
         //$JUnit-END$
diff --git a/tests/src/org/aspectj/systemtest/ajc152/CreatingModelForInjarTests.java b/tests/src/org/aspectj/systemtest/ajc152/CreatingModelForInjarTests.java
new file mode 100644
index 000000000..213dd2bdc
--- /dev/null
+++ b/tests/src/org/aspectj/systemtest/ajc152/CreatingModelForInjarTests.java
@@ -0,0 +1,134 @@
+/********************************************************************
+ * 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.systemtest.ajc152;
+
+import java.io.File;
+import java.util.Iterator;
+import java.util.List;
+
+import junit.framework.Test;
+
+import org.aspectj.asm.AsmManager;
+import org.aspectj.asm.IHierarchy;
+import org.aspectj.asm.IProgramElement;
+import org.aspectj.testing.XMLBasedAjcTestCase;
+import org.aspectj.weaver.World;
+
+public class CreatingModelForInjarTests extends org.aspectj.testing.XMLBasedAjcTestCase {
+	
+	public void testAdviceAndNamedPCD() {
+		runTest("advice and deow");
+
+		// expect:
+		// 		- pkg {package}
+		// 			-  A.aj (binary) {java source file}
+		//				- import declarations {import reference}
+		//				- A {aspect}
+		//					- p {pointcut}
+		//					- before {advice}
+
+		IProgramElement pkgNode = getPkgNode();
+		IProgramElement srcFile = checkChild(pkgNode,IProgramElement.Kind.FILE_JAVA,"A.aj (binary)",1);
+		checkChild(srcFile,IProgramElement.Kind.IMPORT_REFERENCE,"import declarations",-1);
+		IProgramElement aspectNode = checkChild(srcFile,IProgramElement.Kind.ASPECT,"A",-1);
+		checkChild(aspectNode,IProgramElement.Kind.POINTCUT,"p",5);
+		checkChild(aspectNode,IProgramElement.Kind.ADVICE,"before",7);	
+	}
+
+	public void testDeclareWarning() {
+		runTest("advice and deow");		
+
+		// expect:
+		// 		- pkg {package}
+		// 			-  Deow.aj (binary) {java source file}
+		//				- import declarations {import reference}
+		//				- Deow {aspect}
+		//					- declare warning {declare warning}
+
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		IProgramElement dwNode = top.findElementForLabel(top.getRoot(),
+				IProgramElement.Kind.DECLARE_WARNING,
+				"declare warning: \"There should be n..\"");
+		assertNotNull("Couldn't find 'declare warning: \"There should be n..\"' " +
+				"element in the tree",dwNode);
+		assertEquals("expected 'declare warning: \"There should be n..\"'" + 
+				" to be on line 5 but was on " + dwNode.getSourceLocation().getLine(),
+				5, dwNode.getSourceLocation().getLine());			
+	}
+	
+	public void testNumberOfPackageNodes() {
+		runTest("advice and deow");
+		// check that the 'pkg' package node has not been added twice
+		IProgramElement root = AsmManager.getDefault().getHierarchy().getRoot();
+		List l = root.getChildren();
+		int numberOfPkgs = 0;
+		for (Iterator iter = l.iterator(); iter.hasNext();) {
+			IProgramElement element = (IProgramElement) iter.next();
+			if (element.getKind().equals(IProgramElement.Kind.PACKAGE)
+					&& element.getName().equals("pkg")) {
+				numberOfPkgs++;
+			}
+		}
+		assertEquals("expected one package called 'pkg' but found " + numberOfPkgs,1,numberOfPkgs);		
+	}
+	
+	private IProgramElement getPkgNode() {
+		IHierarchy top = AsmManager.getDefault().getHierarchy();
+		IProgramElement pkgNode = top.findElementForLabel(top.getRoot(),
+				IProgramElement.Kind.PACKAGE,"pkg");
+		assertNotNull("Couldn't find 'pkg' element in the tree",pkgNode);
+		return pkgNode;
+	}
+	
+	private IProgramElement checkChild(IProgramElement parent,
+			IProgramElement.Kind childKind,
+			String childName,
+			int childLineNumbr) {
+		List children = parent.getChildren();
+		boolean foundChild = false;
+		for (Iterator iter = children.iterator(); iter.hasNext();) {
+			IProgramElement element = (IProgramElement) iter.next();
+			if (element.getKind().equals(childKind) 
+					&& element.getName().equals(childName) ) {
+				foundChild = true;
+				if (childLineNumbr != -1) {
+					assertEquals("expected " + childKind.toString() + childName + 
+							" to be on line " + childLineNumbr + " but was on " + 
+							element.getSourceLocation().getLine(),
+							childLineNumbr, element.getSourceLocation().getLine());			
+				}
+				return element;
+			}
+		}	
+		assertTrue("expected " + parent.getName() + " to have child " + childName 
+				+ " but it did not", foundChild);
+		return null;
+	}
+	
+	protected void setUp() throws Exception {
+		super.setUp();
+		World.createInjarHierarchy = true;
+	}
+
+	protected void tearDown() throws Exception {
+		super.tearDown();
+        World.createInjarHierarchy = false;
+	}
+
+	// ///////////////////////////////////////
+	public static Test suite() {
+		return XMLBasedAjcTestCase.loadSuite(CreatingModelForInjarTests.class);
+	}
+
+	protected File getSpecFile() {
+		return new File("../tests/src/org/aspectj/systemtest/ajc152/injar.xml");
+	}
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc152/injar.xml b/tests/src/org/aspectj/systemtest/ajc152/injar.xml
new file mode 100644
index 000000000..955069a3e
--- /dev/null
+++ b/tests/src/org/aspectj/systemtest/ajc152/injar.xml
@@ -0,0 +1,13 @@
+<!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd"[]>
+
+<!-- AspectJ v1.5.2 Tests -->
+<suite>
+
+    <ajc-test dir="bugs152/pr145963" title="advice and deow">
+      <compile files="ClassForAspectPath.java" aspectpath="adviceAndDeow.jar" options="-emacssym">
+		<message kind="warning" line="9" text="There should be no printlns"/>
+	  </compile>
+    </ajc-test>
+
+    
+</suite>
diff --git a/weaver/src/org/aspectj/weaver/Advice.java b/weaver/src/org/aspectj/weaver/Advice.java
index 959ce3a18..737e0d087 100644
--- a/weaver/src/org/aspectj/weaver/Advice.java
+++ b/weaver/src/org/aspectj/weaver/Advice.java
@@ -428,5 +428,11 @@ public abstract class Advice extends ShadowMunger {
 	public ResolvedType getConcreteAspect() {
 		return concreteAspect;
 	}
+	
+	public ResolvedType getResolvedDeclaringAspect() {
+		// The aspect which declares this piece of advice 
+		// is 'concreteAspect' since 'declaringType' is null
+		return concreteAspect;
+	}
 
 }
diff --git a/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java b/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java
index 68398df13..7ea351930 100644
--- a/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java
+++ b/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java
@@ -61,7 +61,11 @@ public class AsmRelationshipProvider {
 			checker.getSourceLocation().getLine(),
 			checker.getSourceLocation().getColumn(),
 			checker.getSourceLocation().getOffset());
-			
+
+		if (World.createInjarHierarchy) {
+			checker.createHierarchy();
+		}
+
 		String targetHandle = AsmManager.getDefault().getHandleProvider().createHandleIdentifier(
 			shadow.getSourceLocation().getSourceFile(),
 			shadow.getSourceLocation().getLine(),
diff --git a/weaver/src/org/aspectj/weaver/Checker.java b/weaver/src/org/aspectj/weaver/Checker.java
index 6700b8b36..329716106 100644
--- a/weaver/src/org/aspectj/weaver/Checker.java
+++ b/weaver/src/org/aspectj/weaver/Checker.java
@@ -13,11 +13,17 @@
 
 package org.aspectj.weaver;
 
-import java.util.*;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
 
 import org.aspectj.asm.IRelationship;
-import org.aspectj.bridge.*;
-import org.aspectj.weaver.patterns.*;
+import org.aspectj.bridge.IMessage;
+import org.aspectj.bridge.ISourceLocation;
+import org.aspectj.bridge.Message;
+import org.aspectj.weaver.patterns.DeclareErrorOrWarning;
+import org.aspectj.weaver.patterns.PerClause;
+import org.aspectj.weaver.patterns.Pointcut;
 
 
 public class Checker extends ShadowMunger {
@@ -127,4 +133,8 @@ public class Checker extends ShadowMunger {
 		return isError;
 	}
 
+	public ResolvedType getResolvedDeclaringAspect() {
+		// The aspect which declares this deow is the declaring type
+		return getDeclaringType();
+	}
 }
diff --git a/weaver/src/org/aspectj/weaver/ShadowMunger.java b/weaver/src/org/aspectj/weaver/ShadowMunger.java
index 9646cfc20..6e045e83b 100644
--- a/weaver/src/org/aspectj/weaver/ShadowMunger.java
+++ b/weaver/src/org/aspectj/weaver/ShadowMunger.java
@@ -13,12 +13,19 @@
 
 package org.aspectj.weaver;
 
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
 import java.util.Map;
 
 import org.aspectj.asm.AsmManager;
+import org.aspectj.asm.IProgramElement;
+import org.aspectj.asm.internal.ProgramElement;
 import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.util.PartialOrder;
+import org.aspectj.weaver.bcel.BcelAdvice;
+import org.aspectj.weaver.patterns.DeclareErrorOrWarning;
 import org.aspectj.weaver.patterns.PerClause;
 import org.aspectj.weaver.patterns.Pointcut;
 
@@ -91,11 +98,14 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH
 		if (null == handle) {
 			ISourceLocation sl = getSourceLocation();
 			if (sl != null) {
+				if (World.createInjarHierarchy) {
+					createHierarchy();
+				} 
 				handle = AsmManager.getDefault().getHandleProvider().createHandleIdentifier(
 				            sl.getSourceFile(),
 				            sl.getLine(),
 				            sl.getColumn(),
-							sl.getOffset());
+							sl.getOffset());					
 			}
 		}
 		return handle;
@@ -141,4 +151,118 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH
      * @return true if munger has to check that its exceptions can be throwned based on the shadow
      */
     public abstract boolean mustCheckExceptions();
+    
+    /**
+     * Returns the ResolvedType corresponding to the aspect in which this
+     * shadowMunger is declared. This is different for deow's and advice.
+     */
+    public abstract ResolvedType getResolvedDeclaringAspect();
+    
+    public void createHierarchy() {
+    	IProgramElement sourceFileNode = AsmManager.getDefault().getHierarchy().findElementForSourceLine(getSourceLocation());
+    	if (!sourceFileNode.getKind().equals(IProgramElement.Kind.FILE_JAVA)) {
+			return;
+		}
+    	String name = sourceFileNode.getName();
+    	sourceFileNode.setName(name + " (binary)");
+    	
+    	ResolvedType aspect = getResolvedDeclaringAspect();
+    	
+    	// create package ipe if one exists....
+    	IProgramElement root = AsmManager.getDefault().getHierarchy().getRoot();
+    	if (aspect.getPackageName() != null) {
+    		// check that there doesn't already exist a node with this name
+    		IProgramElement pkgNode = AsmManager.getDefault().getHierarchy().findElementForLabel(
+    				root,IProgramElement.Kind.PACKAGE,aspect.getPackageName());
+    		// note packages themselves have no source location
+    		if (pkgNode == null) {
+    			pkgNode = new ProgramElement(
+    					aspect.getPackageName(), 
+    	                IProgramElement.Kind.PACKAGE, 
+    	                new ArrayList());
+    			root.addChild(pkgNode);
+			}
+    		pkgNode.addChild(sourceFileNode);
+		} else {
+			root.addChild(sourceFileNode);
+		}
+    	
+       	// remove the error child from the 'A.aj' node
+    	if (sourceFileNode instanceof ProgramElement) {
+			IProgramElement errorNode = (IProgramElement) sourceFileNode.getChildren().get(0);
+			if (errorNode.getKind().equals(IProgramElement.Kind.ERROR)) {
+				((ProgramElement)sourceFileNode).removeChild(errorNode);
+			}
+		}
+    	
+    	// add and create empty import declaration ipe
+    	sourceFileNode.addChild(new ProgramElement(
+    			"import declarations",
+    			IProgramElement.Kind.IMPORT_REFERENCE,
+    			null,0,null,null)); 
+
+    	// add and create aspect ipe
+    	IProgramElement aspectNode = new ProgramElement(
+    			aspect.getSimpleName(),
+    			IProgramElement.Kind.ASPECT,
+    			aspect.getSourceLocation(),
+    			aspect.getModifiers(),
+    			null,null); 
+    	sourceFileNode.addChild(aspectNode);
+    	
+    	addChildNodes(aspectNode,aspect.getDeclaredPointcuts());
+
+    	addChildNodes(aspectNode,aspect.getDeclaredAdvice());
+    	addChildNodes(aspectNode,aspect.getDeclares());
+    }
+    
+    private void addChildNodes(IProgramElement parent,ResolvedMember[] children) {
+       	for (int i = 0; i < children.length; i++) {
+			ResolvedMember pcd = children[i];
+			if (pcd instanceof ResolvedPointcutDefinition) {
+				ResolvedPointcutDefinition rpcd = (ResolvedPointcutDefinition)pcd;
+				parent.addChild(new ProgramElement(
+						pcd.getName(),
+						IProgramElement.Kind.POINTCUT,
+					    rpcd.getPointcut().getSourceLocation(),
+						pcd.getModifiers(),
+						null,
+						Collections.EMPTY_LIST));
+			} 
+		}
+    }
+    
+    private void addChildNodes(IProgramElement parent, Collection children) {
+    	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,
+		    			getSourceLocation(),
+		    			this.getDeclaringType().getModifiers(),
+		    			null,null); 
+		    	deowNode.setDetails("\"" + 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));
+			}
+		}
+    }
+    
+	// taken from AsmElementFormatter
+	private String genDeclareMessage(String message) {
+		int length = message.length();
+		if (length < 18) {
+			return message;
+		} else {
+			return message.substring(0, 17) + "..";
+		}
+	}
 }
diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java
index 5cb41d2b4..108fff263 100644
--- a/weaver/src/org/aspectj/weaver/World.java
+++ b/weaver/src/org/aspectj/weaver/World.java
@@ -59,7 +59,10 @@ public abstract class World implements Dump.INode {
     // to be independent of location before we can do that.
     /** Should we take into account source location when comparing mungers - which may trigger full builds */
     public final static boolean compareLocations = true;
-    
+  
+    // see pr145963
+    /** Should we create the hierarchy for binary classes and aspects*/
+    public static boolean createInjarHierarchy = true;
 
     /** Calculator for working out aspect precedence */
     private AspectPrecedenceCalculator precedenceCalculator;