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

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 80271fbdf05d0f991b56b06f3803a4ba380e5c90..a56f2a161b4d77c30e6e1f5924c7022cd4864915 100644 (file)
--- 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 (file)
index 0000000..04c754d
--- /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 4f6d82ac400fc992fbb2aaf4c0d7a2b668174023..d9baeec722c9766a636ebd6fa0909df7ffce59ee 100644 (file)
--- 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 f36716f536d273031503de741a351d2173858d61..fc7622c388834c5e1011bd29b5e6562d7625afba 100644 (file)
--- 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>
          
     <!-- ============================================================================ -->
     <!-- ============================================================================ -->