fixing declare parents problems

14 files changed, 226 insertions, 58 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java
index c2bf0e41a..868499adc 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java
@@ -191,58 +191,30 @@ public class AjLookupEnvironment extends LookupEnvironment {
 	}
 
 	private void doDeclareParents(DeclareParents declareParents, SourceTypeBinding sourceType) {
-		if (declareParents.match(factory.fromEclipse(sourceType))) {
-			TypePatternList l = declareParents.getParents();
-			for (int i=0, len=l.size(); i < len; i++) {
-				addParent(declareParents, sourceType, l.get(i));
+		List newParents = declareParents.findMatchingNewParents(factory.fromEclipse(sourceType));
+		if (!newParents.isEmpty()) {
+			for (Iterator i = newParents.iterator(); i.hasNext(); ) {
+				ResolvedTypeX parent = (ResolvedTypeX)i.next();
+				addParent(sourceType, parent);
 			}
 		}
 	}
 
-	private void addParent(DeclareParents declareParents, SourceTypeBinding sourceType, TypePattern typePattern) {
-		if (typePattern == TypePattern.NO) return;  // already had an error here
-		TypeX iType = typePattern.getExactType();
-		ReferenceBinding b = (ReferenceBinding)factory.makeTypeBinding(iType); //"
-				
-		if (b.isClass()) {
-			if (sourceType.isInterface()) {
-				factory.showMessage(IMessage.ERROR, 
-					"interface can not extend a class", 
-					declareParents.getSourceLocation(), null
-				);
-				// how to handle xcutting errors???
-			}
-			
-			if (sourceType == b || sourceType.isSuperclassOf(b)) {
-				factory.showMessage(IMessage.ERROR,
-					"class can not extend itself", declareParents.getSourceLocation(), null
-				);
-				return;
-			}
-			sourceType.superclass = b;
+	private void addParent(SourceTypeBinding sourceType, ResolvedTypeX parent) {
+		ReferenceBinding parentBinding = (ReferenceBinding)factory.makeTypeBinding(parent); 
+		if (parentBinding.isClass()) {
+			sourceType.superclass = parentBinding;
 		} else {
-			//??? it's not considered an error to extend yourself, nothing happens
-			if (sourceType.equals(b)) {
-				return;
-			}
-			
-			if (sourceType.isInterface() && b.implementsInterface(sourceType, true)) {
-				factory.showMessage(IMessage.ERROR,
-					"interface can not extend itself", declareParents.getSourceLocation(), null
-				);
-				return;
-			}
-			if (sourceType == b || b.isSuperclassOf(sourceType)) return;
 			ReferenceBinding[] oldI = sourceType.superInterfaces;
 			ReferenceBinding[] newI;
 			if (oldI == null) {
 				newI = new ReferenceBinding[1];
-				newI[0] = b;
+				newI[0] = parentBinding;
 			} else {
 				int n = oldI.length;
 				newI = new ReferenceBinding[n+1];
 				System.arraycopy(oldI, 0, newI, 0, n);
-				newI[n] = b;
+				newI[n] = parentBinding;
 			}
 			sourceType.superInterfaces = newI;
 		}
diff --git a/org.aspectj.ajdt.core/testdata/src1/ParentsFail.java b/org.aspectj.ajdt.core/testdata/src1/ParentsFail.java
index 745fb67c9..6ed5f5680 100644
--- a/org.aspectj.ajdt.core/testdata/src1/ParentsFail.java
+++ b/org.aspectj.ajdt.core/testdata/src1/ParentsFail.java
@@ -18,5 +18,5 @@ aspect A {
 	declare parents: C2 implements I;  // CE can't implement
 	declare parents: C2 extends C3;  // CE circular
 	
-	declare parents: C1 extends C1; // CE self
+	declare parents: C1 extends C1; // not considered a CE, just does nothing
 }
\ No newline at end of file
diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/CompileAndRunTestCase.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/CompileAndRunTestCase.java
index 650b2a37c..2326c3126 100644
--- a/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/CompileAndRunTestCase.java
+++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/CompileAndRunTestCase.java
@@ -44,7 +44,7 @@ public class CompileAndRunTestCase extends CommandTestCase {
 	}
 	
 	public void testDeclareParentsFail() throws IOException {
-		CommandTestCase.checkCompile("src1/ParentsFail.java", new int[] {3, 11, 19, 21});
+		CommandTestCase.checkCompile("src1/ParentsFail.java", new int[] {3, 11, 19});
 	}
 	
 	public void testDeclareParents() throws IOException {
diff --git a/tests/ajcTests.xml b/tests/ajcTests.xml
index 97927ea16..6d4fd67b9 100644
--- a/tests/ajcTests.xml
+++ b/tests/ajcTests.xml
@@ -5781,5 +5781,27 @@
         <run class="b_impl.BImpl"/>
     </ajc-test>
 
+    <ajc-test dir="new/declareParents"
+      title="Declare parents with intermediate ancestor"
+      keywords="from-new">
+        <compile files="Driver.java"/>
+        <run class="Driver"/>
+    </ajc-test>
+    
+    <ajc-test dir="new/declareParents" 
+      title="Declare parents removing ancestor"
+      keywords="from-new">
+        <compile files="IllegalAdoption.java">
+            <message kind="error" line="13"/>
+        </compile>
+    </ajc-test>
+    
+
+    <ajc-test dir="options/injars/simple"  pr="35865"
+      title="options -injars checking declare parents interactions">
+        <compile files="DecParents.java,main.jar"
+        	options="!eclipse"/>
+        <run class="DecParents"/>
+    </ajc-test>
     
 </suite>
diff --git a/tests/ajcTestsFailing.xml b/tests/ajcTestsFailing.xml
index 27b67e5b0..1b2b82f8c 100644
--- a/tests/ajcTestsFailing.xml
+++ b/tests/ajcTestsFailing.xml
@@ -4,19 +4,6 @@
 <!-- contains valid tests that the compiler has never passed -->
 <suite>
 
-    <ajc-test dir="new/declareParents"
-      title="Declare parents with intermediate ancestor"
-      keywords="from-new">
-        <compile files="Driver.java"/>
-        <run class="Driver"/>
-    </ajc-test>
-    
-    <ajc-test dir="new/declareParents" 
-      title="Declare parents removing ancestor"
-      keywords="from-new">
-        <compile files="IllegalAdoption.java">
-            <message kind="error" line="13"/>
-        </compile>
-    </ajc-test>
+
 
 </suite>
diff --git a/tests/jimTests.xml b/tests/jimTests.xml
index 7664c3799..cc3257d9f 100644
--- a/tests/jimTests.xml
+++ b/tests/jimTests.xml
@@ -1,7 +1,6 @@
 <!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd">
 <suite>    
 
-
     <!--
     
     <ajc-test dir="new" pr="885"
diff --git a/tests/options/injars/simple/DecParents.java b/tests/options/injars/simple/DecParents.java
new file mode 100644
index 000000000..ecbf5f1e3
--- /dev/null
+++ b/tests/options/injars/simple/DecParents.java
@@ -0,0 +1,23 @@
+
+
+import org.aspectj.testing.Tester;
+
+public aspect DecParents {
+	private interface I {
+		public abstract String doit();
+	}
+	
+	public String I.doit() {
+		return "foo";
+	}
+	
+	declare parents: Main implements I;
+	
+	before(): execution(void Main.main(..)) {
+	}
+	
+    public static void main(String[] args) {
+    	I i = new Main();
+    	System.out.println("Main: " + i.doit());
+    }
+}
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/NewParentTypeMunger.java b/weaver/src/org/aspectj/weaver/NewParentTypeMunger.java
new file mode 100644
index 000000000..05f66ac63
--- /dev/null
+++ b/weaver/src/org/aspectj/weaver/NewParentTypeMunger.java
@@ -0,0 +1,39 @@
+/* *******************************************************************
+ * Copyright (c) 2002 Palo Alto Research Center, Incorporated (PARC).
+ * All rights reserved. 
+ * This program and the accompanying materials are made available 
+ * under the terms of the Common Public License v1.0 
+ * which accompanies this distribution and is available at 
+ * http://www.eclipse.org/legal/cpl-v10.html 
+ *  
+ * Contributors: 
+ *     PARC     initial implementation 
+ * ******************************************************************/
+
+
+package org.aspectj.weaver;
+
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.util.Set;
+
+public class NewParentTypeMunger extends ResolvedTypeMunger {
+	ResolvedTypeX newParent;
+	
+	public NewParentTypeMunger(ResolvedTypeX newParent) {
+		super(Parent, null);
+		this.newParent = newParent;
+	}
+
+	public void write(DataOutputStream s) throws IOException {
+		throw new RuntimeException("unimplemented");
+	}
+
+
+	public ResolvedTypeX getNewParent() {
+		return newParent;
+	}
+
+
+}
diff --git a/weaver/src/org/aspectj/weaver/ResolvedTypeMunger.java b/weaver/src/org/aspectj/weaver/ResolvedTypeMunger.java
index 8944e788f..086ade7b2 100644
--- a/weaver/src/org/aspectj/weaver/ResolvedTypeMunger.java
+++ b/weaver/src/org/aspectj/weaver/ResolvedTypeMunger.java
@@ -154,6 +154,8 @@ public abstract class ResolvedTypeMunger {
 	// not serialized, only created during concretization of aspects
 	public static final Kind PerObjectInterface = new Kind("PerObjectInterface", 3);
 	public static final Kind PrivilegedAccess = new Kind("PrivilegedAccess", 4);
+	
+	public static final Kind Parent = new Kind("Parent", 6);
 
 	public static final String SUPER_DISPATCH_NAME = "superDispatch";
 
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
index 5cd46b71d..864f127d6 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
@@ -246,6 +246,8 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName {
     	if (ret == null) {
     		//System.err.println("creating lazy class gen for: " + this);
     		ret = new LazyClassGen(this);
+    		//ret.print(System.err);
+    		//System.err.println("made LCG from : " + this.getJavaClass().getSuperclassName() );
     		if (isAspect()) {
     			lazyClassGen = ret;
     		}				
@@ -264,6 +266,22 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName {
 	public ISourceLocation getSourceLocation() {
 		return getResolvedTypeX().getSourceContext().makeSourceLocation(0); //FIXME, we can do better than this
 	}
+
+	public void addParent(ResolvedTypeX newParent) {
+		if (newParent.isClass()) {
+			superClass = newParent;
+		} else {
+			ResolvedTypeX[] oldInterfaceNames = getDeclaredInterfaces();
+			int len = oldInterfaceNames.length;
+			ResolvedTypeX[] newInterfaceNames = new ResolvedTypeX[len+1];
+			System.arraycopy(oldInterfaceNames, 0, newInterfaceNames, 0, len);
+			newInterfaceNames[len] = newParent;
+			
+			interfaces = newInterfaceNames;
+		}
+		//System.err.println("javaClass: " + Arrays.asList(javaClass.getInterfaceNames()) + " super " + javaClass.getSuperclassName());
+		//if (lazyClassGen != null) lazyClassGen.print();
+	}
 } 
     
     
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java b/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java
index 0e08a1b14..9c1b28a0c 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java
@@ -29,6 +29,7 @@ import org.aspectj.weaver.NameMangler;
 import org.aspectj.weaver.NewConstructorTypeMunger;
 import org.aspectj.weaver.NewFieldTypeMunger;
 import org.aspectj.weaver.NewMethodTypeMunger;
+import org.aspectj.weaver.NewParentTypeMunger;
 import org.aspectj.weaver.PerObjectInterfaceTypeMunger;
 import org.aspectj.weaver.PrivilegedAccessMunger;
 import org.aspectj.weaver.ResolvedMember;
@@ -60,13 +61,25 @@ public class BcelTypeMunger extends ConcreteTypeMunger {
 			return mungePrivilegedAccess(weaver, (PrivilegedAccessMunger)munger);
 		} else if (munger.getKind() == ResolvedTypeMunger.Constructor) {
 			return mungeNewConstructor(weaver, (NewConstructorTypeMunger)munger);
+		} else if (munger.getKind() == ResolvedTypeMunger.Parent) {
+			return mungeNewParent(weaver, (NewParentTypeMunger)munger);
 		} else {
 			throw new RuntimeException("unimplemented");
 		}
 	}
 
 
-	
+	private boolean mungeNewParent(BcelClassWeaver weaver, NewParentTypeMunger munger) {
+		LazyClassGen gen = weaver.getLazyClassGen();
+		ResolvedTypeX newParent = munger.getNewParent();
+		if (newParent.isClass()) {
+			//gen.setSuperClass(newParent);
+		} else {
+			gen.addInterface(newParent);
+		}
+		return true;
+	}
+
 	private boolean mungePrivilegedAccess(
 		BcelClassWeaver weaver,
 		PrivilegedAccessMunger munger)
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index bb6515a40..c3598ed49 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -22,6 +22,7 @@ import org.apache.bcel.classfile.JavaClass;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.util.FileUtil;
 import org.aspectj.weaver.*;
+import org.aspectj.weaver.patterns.DeclareParents;
 import org.aspectj.weaver.patterns.Pointcut;
 
 public class BcelWeaver implements IWeaver {
@@ -45,7 +46,8 @@ public class BcelWeaver implements IWeaver {
     private boolean needToReweaveWorld = false;
 
     private List shadowMungerList = null; // setup by prepareForWeave
-    private List typeMungerList = null; // setup by prepareForWeave 
+	private List typeMungerList = null; // setup by prepareForWeave 
+	private List declareParentsList = null; // setup by prepareForWeave 
 
     private ZipOutputStream zipOutputStream;
 
@@ -172,6 +174,7 @@ public class BcelWeaver implements IWeaver {
 
 		shadowMungerList = xcutSet.getShadowMungers();
 		typeMungerList = xcutSet.getTypeMungers();
+		declareParentsList = xcutSet.getDeclareParents();
     	
 		//XXX this gets us a stable (but completely meaningless) order
 		Collections.sort(
@@ -284,6 +287,32 @@ public class BcelWeaver implements IWeaver {
 
 	public void weave(ResolvedTypeX onType) {
 		onType.clearInterTypeMungers();
+		
+		// need to do any declare parents before the matching below
+		for (Iterator i = declareParentsList.iterator(); i.hasNext(); ) {
+			DeclareParents p = (DeclareParents)i.next();
+			List newParents = p.findMatchingNewParents(onType);
+			if (!newParents.isEmpty()) {
+				//???
+				BcelObjectType classType = BcelWorld.getBcelObjectType(world.resolve(onType.getClassName()));
+				//System.err.println("need to do declare parents for: " + onType);
+				for (Iterator j = newParents.iterator(); j.hasNext(); ) {
+					ResolvedTypeX newParent = (ResolvedTypeX)j.next();
+					if (newParent.isClass()) {
+						world.showMessage(IMessage.ERROR,
+							"can't use declare parents to change superclass of binary form \'" +
+							onType.getClassName() + "\' (implementation limitation)",
+							p.getSourceLocation(), null);
+						continue;
+					}
+					
+					classType.addParent(newParent);
+					ResolvedTypeMunger newParentMunger = new NewParentTypeMunger(newParent);
+					onType.addInterTypeMunger(new BcelTypeMunger(newParentMunger, null));
+				}
+			}
+		}
+		
 		for (Iterator i = typeMungerList.iterator(); i.hasNext(); ) {
 			ConcreteTypeMunger m = (ConcreteTypeMunger)i.next();
 			if (m.matches(onType)) {
@@ -292,6 +321,7 @@ public class BcelWeaver implements IWeaver {
 		}
 	}
 
+
 	// exposed for ClassLoader dynamic weaving
 	public LazyClassGen weaveWithoutDump(UnwovenClassFile classFile, BcelObjectType classType) throws IOException {
 		return weave(classFile, classType, false);
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
index 39f3427e9..aa5c157ec 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
@@ -177,6 +177,11 @@ public final class LazyClassGen {
     public void addInterface(TypeX typeX) {
     	myGen.addInterface(typeX.getName());
     }
+    
+	public void setSuperClass(TypeX typeX) {
+		myGen.setSuperclassName(typeX.getName());
+	 }
+
 
     // non-recursive, may be a bug, ha ha.
     private List getClassGens() {
diff --git a/weaver/src/org/aspectj/weaver/patterns/DeclareParents.java b/weaver/src/org/aspectj/weaver/patterns/DeclareParents.java
index d71633e7f..efc8bf69b 100644
--- a/weaver/src/org/aspectj/weaver/patterns/DeclareParents.java
+++ b/weaver/src/org/aspectj/weaver/patterns/DeclareParents.java
@@ -16,10 +16,15 @@ package org.aspectj.weaver.patterns;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
+import org.aspectj.bridge.IMessage;
 import org.aspectj.weaver.ISourceContext;
 import org.aspectj.weaver.ResolvedTypeX;
+import org.aspectj.weaver.TypeX;
+import org.aspectj.weaver.World;
 
 public class DeclareParents extends Declare {
 	private TypePattern child;
@@ -103,5 +108,58 @@ public class DeclareParents extends Declare {
 	public boolean isAdviceLike() {
 		return false;
 	}
+	
+	private ResolvedTypeX maybeGetNewParent(ResolvedTypeX targetType, TypePattern typePattern, World world) {
+		if (typePattern == TypePattern.NO) return null;  // already had an error here
+		TypeX iType = typePattern.getExactType();
+		ResolvedTypeX parentType = iType.resolve(world);
+			
+		if (parentType.isAssignableFrom(targetType)) return null;  // already a parent
+					
+		if (targetType.isAssignableFrom(parentType)) {
+			world.showMessage(IMessage.ERROR,
+				"type can not extend itself", this.getSourceLocation(), null
+			);
+			return null;
+		}
+					
+		if (parentType.isClass()) {
+			if (targetType.isInterface()) {
+				world.showMessage(IMessage.ERROR, 
+					"interface can not extend a class", 
+					this.getSourceLocation(), null
+				);
+				return null;
+				// how to handle xcutting errors???
+			}
+			
+			if (!targetType.getSuperclass().isAssignableFrom(parentType)) {
+				world.showMessage(IMessage.ERROR,
+									"can only insert a class into hierarchy, but "
+									+ iType.getName() + " is not a subtype of " +
+									targetType.getSuperclass().getName(), 
+									this.getSourceLocation(), null
+				);
+				return null;
+			} else {
+				return parentType;
+			}				
+		} else {
+			return parentType;
+		}
+	}
+	
+
+	public List/*<ResolvedTypeX>*/ findMatchingNewParents(ResolvedTypeX onType) {
+		if (!match(onType)) return Collections.EMPTY_LIST;
+		
+		List ret = new ArrayList();
+		for (int i=0; i < parents.size(); i++) {
+			ResolvedTypeX t = maybeGetNewParent(onType, parents.get(i), onType.getWorld());
+			if (t != null) ret.add(t);
+		}
+		
+		return ret;
+	}
 
 }