tests and fix for pr1202212 - poor job of testing modifiers on ITD methods

4 files changed, 99 insertions, 0 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 80271fbdf..a56f2a161 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
@@ -28,6 +28,7 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.flow.InitializationFlowCont
 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.SourceTypeBinding;
 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;
@@ -89,6 +90,7 @@ public class InterTypeMethodDeclaration extends InterTypeDeclaration {
 	
 	
 	public void resolveStatements() {
+		checkAndSetModifiersForMethod();
         if ((modifiers & AccSemicolonBody) != 0) {
             if ((declaredModifiers & AccAbstract) == 0)
                 scope.problemReporter().methodNeedBody(this);
@@ -275,4 +277,68 @@ public class InterTypeMethodDeclaration extends InterTypeDeclaration {
 	protected Shadow.Kind getShadowKindForBody() {
 		return Shadow.MethodExecution;
 	}
+	
+	// XXX this code is copied from MethodScope, with a few adjustments for ITDs...
+	private void checkAndSetModifiersForMethod() {
+		
+		// for reported problems, we want the user to see the declared selector
+		char[] realSelector = this.selector;
+		this.selector = declaredSelector;
+		
+		final ReferenceBinding declaringClass = this.binding.declaringClass;
+		if ((declaredModifiers & AccAlternateModifierProblem) != 0)
+			scope.problemReporter().duplicateModifierForMethod(onTypeBinding, this);
+
+		// after this point, tests on the 16 bits reserved.
+		int realModifiers = declaredModifiers & AccJustFlag;
+
+		// check for abnormal modifiers
+		int unexpectedModifiers = ~(AccPublic | AccPrivate | AccProtected
+			| AccAbstract | AccStatic | AccFinal | AccSynchronized | AccNative | AccStrictfp);
+		if ((realModifiers & unexpectedModifiers) != 0) {
+			scope.problemReporter().illegalModifierForMethod(this);
+			declaredModifiers &= ~AccJustFlag | ~unexpectedModifiers;
+		}
+
+		// check for incompatible modifiers in the visibility bits, isolate the visibility bits
+		int accessorBits = realModifiers & (AccPublic | AccProtected | AccPrivate);
+		if ((accessorBits & (accessorBits - 1)) != 0) {
+			scope.problemReporter().illegalVisibilityModifierCombinationForMethod(onTypeBinding, this);
+
+			// need to keep the less restrictive so disable Protected/Private as necessary
+			if ((accessorBits & AccPublic) != 0) {
+				if ((accessorBits & AccProtected) != 0)
+					declaredModifiers &= ~AccProtected;
+				if ((accessorBits & AccPrivate) != 0)
+					declaredModifiers &= ~AccPrivate;
+			} else if ((accessorBits & AccProtected) != 0 && (accessorBits & AccPrivate) != 0) {
+				declaredModifiers &= ~AccPrivate;
+			}
+		}
+
+		// check for modifiers incompatible with abstract modifier
+		if ((declaredModifiers & AccAbstract) != 0) {
+			int incompatibleWithAbstract = AccStatic | AccFinal | AccSynchronized | AccNative | AccStrictfp;
+			if ((declaredModifiers & incompatibleWithAbstract) != 0)
+				scope.problemReporter().illegalAbstractModifierCombinationForMethod(onTypeBinding, this);
+			if (!onTypeBinding.isAbstract())
+				scope.problemReporter().abstractMethodInAbstractClass((SourceTypeBinding) onTypeBinding, this);
+		}
+
+		/* DISABLED for backward compatibility with javac (if enabled should also mark private methods as final)
+		// methods from a final class are final : 8.4.3.3 
+		if (methodBinding.declaringClass.isFinal())
+			modifiers |= AccFinal;
+		*/
+		// native methods cannot also be tagged as strictfp
+		if ((declaredModifiers & AccNative) != 0 && (declaredModifiers & AccStrictfp) != 0)
+			scope.problemReporter().nativeMethodsCannotBeStrictfp(onTypeBinding, this);
+
+		// static members are only authorized in a static member or top level type
+		if (((realModifiers & AccStatic) != 0) && declaringClass.isNestedType() && !declaringClass.isStatic())
+			scope.problemReporter().unexpectedStaticModifierForMethod(onTypeBinding, this);
+
+		// restore the true selector now that any problems have been reported
+		this.selector = realSelector;		
+	}
 }
diff --git a/tests/bugs150/pr102212.aj b/tests/bugs150/pr102212.aj
new file mode 100644
index 000000000..04c754d6c
--- /dev/null
+++ b/tests/bugs150/pr102212.aj
@@ -0,0 +1,22 @@
+interface Interface {}
+abstract class Parent {}
+class Child extends Parent implements Interface {}
+
+public aspect pr102212 {
+  // illegal modifier combination not caught by ajc
+  public abstract synchronized void Parent._abstract();
+
+  public synchronized void Child._abstract() {}
+
+
+  // the following is legal - it's a default implementation....
+  public /* implicit abstract */ synchronized void Interface._interface() {}
+
+
+  // use Child to make java complain: "illegal modifiers: 0x421"
+  // (this corresponds to "public abstract synchronized")
+  public static void main(String[] args) {
+    new Child();
+  }
+}
+
diff --git a/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
index 4f6d82ac4..d9baeec72 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
@@ -369,6 +369,11 @@ public class Ajc150Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   public void testOverrideAndCovarianceWithDecPRuntimeMultiFiles() {
 	  runTest("override and covariance with decp - runtime separate files");
   }
+  
+  public void testAbstractSynchronizedITDMethods() {
+	  runTest("abstract synchronized itdms not detected");
+  }
+  
   // helper methods.....
   
   public SyntheticRepository createRepos(File cpentry) {
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
index f36716f53..fc7622c38 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
@@ -456,6 +456,12 @@
             </stdout>
         </run>
     </ajc-test>
+    
+    <ajc-test dir="bugs150" pr="102212" title="abstract synchronized itdms not detected">
+        <compile files="pr102212.aj">
+            <message line="7" kind="error" text="The abstract method _abstract in type Parent can only set a visibility modifier, one of public or protected"/>
+        </compile>
+    </ajc-test>
          
     <!-- ============================================================================ -->
     <!-- ============================================================================ -->