fix for pr87530, final modifier on interface itdm

9 files changed, 150 insertions, 23 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java
index a56f2a161..666335db7 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java
@@ -63,6 +63,10 @@ public class InterTypeMethodDeclaration extends InterTypeDeclaration {
 	protected char[] getPrefix() {
 		return (NameMangler.ITD_PREFIX + "interMethod$").toCharArray();
 	}
+	
+	public boolean isFinal() {
+		return  (declaredModifiers & AccFinal) != 0;		
+	}
 
 	public void analyseCode(
 		ClassScope currentScope,
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 4bd5cd7be..32dd4c188 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
@@ -384,7 +384,7 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC
 			for (Iterator i = mungers.iterator(); i.hasNext(); ) {
 				ConcreteTypeMunger m = (ConcreteTypeMunger)i.next();
 				EclipseTypeMunger munger = factory.makeEclipseTypeMunger(m);
-				if (munger.munge(sourceType)) {
+				if (munger.munge(sourceType,onType)) {
 					if (onType.isInterface() &&
 						munger.getMunger().needsAccessToTopmostImplementor())
 					{
@@ -509,7 +509,7 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC
 		for (Iterator i = onType.getInterTypeMungers().iterator(); i.hasNext();) {
 			EclipseTypeMunger munger = (EclipseTypeMunger) i.next();
 			//System.out.println("applying: " + munger + " to " + new String(sourceType.sourceName));
-			munger.munge(sourceType);
+			munger.munge(sourceType,onType);
 		}
 		
 		// Call if you would like to do source weaving of declare @method/@constructor 
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseTypeMunger.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseTypeMunger.java
index 038d92d22..e499f2df5 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseTypeMunger.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseTypeMunger.java
@@ -17,6 +17,7 @@ import java.lang.reflect.Modifier;
 
 import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration;
+import org.aspectj.org.eclipse.jdt.internal.compiler.env.IConstants;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding;
@@ -24,6 +25,7 @@ import org.aspectj.weaver.ConcreteTypeMunger;
 import org.aspectj.weaver.NewConstructorTypeMunger;
 import org.aspectj.weaver.NewFieldTypeMunger;
 import org.aspectj.weaver.NewMethodTypeMunger;
+import org.aspectj.weaver.ResolvedMember;
 import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.ResolvedTypeMunger;
 
@@ -70,17 +72,29 @@ public class EclipseTypeMunger extends ConcreteTypeMunger {
 	 * Modifies signatures of a TypeBinding through its ClassScope,
 	 * i.e. adds Method|FieldBindings, plays with inheritance, ...
 	 */
-	public boolean munge(SourceTypeBinding sourceType) {
-		ResolvedType rt = world.fromEclipse(sourceType);
+	public boolean munge(SourceTypeBinding sourceType, ResolvedType onType) {
+		ResolvedType rt = onType;
 		if (rt.isRawType() || rt.isParameterizedType()) rt = rt.getGenericType();
-		if (!rt.equals(targetTypeX)) return false; //??? move this test elsewhere
+		boolean isExactTargetType = rt.equals(targetTypeX);
+		if (!isExactTargetType) {
+			// might be the topmost implementor of an interface we care about
+			if (munger.getKind() != ResolvedTypeMunger.Method) return false;
+			if (onType.isInterface()) return false;
+			if (!munger.needsAccessToTopmostImplementor()) return false;
+			// so we do need access, and this type could be it...
+			if (!onType.isTopmostImplementor(targetTypeX)) return false;
+			// we are the topmost implementor of an interface type that needs munging
+			// but we only care about public methods here (we only do this at all to 
+			// drive the JDT MethodVerifier correctly)
+			if (!Modifier.isPublic(munger.getSignature().getModifiers())) return false;
+		}
 		//System.out.println("munging: " + sourceType);
 //		System.out.println("match: " + world.fromEclipse(sourceType) +
 //				" with " + targetTypeX);
 		if (munger.getKind() == ResolvedTypeMunger.Field) {
 			mungeNewField(sourceType, (NewFieldTypeMunger)munger);
 		} else if (munger.getKind() == ResolvedTypeMunger.Method) {
-			mungeNewMethod(sourceType, (NewMethodTypeMunger)munger);
+			return mungeNewMethod(sourceType, onType, (NewMethodTypeMunger)munger, isExactTargetType);
 		} else if (munger.getKind() == ResolvedTypeMunger.Constructor) {
 			mungeNewConstructor(sourceType, (NewConstructorTypeMunger)munger);
 		} else {
@@ -90,18 +104,39 @@ public class EclipseTypeMunger extends ConcreteTypeMunger {
 	}
 	
 
-	private void mungeNewMethod(SourceTypeBinding sourceType, NewMethodTypeMunger munger) {
-//		if (shouldTreatAsPublic()) {
-//			MethodBinding binding = world.makeMethodBinding(munger.getSignature());
-//			findOrCreateInterTypeMemberFinder(classScope).addInterTypeMethod(binding);
-//			//classScope.referenceContext.binding.addMethod(binding);
-//		} else {
-			InterTypeMethodBinding binding =
-				new InterTypeMethodBinding(world, munger.getSignature(), aspectType, sourceMethod);
-			findOrCreateInterTypeMemberFinder(sourceType).addInterTypeMethod(binding);
-//		}
+	private boolean mungeNewMethod(SourceTypeBinding sourceType, ResolvedType onType, NewMethodTypeMunger munger, boolean isExactTargetType) {
+		InterTypeMethodBinding binding =
+			new InterTypeMethodBinding(world, munger.getSignature(), aspectType, sourceMethod);
 
+		if (!isExactTargetType) {
+			// we're munging an interface ITD onto a topmost implementor
+			ResolvedMember existingMember = onType.lookupMemberIncludingITDsOnInterfaces(getSignature()); 
+			if (existingMember != null) {
+				// already have an implementation, so don't do anything
+				if (onType == existingMember.getDeclaringType() && Modifier.isFinal(munger.getSignature().getModifiers())) {
+					// final modifier on default implementation is taken to mean that
+					// no-one else can provide an implementation
+					MethodBinding offendingBinding = sourceType.getExactMethod(binding.selector, binding.parameters, sourceType.scope.compilationUnitScope());
+					sourceType.scope.problemReporter().finalMethodCannotBeOverridden(offendingBinding, binding);
+				}
+				// so that we find methods from our superinterfaces later on...
+				findOrCreateInterTypeMemberFinder(sourceType);
+				return false;
+			} 
+		}
+		
+		// retain *only* the visibility modifiers and abstract when putting methods on an interface...
+		if (sourceType.isInterface()) {
+			boolean isAbstract = (binding.modifiers & IConstants.AccAbstract) != 0;
+			binding.modifiers = (binding.modifiers & (IConstants.AccPublic | IConstants.AccProtected | IConstants.AccPrivate));
+			if (isAbstract) binding.modifiers |= IConstants.AccAbstract;
+		}
+		
+		findOrCreateInterTypeMemberFinder(sourceType).addInterTypeMethod(binding);
+		return true;
 	}
+	
+	
 	private void mungeNewConstructor(SourceTypeBinding sourceType, NewConstructorTypeMunger munger) {		
 		if (shouldTreatAsPublic()) {
 			MethodBinding binding = world.makeMethodBinding(munger.getSignature());
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InterTypeMemberFinder.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InterTypeMemberFinder.java
index db7e8131e..e69e40978 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InterTypeMemberFinder.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InterTypeMemberFinder.java
@@ -14,8 +14,10 @@
 
 package org.aspectj.ajdt.internal.compiler.lookup;
 
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Iterator;
 import java.util.List;
 
 import org.aspectj.org.eclipse.jdt.core.compiler.CharOperation;
@@ -242,9 +244,11 @@ public class InterTypeMemberFinder implements IMemberFinder {
 //		return m2.declaringClass;
 //	}
 
+	// find all of my methods, including ITDs
+	// PLUS: any public ITDs made on interfaces that I implement
 	public MethodBinding[] methods(SourceTypeBinding sourceTypeBinding) {
 		MethodBinding[] orig = sourceTypeBinding.methods();
-		if (interTypeMethods.isEmpty()) return orig;
+//		if (interTypeMethods.isEmpty()) return orig;
 		
 		List ret = new ArrayList(Arrays.asList(orig));
 		for (int i=0, len=interTypeMethods.size(); i < len; i++) {
@@ -252,10 +256,34 @@ public class InterTypeMemberFinder implements IMemberFinder {
 			ret.add(method);
 		}
 		
+		ReferenceBinding [] interfaces = sourceTypeBinding.superInterfaces();
+		for (int i = 0; i < interfaces.length; i++) {
+			if (interfaces[i] instanceof SourceTypeBinding) {
+				SourceTypeBinding intSTB = (SourceTypeBinding) interfaces[i];
+				addPublicITDSFrom(intSTB,ret);
+			}
+		}
+		
 		if (ret.isEmpty()) return SourceTypeBinding.NoMethods;
 		return (MethodBinding[])ret.toArray(new MethodBinding[ret.size()]);	
 	}
 	
+	private void addPublicITDSFrom(SourceTypeBinding anInterface,List toAList) {
+		if (anInterface.memberFinder != null) {
+			InterTypeMemberFinder finder = (InterTypeMemberFinder) anInterface.memberFinder;
+			for (Iterator iter = finder.interTypeMethods.iterator(); iter.hasNext();) {
+				MethodBinding aBinding = (MethodBinding) iter.next();
+				if (Modifier.isPublic(aBinding.modifiers)) {
+					toAList.add(aBinding);
+				}
+			}
+		}
+		ReferenceBinding superType = anInterface.superclass;
+		if (superType instanceof SourceTypeBinding && superType.isInterface()) {
+			addPublicITDSFrom((SourceTypeBinding)superType,toAList);
+		}
+	}
+	
 	//XXX conflicts
 	public MethodBinding[] getMethods(
 		SourceTypeBinding sourceTypeBinding,
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InterTypeMethodBinding.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InterTypeMethodBinding.java
index 290c9ee15..658b6ef13 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InterTypeMethodBinding.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InterTypeMethodBinding.java
@@ -17,6 +17,7 @@ import org.aspectj.weaver.AjcMemberMaker;
 import org.aspectj.weaver.Member;
 import org.aspectj.weaver.ResolvedMember;
 import org.aspectj.weaver.UnresolvedType;
+import org.aspectj.ajdt.internal.compiler.ast.InterTypeMethodDeclaration;
 import org.aspectj.org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.InvocationSite;
 import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
@@ -128,7 +129,11 @@ public class InterTypeMethodBinding extends MethodBinding {
 		return false;
 	}
 
-
+	public boolean isFinal() {
+		if (sourceMethod == null || !(sourceMethod instanceof InterTypeMethodDeclaration)) return super.isFinal();
+		return ((InterTypeMethodDeclaration)sourceMethod).isFinal();
+	}
+	
 	public MethodBinding getAccessMethod(boolean staticReference) {
 		if (staticReference) return postDispatchMethod;
 		else return syntheticMethod;
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/problem/AjProblemReporter.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/problem/AjProblemReporter.java
index 34028b337..421286af1 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/problem/AjProblemReporter.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/problem/AjProblemReporter.java
@@ -412,4 +412,14 @@ public class AjProblemReporter extends ProblemReporter {
     	if (!(methodDecl instanceof PointcutDeclaration))
     			super.unusedPrivateMethod(methodDecl);
     }
+
+    /**
+     * A side-effect of the way that we handle itds on default methods on top-most implementors
+     * of interfaces is that a class acquiring a final default ITD will erroneously report
+     * that it can't override its own member. This method detects that situation.
+     */
+    public void finalMethodCannotBeOverridden(MethodBinding currentMethod, MethodBinding inheritedMethod) {
+    	if (currentMethod == inheritedMethod) return;
+    	super.finalMethodCannotBeOverridden(currentMethod, inheritedMethod);
+    }
 }
diff --git a/org.eclipse.jdt.core/jdtcore-for-aspectj-src.zip b/org.eclipse.jdt.core/jdtcore-for-aspectj-src.zip
index c486d3bad..2cb7b2bd3 100644
--- a/org.eclipse.jdt.core/jdtcore-for-aspectj-src.zip
+++ b/org.eclipse.jdt.core/jdtcore-for-aspectj-src.zip
diff --git a/org.eclipse.jdt.core/jdtcore-for-aspectj.jar b/org.eclipse.jdt.core/jdtcore-for-aspectj.jar
index e5c64c28a..2c246f462 100644
--- a/org.eclipse.jdt.core/jdtcore-for-aspectj.jar
+++ b/org.eclipse.jdt.core/jdtcore-for-aspectj.jar
diff --git a/weaver/src/org/aspectj/weaver/ResolvedType.java b/weaver/src/org/aspectj/weaver/ResolvedType.java
index 0e8ba26c7..73142c23b 100644
--- a/weaver/src/org/aspectj/weaver/ResolvedType.java
+++ b/weaver/src/org/aspectj/weaver/ResolvedType.java
@@ -905,6 +905,24 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
      * Look up a member, takes into account any ITDs on this type.
      * return null if not found */
 	public ResolvedMember lookupMemberNoSupers(Member member) {
+		ResolvedMember ret = lookupDirectlyDeclaredMemberNoSupers(member);
+		if (ret == null && interTypeMungers != null) {
+			for (Iterator i = interTypeMungers.iterator(); i.hasNext();) {
+				ConcreteTypeMunger tm = (ConcreteTypeMunger) i.next();
+				if (matches(tm.getSignature(), member)) {
+					return tm.getSignature();
+				}
+			}
+		}
+		return ret;
+	}
+	
+	/**
+	 * as lookupMemberNoSupers, but does not include ITDs
+	 * @param member
+	 * @return
+	 */
+	public ResolvedMember lookupDirectlyDeclaredMemberNoSupers(Member member) {
 		ResolvedMember ret;
 		if (member.getKind() == Member.FIELD) {
 			ret = lookupMember(member, getDeclaredFields());
@@ -912,11 +930,35 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 			// assert member.getKind() == Member.METHOD || member.getKind() == Member.CONSTRUCTOR
 			ret = lookupMember(member, getDeclaredMethods());
 		}
-		if (ret == null && interTypeMungers != null) {
-			for (Iterator i = interTypeMungers.iterator(); i.hasNext();) {
-				ConcreteTypeMunger tm = (ConcreteTypeMunger) i.next();
-				if (matches(tm.getSignature(), member)) {
-					return tm.getSignature();
+		return ret;
+	}
+	
+	/**
+	 * This lookup has specialized behaviour - a null result tells the
+	 * EclipseTypeMunger that it should make a default implementation of a
+	 * method on this type.
+	 * @param member
+	 * @return
+	 */
+	public ResolvedMember lookupMemberIncludingITDsOnInterfaces(Member member) {
+		return lookupMemberIncludingITDsOnInterfaces(member, this);
+	}
+	
+	private ResolvedMember lookupMemberIncludingITDsOnInterfaces(Member member, ResolvedType onType) {
+		ResolvedMember ret = onType.lookupMemberNoSupers(member);
+		if (ret != null) {
+			return ret;
+		} else {
+			ResolvedType superType = onType.getSuperclass();
+			if (superType != null) {
+				ret = lookupMemberIncludingITDsOnInterfaces(member,superType);
+			}
+			if (ret == null) {
+				// try interfaces then, but only ITDs now...
+				ResolvedType[] superInterfaces = onType.getDeclaredInterfaces();
+				for (int i = 0; i < superInterfaces.length; i++) {
+					ret = superInterfaces[i].lookupMethodInITDs(member);
+					if (ret != null) return ret;
 				}
 			}
 		}
@@ -1256,6 +1298,9 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 	public boolean checkLegalOverride(ResolvedMember parent, ResolvedMember child) {
 		//System.err.println("check: " + child.getDeclaringType() + " overrides " + parent.getDeclaringType());
 		if (Modifier.isFinal(parent.getModifiers())) {
+			// XXX horrible test, if we're in eclipes, child.getSourceLocation will be
+			// null, and this message will have already been issued.
+			if (child.getSourceLocation() == null) return false;
 			world.showMessage(Message.ERROR,
 					WeaverMessages.format(WeaverMessages.CANT_OVERRIDE_FINAL_MEMBER,parent),
 					child.getSourceLocation(),null);