Fix for Bugzilla Bug 70794

The introduction on interface causes the interface implementation class error

tests/bugs/interfaceITDs/PackagePackage.java

@@ -0,0 +1,27 @@
+interface PackagePackage {
+	//empty interface
+}
+
+
+//aspectj introduce a method to this interface
+ privileged aspect aspectWorld {
+	abstract void PackagePackage.world();
+//	 void test.andy() {
+//	 	
+//	 }
+
+}
+
+//class test implements hello interface, and
+//method world
+ class test implements PackagePackage{
+    
+	public void world() {
+		 System.out.println("hello");
+	}
+	
+	public static void main(String[] args) {
+		test t = new test();
+		t.world();
+	}
+}

tests/bugs/interfaceITDs/PackagePublic.java

@@ -0,0 +1,23 @@
+interface PackagePublic {
+	//empty interface
+}
+
+
+//aspectj introduce a method to this interface
+ privileged aspect aspectWorld {
+	public abstract void PackagePublic.world();
+}
+
+//class test implements hello interface, and
+//method world
+ class test implements PackagePublic{
+    
+	public void world() {
+		 System.out.println("hello");
+	}
+	
+	public static void main(String[] args) {
+		test t = new test();
+		t.world();
+	}
+}

tests/bugs/interfaceITDs/PublicPackage.java

@@ -0,0 +1,23 @@
+public interface PublicPackage {
+	//empty interface
+}
+
+
+//aspectj introduce a method to this interface
+privileged aspect aspectWorld {
+	abstract void PublicPackage.world();
+}
+
+//class test implements hello interface, and
+//method world
+ class test implements PublicPackage {
+    
+	public void world() {
+		 System.out.println("hello");
+	}
+	
+	public static void main(String[] args) {
+		test t = new test();
+		t.world();
+	}
+}

tests/bugs/interfaceITDs/PublicPublic.java

@@ -0,0 +1,23 @@
+public interface PublicPublic {
+	//empty interface
+}
+
+
+//aspectj introduce a method to this interface
+privileged aspect aspectWorld {
+	public abstract void PublicPublic.world();
+}
+
+//class test implements hello interface, and
+//method world
+ class test implements PublicPublic {
+    
+	public void world() {
+		 System.out.println("hello");
+	}
+	
+	public static void main(String[] args) {
+		test t = new test();
+		t.world();
+	}
+}

tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java

@@ -248,5 +248,21 @@ public class Ajc121Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   	runTest("wrong line for method execution join point");
   }
 
+  public void test046_interfaceITD_pr70794_1() {
+    runTest("The introduction on interface causes the interface implementation class error (1)");
+  }
+
+  public void test047_interfaceITD_pr70794_2() {
+    runTest("The introduction on interface causes the interface implementation class error (2)");
+  }
+
+  public void test048_interfaceITD_pr70794_3() {
+    runTest("The introduction on interface causes the interface implementation class error (3)");
+  }
+
+  public void test049_interfaceITD_pr70794_4() {
+    runTest("The introduction on interface causes the interface implementation class error (4)");
+  }
+  
 }
 

tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml

@@ -377,4 +377,30 @@
 			<message kind="warning" line="39" text="static init"/>
 			<message kind="warning" line="47" text="advice"/>
 		</compile>
-	</ajc-test>
+	</ajc-test>
+
+	<ajc-test dir="bugs/interfaceITDs" pr="70794"
+	  	title="The introduction on interface causes the interface implementation class error (1)">
+		<compile files="PublicPublic.java"/>
+	</ajc-test>
+		
+	
+	<ajc-test dir="bugs/interfaceITDs" pr="70794"
+	  	title="The introduction on interface causes the interface implementation class error (2)">
+		<compile files="PackagePublic.java"/>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/interfaceITDs" pr="70794"
+	  	title="The introduction on interface causes the interface implementation class error (3)">
+		<compile files="PackagePackage.java">
+			<message kind="error" line="17" text="abstract intertype method declaration 'void PackagePackage.world()' on interface PackagePackage must be declared public (compiler limitation)"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/interfaceITDs" pr="70794"
+	  	title="The introduction on interface causes the interface implementation class error (4)">
+		<compile files="PublicPackage.java">
+			<message kind="error" line="13" text="abstract intertype method declaration 'void PublicPackage.world()' on interface PublicPackage must be declared public (compiler limitation)"/>
+		</compile>
+	</ajc-test>
+		

weaver/src/org/aspectj/weaver/ResolvedTypeX.java

@@ -975,31 +975,75 @@ public abstract class ResolvedTypeX extends TypeX {
         collector.addAll(getInterTypeMungers());
     }
     
+ 
     
     /**
-     * Check that we don't have any abstract type mungers unless this
-     * type is abstract.
+     * Check:
+     * 1) That we don't have any abstract type mungers unless this type is abstract.
+     * 2) That an abstract ITDM on an interface is declared public. (Compiler limitation) (PR70794)
      */
     public void checkInterTypeMungers() {
         if (isAbstract()) return;
         
+        boolean itdProblem = false;
+        
         for (Iterator iter = getInterTypeMungersIncludingSupers().iterator(); iter.hasNext();) {
-			ConcreteTypeMunger element = (ConcreteTypeMunger) iter.next();
-            if (element.getSignature() != null && element.getSignature().isAbstract()) {
-            	ISourceLocation xtraLocation = element.getSourceLocation();
-            	if (xtraLocation == null) {
-            		// Until intertype mungers remember where they came from, the source location
-            		// for the element is null when binary weaving.  In these cases uses the
-            		// source location for the aspect containing the ITD
-     				xtraLocation = element.getAspectType().getSourceLocation();
-            	}
+			ConcreteTypeMunger munger = (ConcreteTypeMunger) iter.next();
+			itdProblem = checkAbstractDeclaration(munger) || itdProblem; // Rule 2
+
+        }
+        
+        if (itdProblem) return; // If the rules above are broken, return right now
+        
+		for (Iterator iter = getInterTypeMungersIncludingSupers().iterator(); iter.hasNext();) {
+			ConcreteTypeMunger munger = (ConcreteTypeMunger) iter.next();
+            if (munger.getSignature() != null && munger.getSignature().isAbstract()) { // Rule 1
                 world.getMessageHandler().handleMessage(
-                    new Message("must implement abstract inter-type declaration: " + element.getSignature(),
+                    new Message("must implement abstract inter-type declaration: " + munger.getSignature(),
                         "", IMessage.ERROR, getSourceLocation(), null, 
-                        new ISourceLocation[] { xtraLocation }));
+                        new ISourceLocation[] { getMungerLocation(munger) }));
             }
 		}
     }
+    
+    /**
+     * See PR70794.  This method checks that if an abstract inter-type method declaration is made on
+     * an interface then it must also be public.
+     * This is a compiler limitation that could be made to work in the future (if someone
+     * provides a worthwhile usecase)
+     * 
+     * @return indicates if the munger failed the check
+     */
+    private boolean checkAbstractDeclaration(ConcreteTypeMunger munger) {
+		if (munger.getMunger()!=null && (munger.getMunger() instanceof NewMethodTypeMunger)) {
+			ResolvedMember itdMember = munger.getSignature();
+			ResolvedTypeX onType = itdMember.getDeclaringType().resolve(world);
+			if (onType.isInterface() && itdMember.isAbstract() && !itdMember.isPublic()) {
+					world.getMessageHandler().handleMessage(
+							new Message(WeaverMessages.format(WeaverMessages.ITD_ABSTRACT_MUST_BE_PUBLIC_ON_INTERFACE,munger.getSignature(),onType),"",
+									Message.ERROR,getSourceLocation(),null,
+									new ISourceLocation[]{getMungerLocation(munger)})	
+						);
+					return true;
+				}			
+		}
+		return false;
+    }
+    
+    /**
+     * Get a source location for the munger.
+     * Until intertype mungers remember where they came from, the source location
+     * for the munger itself is null.  In these cases use the
+     * source location for the aspect containing the ITD.
+     * 
+     */
+    private ISourceLocation getMungerLocation(ConcreteTypeMunger munger) {
+    	ISourceLocation sloc = munger.getSourceLocation();
+    	if (sloc == null) {
+    		sloc = munger.getAspectType().getSourceLocation();
+    	}
+    	return sloc;
+    }
 	
     /**
      * Returns a ResolvedTypeX object representing the declaring type of this type, or

weaver/src/org/aspectj/weaver/WeaverMessages.java

@@ -53,7 +53,7 @@ public class WeaverMessages {
 	public static final String ITD_CONFLICT = "itdConflict";
 	public static final String ITD_MEMBER_CONFLICT = "itdMemberConflict";
 	public static final String ITD_NON_EXPOSED_IMPLEMENTOR = "itdNonExposedImplementor";
-	
+	public static final String ITD_ABSTRACT_MUST_BE_PUBLIC_ON_INTERFACE = "itdAbstractMustBePublicOnInterface";
 	
 	public static final String NON_VOID_RETURN = "nonVoidReturn";
 	public static final String INCOMPATIBLE_RETURN_TYPE="incompatibleReturnType";

weaver/src/org/aspectj/weaver/weaver-messages.properties

@@ -52,6 +52,7 @@ overridingStatic={0} cannot override {1}; overriding method is static
 itdConflict=intertype declaration from {0} conflicts with intertype declaration: {1} from {2}
 itdMemberConflict=inter-type declaration from {0} conflicts with existing member: {1}
 itdNonExposedImplementor=type {0} must be accessible for weaving interface inter type declaration from aspect {1}
+itdAbstractMustBePublicOnInterface=abstract intertype method declaration ''{0}'' on interface {1} must be declared public (compiler limitation)
 
 # advice messages...
 nonVoidReturn=applying to join point that doesn't return void: {0}