@override support for ITDs. Plus some binary weaving generic ITD tests.

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java

@@ -17,6 +17,7 @@ import java.lang.reflect.Modifier;
 
 import org.aspectj.ajdt.internal.compiler.lookup.EclipseFactory;
 import org.aspectj.ajdt.internal.compiler.lookup.EclipseTypeMunger;
+import org.aspectj.ajdt.internal.compiler.problem.AjProblemReporter;
 import org.aspectj.org.eclipse.jdt.internal.compiler.ClassFile;
 import org.aspectj.org.eclipse.jdt.internal.compiler.CompilationResult;
 import org.aspectj.org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
@@ -26,6 +27,8 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.flow.FlowInfo;
 import org.aspectj.org.eclipse.jdt.internal.compiler.flow.InitializationFlowContext;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ClassScope;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
+import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;
+import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.TagBits;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
 import org.aspectj.org.eclipse.jdt.internal.compiler.parser.Parser;
 import org.aspectj.org.eclipse.jdt.internal.compiler.problem.AbortCompilationUnit;
@@ -83,6 +86,8 @@ public class InterTypeMethodDeclaration extends InterTypeDeclaration {
 			
 		super.resolve(upperScope);
 	}
+	
+	
 	public void resolveStatements() {
         if ((modifiers & AccSemicolonBody) != 0) {
             if ((declaredModifiers & AccAbstract) == 0)
@@ -93,6 +98,45 @@ public class InterTypeMethodDeclaration extends InterTypeDeclaration {
                 scope.problemReporter().methodNeedingNoBody(this);
         }        
         
+        // check @Override annotation - based on MethodDeclaration.resolveStatements() @Override processing
+		checkOverride: {
+			if (this.binding == null) break checkOverride;
+			if (this.scope.compilerOptions().sourceLevel < JDK1_5) break checkOverride;
+			int bindingModifiers = this.binding.modifiers;
+			boolean hasOverrideAnnotation = (this.binding.tagBits & TagBits.AnnotationOverride) != 0;
+			
+			// Need to verify
+			if (hasOverrideAnnotation) {
+				
+				// Work out the real method binding that we can use for comparison
+				EclipseFactory world = EclipseFactory.fromScopeLookupEnvironment(scope);
+				MethodBinding realthing = world.makeMethodBinding(munger.getSignature());
+				
+				boolean reportError = true;				
+				// Go up the hierarchy, looking for something we override
+				ReferenceBinding supertype = onTypeBinding.superclass();
+				while (supertype!=null && reportError) {
+					MethodBinding[] possibles = supertype.getMethods(declaredSelector);
+					for (int i = 0; i < possibles.length; i++) {
+						MethodBinding mb = possibles[i];
+
+						boolean couldBeMatch = true;
+						if (mb.parameters.length!=realthing.parameters.length) couldBeMatch=false;
+						else {
+							for (int j = 0; j < mb.parameters.length && couldBeMatch; j++) {
+								if (!mb.parameters[j].equals(realthing.parameters[j])) couldBeMatch=false;
+							}
+						}
+						// return types compatible? (allow for covariance)
+						if (couldBeMatch && !returnType.resolvedType.isCompatibleWith(mb.returnType)) couldBeMatch=false;
+						if (couldBeMatch) reportError = false;
+					}
+					supertype = supertype.superclass(); // superclass of object is null
+				}
+				// If we couldn't find something we override, report the error
+				if (reportError) ((AjProblemReporter)this.scope.problemReporter()).itdMethodMustOverride(this,realthing);			
+			}
+		}
         
 		if (!Modifier.isAbstract(declaredModifiers)) super.resolveStatements();
 		if (Modifier.isStatic(declaredModifiers)) {

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/problem/AjProblemReporter.java

@@ -31,6 +31,7 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.ast.Annotation;
 import org.aspectj.org.eclipse.jdt.internal.compiler.ast.ExplicitConstructorCall;
 import org.aspectj.org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 import org.aspectj.org.eclipse.jdt.internal.compiler.impl.ReferenceContext;
+import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ArrayBinding;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding;
@@ -313,5 +314,63 @@ public class AjProblemReporter extends ProblemReporter {
     		methodDecl.sourceEnd,this.referenceContext, 
 			this.referenceContext == null ? null : this.referenceContext.compilationResult());
     }
+    
+    
+    /**
+     * Called when there is an ITD marked @override that doesn't override a supertypes method.
+     * The method and the binding are passed - some information is useful from each.  The 'method'
+     * knows about source offsets for the message, the 'binding' has the signature of what the
+     * ITD is trying to be in the target class.
+     */
+    public void itdMethodMustOverride(AbstractMethodDeclaration method,MethodBinding binding) {
+		this.handle(
+				IProblem.MethodMustOverride,
+				new String[] {new String(binding.selector), typesAsString(binding.isVarargs(), binding.parameters, false), new String(binding.declaringClass.readableName()), },
+				new String[] {new String(binding.selector), typesAsString(binding.isVarargs(), binding.parameters, true), new String(binding.declaringClass.shortReadableName()),},
+				method.sourceStart,
+				method.sourceEnd,
+				this.referenceContext, 
+				this.referenceContext == null ? null : this.referenceContext.compilationResult());
+	}
+    
+    /**
+     * Overrides the implementation in ProblemReporter and is ITD aware.
+     * To report a *real* problem with an ITD marked @override, the other methodMustOverride() method is used.
+     */
+    public void methodMustOverride(AbstractMethodDeclaration method) {
+    	MethodBinding binding = method.binding;
+    	
+    	// ignore ajc$ methods
+    	if (new String(method.selector).startsWith("ajc$")) return;
+    	
+    	ResolvedType onTypeX =  factory.fromEclipse(method.binding.declaringClass);
+    	for (Iterator i = onTypeX.getInterTypeMungersIncludingSupers().iterator(); i.hasNext(); ) {
+			ConcreteTypeMunger m = (ConcreteTypeMunger)i.next();
+			ResolvedMember sig = m.getSignature();
+			if (ResolvedType.matches(AjcMemberMaker.interMethod(sig,m.getAspectType(),
+						sig.getDeclaringType().resolve(factory.getWorld()).isInterface()),
+					    factory.makeResolvedMember(method.binding))) {
+				// match, so dont need to report a problem!
+				return;
+			}
+		}
+    	// report the error...
+    	super.methodMustOverride(method);
+    }
+    
+    
+    private String typesAsString(boolean isVarargs, TypeBinding[] types, boolean makeShort) {
+    	StringBuffer buffer = new StringBuffer(10);
+    	for (int i = 0, length = types.length; i < length; i++) {
+    		if (i != 0)
+    			buffer.append(", "); //$NON-NLS-1$
+    		TypeBinding type = types[i];
+    		boolean isVarargType = isVarargs && i == length-1;
+    		if (isVarargType) type = ((ArrayBinding)type).elementsType();
+    		buffer.append(new String(makeShort ? type.shortReadableName() : type.readableName()));
+    		if (isVarargType) buffer.append("..."); //$NON-NLS-1$
+    	}
+    	return buffer.toString();
+    }
 
 }

tests/java5/generics/itds/C.java

@@ -7,5 +7,7 @@ public abstract aspect C<T> {
   }
 }
 
+public aspect C<String> { }
+
 class Foo {
 }

tests/java5/generics/itds/D.java

@@ -8,4 +8,6 @@ public abstract aspect D<T> {
                 
 }
 
+aspect E extends D<String> {}
+
 class Goo<P> {}

tests/java5/generics/itds/FieldITDOnGeneric.aj → tests/java5/generics/itds/FieldITDOnGenericType.aj

@@ -12,7 +12,7 @@ public class FieldITDOnGenericType {
   public static void main(String[] argv) {
     MathUtils<Integer> mu = new MathUtils<Integer>();
     mu.n=42;
-    System.err.prinltn(">"+mu.n);
+    System.err.println(">"+mu.n);
   }
 }
 

tests/java5/generics/itds/binaryweaving/TestA_generictype.java

@@ -1,4 +1,3 @@
 import java.util.*;
 
-class TestA_generictype<N> {// extends Number> { 
-}
+class TestA_generictype<N extends Number> { }

tests/java5/generics/itds/binaryweaving/TestB_aspect1.aj

@@ -0,0 +1,5 @@
+import java.util.*;
+
+aspect TestB_aspect1 {
+  public <L extends Number> void TestB_generictype<X,Y>.mxy(List<L> ll1, List<X> lx,List<Y> ly,List<L> ll2) {} 
+}

tests/java5/generics/itds/binaryweaving/TestB_aspect2.aj

@@ -0,0 +1,5 @@
+import java.util.*;
+
+aspect TestB_aspect2 {
+  public <L extends Number> void TestB_generictype<X,Y>.myx(List<L> ll1, List<Y> ly,List<X> lx,List<L> ll2) {} 
+}

tests/java5/generics/itds/binaryweaving/TestB_class.java

@@ -0,0 +1,15 @@
+import java.util.*;
+
+public class TestB_class {
+  public static void main(String []argv) { 
+    TestB_generictype<Float,String>  sc1=new TestB_generictype<Float,String>();
+    TestB_generictype<Integer,Float> sc2=new TestB_generictype<Integer,Float>();
+
+    List<Integer> li = new ArrayList<Integer>();
+    List<String>  ls = new ArrayList<String>();
+    List<Float>   lf = new ArrayList<Float>();
+
+    sc1.mxy(li,lf,ls,li);
+    sc2.myx(li,lf,li,li);
+  }
+}

tests/java5/generics/itds/binaryweaving/TestB_generictype.java

@@ -0,0 +1,3 @@
+import java.util.*;
+
+class TestB_generictype<N extends Number,P> { }

tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java

@@ -161,12 +161,13 @@ public class GenericsTests extends XMLBasedAjcTestCase {
 	 * PASS recursive type variable definitions
 	 * PASS generic aspects
 	 * PASS parameterizing ITDs with type variables
-     * TODO using type variables from the target type in your *STATIC* ITD (field/method/ctor) (error scenario)
+     * PASS using type variables from the target type in your *STATIC* ITD (field/method/ctor) (error scenario)
+     * PASS basic binary weaving of generic itds
+	 * TODO generic aspect binary weaving (or at least multi source file weaving)
 	 * TODO binary weaving with changing types (moving between generic and simple)
 	 * TODO bridge method creation (also relates to covariance overrides..)
 	 * TODO exotic class/interface bounds ('? extends List<String>','? super anything')
 	 * TODO signature attributes for generic ITDs (public only?)
-	 * TODO generic aspect binary weaving (or at least multi source file weaving)
 	 * 
 	 * 
 	 * strangeness:
@@ -218,8 +219,9 @@ public class GenericsTests extends XMLBasedAjcTestCase {
 	public void testPR96220_GenericAspects1() {runTest("generic aspects - 1");}
 	public void testPR96220_GenericAspects2() {runTest("generic aspects - 2");}
 	public void testPR96220_GenericAspects3() {runTest("generic aspects - 3");}
-//	public void testGenericAspects4()         {runTest("generic aspects - 4");}
-//	public void testGenericAspects5()         {runTest("generic aspects - 5 (ajdk)");}
+	public void testGenericAspects4()         {runTest("generic aspects - 4");} 
+    // TODO FREAKYGENERICASPECTPROBLEM why does everything have to be in one source file?
+	// public void testGenericAspects5()         {runTest("generic aspects - 5 (ajdk)");} 
 	public void testTypeVariablesInDeclareWarning() { runTest("generic aspect with declare warning using type vars");}
 	public void testTypeVariablesInExecutionAdvice() { runTest("generic aspect with execution advice using type vars");}
 	public void testTypeVariablesInAnonymousPointcut() { runTest("generic aspect with anonymous pointcut");}
@@ -367,13 +369,18 @@ public class GenericsTests extends XMLBasedAjcTestCase {
 	public void testSophisticatedAspectsT() {runTest("uberaspects - T");}
 	public void testSophisticatedAspectsU() {runTest("uberaspects - U");} //  includes nasty casts
 	
-	public void testBinaryWeavingITDsA() {runTest("binary weaving ITDs - A");}
+	// FIXME asc these two tests have peculiar error messages - generic aspect related
+//	public void testItdUsingTypeParameter() {runTest("itd using type parameter");}
+//	public void testItdIncorrectlyUsingTypeParameter() {runTest("itd incorrectly using type parameter");}
 	
+
+	public void testBinaryWeavingITDsA() {runTest("binary weaving ITDs - A");}
+	public void testBinaryWeavingITDsB() {runTest("binary weaving ITDs - B");}
+	public void testBinaryWeavingITDs1() {runTest("binary weaving ITDs - 1");}
 	// ?? Looks like reweavable files dont process their type mungers correctly.
 	// See AjLookupEnvironment.weaveInterTypeDeclarations(SourceTypeBinding,typeMungers,declareparents,...)
 	//   it seems to process any it discovers from the weaver state info then not apply new ones (the ones
 	//   passed in!)
-//	public void testBinaryWeavingITDs1() {runTest("binary weaving ITDs - 1");}
 //	public void testBinaryWeavingITDs2() {runTest("binary weaving ITDs - 2");}
 //	public void testBinaryWeavingITDs3() {runTest("binary weaving ITDs - 3");}
 	
@@ -386,22 +393,15 @@ public class GenericsTests extends XMLBasedAjcTestCase {
 	public void testStaticGenericMethodITD()       {runTest("static generic method itd");}
 	
 	
-
-//	public void testGenericITFSharingTypeVariable() {
-//		runTest("generic intertype field declaration, sharing type variable");
-//	}
-
-//	public void testItdOnGenericType() {
-//		runTest("ITDs on generic type");
-//	}
-//	
-//	public void testItdUsingTypeParameter() {
-//		runTest("itd using type parameter");
-//	}
-//	
-//	public void testItdIncorrectlyUsingTypeParameter() {
-//		runTest("itd incorrectly using type parameter");
-//	}
+	public void testAtOverride0()  {runTest("atOverride used with ITDs");}
+	public void testAtOverride1()  {runTest("atOverride used with ITDs - 1");}
+	public void testAtOverride2()  {runTest("atOverride used with ITDs - 2");}
+	public void testAtOverride3()  {runTest("atOverride used with ITDs - 3");}
+	public void testAtOverride4()  {runTest("atOverride used with ITDs - 4");}
+	public void testAtOverride5()  {runTest("atOverride used with ITDs - 5");}
+	public void testAtOverride6()  {runTest("atOverride used with ITDs - 6");}
+	
+	public void testGenericITFSharingTypeVariable() {runTest("generic intertype field declaration, sharing type variable");}
 
 	// ----------------------------------------------------------------------------------------
 	// generic declare parents tests

tests/src/org/aspectj/systemtest/ajc150/ajc150.xml

@@ -311,7 +311,37 @@
             <message kind="error" line="5" text="Syntax error on tokens, valid member declaration expected instead"/>
         </compile>
     </ajc-test>
+    
+    <!-- atOverride tests with ITDs -->
+    
+    <ajc-test dir="java5/generics/itds" pr="106630" title="atOverride used with ITDs">
+      <compile files="AtOverride.aj" options="-1.5"/>
+    </ajc-test>
+    <ajc-test dir="java5/generics/itds" pr="106630" title="atOverride used with ITDs - 1">
+      <compile files="AtOverride1.aj" options="-1.5">
+        <message kind="error" line="9" text="The method method() of type Child must override a superclass method"/>
+      </compile>
+    </ajc-test>
+    <ajc-test dir="java5/generics/itds" pr="106630" title="atOverride used with ITDs - 2">
+      <compile files="AtOverride2.aj" options="-1.5"/>
+    </ajc-test>
+    <ajc-test dir="java5/generics/itds" pr="106630" title="atOverride used with ITDs - 3">
+      <compile files="AtOverride3.aj" options="-1.5"/>
+    </ajc-test>
+    <ajc-test dir="java5/generics/itds" pr="106630" title="atOverride used with ITDs - 4">
+      <compile files="AtOverride4.aj" options="-1.5"/>
+    </ajc-test>
+    <ajc-test dir="java5/generics/itds" pr="106630" title="atOverride used with ITDs - 5">
+      <compile files="AtOverride5.aj" options="-1.5">
+        <message kind="error" line="11" text="The method method() of type Child must override a superclass method"/>
+      </compile>
+    </ajc-test>
+    <ajc-test dir="java5/generics/itds" pr="106630" title="atOverride used with ITDs - 6">
+      <compile files="AtOverride6.aj" options="-1.5"/>
+    </ajc-test>
                
+    <!-- end of atOverride tests with ITDs -->
+
     <ajc-test dir="../docs/dist/doc/examples/introduction" title="introduction sample" vm="1.5">
         <compile files="CloneablePoint.java,ComparablePoint.java,HashablePoint.java,Point.java" options="-1.5"/>
     </ajc-test>
@@ -3189,10 +3219,10 @@
    </ajc-test>
    
    <ajc-test dir="java5/generics/itds" title="generic intertype field declaration, sharing type variable">
-     <compile files="FieldITDOnGeneric.aj" options="-1.5"/>
-     <run class="FieldITDOnGeneric">
+     <compile files="FieldITDOnGenericType.aj" options="-1.5"/>
+     <run class="FieldITDOnGenericType">
        <stderr>
-         <line text="foo"/>
+         <line text=">42"/>
        </stderr>
      </run>
    </ajc-test>
@@ -3310,7 +3340,13 @@
    	   <compile files="TestA_generictype.java" outjar="code.jar" options="-1.5,-Xreweavable"/>
        <compile files="TestA_aspect.aj,TestA_class.java" inpath="code.jar" options="-1.5,-Xreweavable"/>
        <run class="TestA_class"/>
-    </ajc-test>
+   </ajc-test>
+
+   <ajc-test dir="java5/generics/itds/binaryweaving" vm="1.5" title="binary weaving ITDs - B">
+   	   <compile files="TestB_generictype.java" outjar="code.jar" options="-1.5,-Xreweavable"/>
+       <compile files="TestB_aspect1.aj,TestB_aspect2.aj,TestB_class.java" inpath="code.jar" options="-1.5,-Xreweavable"/>
+       <run class="TestB_class"/>
+   </ajc-test>
     
    <ajc-test dir="java5/generics/itds/binaryweaving" vm="1.5" title="binary weaving ITDs - 1">
    	   <compile files="BaseClass.java" outjar="code.jar" options="-1.5,-Xreweavable"/>

weaver/src/org/aspectj/weaver/ResolvedType.java

@@ -1122,7 +1122,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 		
 		//System.err.println("add: " + munger + " to " + this.getClassName() + " with " + interTypeMungers);
 		if (sig.getKind() == Member.METHOD) {
-			if (!compareToExistingMembers(munger, getMethods())) return;
+			if (!compareToExistingMembers(munger, getMethodsWithoutIterator(false) /*getMethods()*/)) return;
 			if (this.isInterface()) {
 				if (!compareToExistingMembers(munger, 
 						Arrays.asList(world.getCoreType(OBJECT).getDeclaredMethods()).iterator())) return;
@@ -1169,6 +1169,9 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 		interTypeMungers.add(munger);
 	}
 	
+	private boolean compareToExistingMembers(ConcreteTypeMunger munger, List existingMembersList) {
+		return compareToExistingMembers(munger,existingMembersList.iterator());
+	}
 	
 	//??? returning too soon
 	private boolean compareToExistingMembers(ConcreteTypeMunger munger, Iterator existingMembers) {
@@ -1194,12 +1197,16 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 						//??? might need list of these overridden abstracts
 						continue;
 					} else {
-						//XXX dual errors possible if (this instanceof BcelObjectType) return false;  //XXX ignores separate comp
-						getWorld().getMessageHandler().handleMessage(
+					  // bridge methods can differ solely in return type.
+					  // FIXME this whole method seems very hokey - unaware of covariance/varargs/bridging - it
+					  // could do with a rewrite !
+					  boolean sameReturnTypes = (existingMember.getReturnType().equals(sig.getReturnType()));
+					  if (sameReturnTypes)
+						  getWorld().getMessageHandler().handleMessage(
 							MessageUtil.error(WeaverMessages.format(WeaverMessages.ITD_MEMBER_CONFLICT,munger.getAspectType().getName(),
 									existingMember),
 							munger.getSourceLocation())
-						);
+						  );
 					}
 				} else if (isDuplicateMemberWithinTargetType(existingMember,this,sig)) {
 				    	getWorld().getMessageHandler().handleMessage(
@@ -1245,7 +1252,18 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 			return false;
 		}
 		
-		if (!parent.getReturnType().equals(child.getReturnType())) {
+		boolean incompatibleReturnTypes = false;
+		
+		// In 1.5 mode, allow for covariance on return type
+		if (world.isInJava5Mode() && parent.getKind()==Member.METHOD) {
+	      ResolvedType rtParentReturnType = parent.getReturnType().resolve(world);
+		  ResolvedType rtChildReturnType  = child.getReturnType().resolve(world);
+		  incompatibleReturnTypes = !rtParentReturnType.isAssignableFrom(rtChildReturnType);
+		} else {
+		  incompatibleReturnTypes =!parent.getReturnType().equals(child.getReturnType());
+		}
+		
+		if (incompatibleReturnTypes) {
 			world.showMessage(IMessage.ERROR,
 					WeaverMessages.format(WeaverMessages.ITD_RETURN_TYPE_MISMATCH,parent,child),
 					child.getSourceLocation(), parent.getSourceLocation());