testcases and fix for binary weaving ITDs!!!!!!!!!!! (pr85132, etc,etc)

9 files changed, 78 insertions, 24 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 cb6f5e4ba..e8b7bbc17 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
@@ -55,8 +55,8 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.problem.ProblemReporter;
 import org.aspectj.weaver.AsmRelationshipProvider;
 import org.aspectj.weaver.ConcreteTypeMunger;
 import org.aspectj.weaver.ReferenceType;
-import org.aspectj.weaver.ResolvedTypeMunger;
 import org.aspectj.weaver.ResolvedType;
+import org.aspectj.weaver.ResolvedTypeMunger;
 import org.aspectj.weaver.UnresolvedType;
 import org.aspectj.weaver.WeaverMessages;
 import org.aspectj.weaver.WeaverStateInfo;
@@ -439,7 +439,7 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC
 		ResolvedType onType = factory.fromEclipse(sourceType);
 		// AMC we shouldn't need this when generic sigs are fixed??
 		if (onType.isRawType()) onType = onType.getGenericType();
-		
+
 		WeaverStateInfo info = onType.getWeaverState();
 		
 		// this test isnt quite right - there will be a case where we fail to flag a problem
@@ -453,7 +453,7 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC
 			CompilationAndWeavingContext.leavingPhase(tok);
 			return;
 		}
-		
+
 		// Check if the type we are looking at is the topMostImplementor of a dangerous interface - 
 		// report a problem if it is.
 		for (Iterator i = dangerousInterfaces.entrySet().iterator(); i.hasNext();) {
@@ -587,9 +587,9 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC
 				weaveInterTypeDeclarations((SourceTypeBinding) memberTypes[i], typeMungers, declareParents,declareAnnotationOnTypes, false);
 			}
 		}
-		CompilationAndWeavingContext.leavingPhase(tok);
-	}
-
+			CompilationAndWeavingContext.leavingPhase(tok);
+		}
+		
 	/**
 	 * Called when we discover we are weaving intertype declarations on some type that has
 	 * an existing 'WeaverStateInfo' object - this is typically some previously woven type
@@ -855,6 +855,8 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC
 				packageBinding,
 				needFieldsAndMethods,
 				accessRestriction);
+			factory.getWorld().validateType(factory.fromBinding(ret));
+			// if you need the bytes to pass to validate, here they are:ClassFileReader)binaryType).getReferenceBytes()
 			weaveInterTypeDeclarations(ret);			
 			return ret;
 		} finally {
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java
index 91174eef5..92a78e408 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java
@@ -497,4 +497,8 @@ public class EclipseSourceType extends AbstractReferenceTypeDelegate {
 		tv.setRank(aTypeParameter.binding.rank);
 		return tv;
 	}
+
+	public void ensureDelegateConsistent() {
+		// do nothing, currently these can't become inconsistent (phew)
+	}
 }
diff --git a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
index 2daba1097..6e99c2916 100644
--- a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
+++ b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
@@ -273,27 +273,25 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed {
 		checkWasntFullBuild();
 	}
 	
+	
+	public void testPr85132() {
+		initialiseProject("PR85132");
+		build("PR85132");
+		alter("PR85132","inc1");
+		build("PR85132");
+	}
+	
+	public void testPr92837() {
+		initialiseProject("PR92837");
+		build("PR92837");
+		alter("PR92837","inc1");
+		build("PR92837");
+	}
+	
 	// other possible tests:
 	// - memory usage (freemem calls?)
 	// - relationship map
 
-	
-//	public void testPr85132() {
-//		super.VERBOSE=true;
-//		initialiseProject("PR85132");
-//		build("PR85132");
-//		alter("PR85132","inc1");
-//		build("PR85132");
-//	}
-//	
-//	public void testPr92837() {
-//		super.VERBOSE=true;
-//		initialiseProject("PR92837");
-//		build("PR92837");
-//		alter("PR92837","inc1");
-//		build("PR92837");
-//	}
-	
 	// ---------------------------------------------------------------------------------------------------
 
 	/**
diff --git a/weaver/src/org/aspectj/weaver/BoundedReferenceType.java b/weaver/src/org/aspectj/weaver/BoundedReferenceType.java
index 0497fccf2..51adc3885 100644
--- a/weaver/src/org/aspectj/weaver/BoundedReferenceType.java
+++ b/weaver/src/org/aspectj/weaver/BoundedReferenceType.java
@@ -245,5 +245,9 @@ public class BoundedReferenceType extends ReferenceType {
 			return resolvedTypeX.getTypeVariables();
 		}
 
+		public void ensureDelegateConsistent() {
+			resolvedTypeX.getDelegate().ensureDelegateConsistent();
+		}
+
 	}
 }
diff --git a/weaver/src/org/aspectj/weaver/ReferenceTypeDelegate.java b/weaver/src/org/aspectj/weaver/ReferenceTypeDelegate.java
index e6523b0c5..7bd63ddf6 100644
--- a/weaver/src/org/aspectj/weaver/ReferenceTypeDelegate.java
+++ b/weaver/src/org/aspectj/weaver/ReferenceTypeDelegate.java
@@ -24,7 +24,10 @@ public interface ReferenceTypeDelegate {
 	
 	// TODO asc move to proxy
 	public void addAnnotation(AnnotationX annotationX);
+	public void ensureDelegateConsistent(); // Required evil because of mutator methods in delegates :(  (see pr85132)
 
+	
+	
 	public boolean isAspect();
     public boolean isAnnotationStyleAspect();
     public boolean isInterface();
diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java
index 4780b58ae..1d42e44d2 100644
--- a/weaver/src/org/aspectj/weaver/World.java
+++ b/weaver/src/org/aspectj/weaver/World.java
@@ -807,4 +807,7 @@ public abstract class World implements Dump.INode {
 			}
 		}
 	}
+
+	public void validateType(UnresolvedType type) { }
+	
 }
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
index 7c2b16dda..c6818b2bf 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
@@ -85,6 +85,13 @@ public class BcelObjectType extends AbstractReferenceTypeDelegate {
 	private boolean haveLookedForDeclaredSignature = false;
 	private String declaredSignature = null;
 	private boolean isGenericType = false;
+	
+	/**
+	 * A BcelObjectType is 'damaged' if it has been modified from what was original constructed from
+	 * the bytecode.  This currently happens if the parents are modified or an annotation is added -
+	 * ideally BcelObjectType should be immutable but that's a bigger piece of work!!!!!!!!!! XXX
+	 */
+	private boolean damaged = false;
 
 	public Collection getTypeMungers() {
 		return typeMungers;	
@@ -223,6 +230,7 @@ public class BcelObjectType extends AbstractReferenceTypeDelegate {
 
 	//??? method only used for testing
 	public void addPointcutDefinition(ResolvedPointcutDefinition d) {
+		damaged = true;
 		int len = pointcuts.length;
 		ResolvedPointcutDefinition[] ret = new ResolvedPointcutDefinition[len+1];
 		System.arraycopy(pointcuts, 0, ret, 0, len);
@@ -305,6 +313,10 @@ public class BcelObjectType extends AbstractReferenceTypeDelegate {
         return javaClass;
     }
     
+    public void ensureDelegateConsistent() {
+    	if (damaged) {resetState();damaged=false;}
+    }
+    
     public void resetState() {
 		this.interfaces = null;
     	this.superClass = null;
@@ -387,7 +399,7 @@ public class BcelObjectType extends AbstractReferenceTypeDelegate {
 	}
 	
 	public void addAnnotation(AnnotationX annotation) {
-		
+		damaged = true;
 		// Add it to the set of annotations
 		int len = annotations.length;
 		AnnotationX[] ret = new AnnotationX[len+1];
@@ -438,6 +450,7 @@ public class BcelObjectType extends AbstractReferenceTypeDelegate {
 	}
 
 	public void addParent(ResolvedType newParent) {
+		damaged = true;
 		if (newParent.isClass()) {
 			superClass = newParent;
 		} else {
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
index d547fa33c..cdaead48c 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
@@ -14,6 +14,7 @@
 
 package org.aspectj.weaver.bcel;
 
+import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Modifier;
@@ -499,4 +500,26 @@ public class BcelWorld extends World implements Repository {
 		throw new RuntimeException("Not implemented");
 	}
 
+    // @Override
+    /** 
+     * The aim of this method is to make sure a particular type is 'ok'.  Some
+     * operations on the delegate for a type modify it and this method is
+     * intended to undo that...  see pr85132
+     */
+	public void validateType(UnresolvedType type) {
+		ResolvedType result = typeMap.get(type.getSignature());
+		if (result==null) return; // We haven't heard of it yet
+		if (!result.isExposedToWeaver()) return; // cant need resetting
+		ReferenceType rt = (ReferenceType)result;
+		rt.getDelegate().ensureDelegateConsistent();
+		// If we want to rebuild it 'from scratch' then:
+//		ClassParser cp = new ClassParser(new ByteArrayInputStream(newbytes),new String(cs));
+//		try {
+//			rt.setDelegate(makeBcelObjectType(rt,cp.parse(),true));
+//		} catch (ClassFormatException e) {
+//			e.printStackTrace();
+//		} catch (IOException e) {
+//			e.printStackTrace();
+//		}
+	}
 }
diff --git a/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java b/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
index e3fec7dc0..69ae60a8e 100644
--- a/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
+++ b/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
@@ -310,4 +310,8 @@ public class ReflectionBasedReferenceTypeDelegate implements ReferenceTypeDelega
 		return null;
 	}
 
+	public void ensureDelegateConsistent() {
+		// Nothing to do - a reflection based delegate can't become inconsistent...
+	}
+
 }