fixes inefficient ordering of tests in SignaturePattern (that I inadvertantly introduced earlier), and corrects @DeclareParents impl in AJTypeSystem :- fields should NOT be shown, and nor should non-interface members.

8 files changed, 27 insertions, 60 deletions

diff --git a/aspectj5rt/java5-src/org/aspectj/internal/lang/reflect/AjTypeImpl.java b/aspectj5rt/java5-src/org/aspectj/internal/lang/reflect/AjTypeImpl.java
index 54c500801..482ccebb9 100644
--- a/aspectj5rt/java5-src/org/aspectj/internal/lang/reflect/AjTypeImpl.java
+++ b/aspectj5rt/java5-src/org/aspectj/internal/lang/reflect/AjTypeImpl.java
@@ -662,17 +662,12 @@ public class AjTypeImpl<T> implements AjType<T> {
             for (Field f : clazz.getDeclaredFields()) {
                 if (!f.getType().isInterface()) continue;
                 if (!Modifier.isPublic(f.getModifiers()) || !Modifier.isStatic(f.getModifiers())) continue;
-                if (f.isAnnotationPresent(org.aspectj.lang.annotation.DeclareParents.class)) {
-                	 Class itdSource = f.getType();
-                	 try {
-                	 	itdSource = f.get(null).getClass();
-                	 } catch (IllegalAccessException ex) {
-                		//
-                	 }
-                    for (Method itdM : itdSource.getDeclaredMethods()) {
+                if (f.isAnnotationPresent(org.aspectj.lang.annotation.DeclareParents.class)) {                	 
+                    for (Method itdM : f.getType().getDeclaredMethods()) {
                         if (!Modifier.isPublic(itdM.getModifiers()) && publicOnly) continue;
                         InterTypeMethodDeclaration itdm = new InterTypeMethodDeclarationImpl(
-                                    this, AjTypeSystem.getAjType(f.getType()), itdM
+                                    this, AjTypeSystem.getAjType(f.getType()), itdM,
+                                    Modifier.PUBLIC
                         );
                         toList.add(itdm);
                     }
@@ -684,28 +679,7 @@ public class AjTypeImpl<T> implements AjType<T> {
 	private void addAnnotationStyleITDFields(List<InterTypeFieldDeclaration> toList, boolean publicOnly) {
         //AV: I think it is meaningless
         //@AJ decp is interface driven ie no field
-        // AMC: private fields in the mixin type should show as ITDs private to the aspect...
-        if (isAspect()) {
-            for (Field f : clazz.getDeclaredFields()) {
-                if (!f.getType().isInterface()) continue;
-                if (!Modifier.isPublic(f.getModifiers()) || !Modifier.isStatic(f.getModifiers())) continue;
-                if (f.isAnnotationPresent(org.aspectj.lang.annotation.DeclareParents.class)) {
-                	 Class itdSource = f.getType();
-                	 try {
-                	 	itdSource = f.get(null).getClass();
-                	 } catch (IllegalAccessException ex) {
-                		//
-                	 }
-                    for (Field itdF : itdSource.getDeclaredFields()) {
-                        if (!Modifier.isPublic(itdF.getModifiers()) && publicOnly) continue;
-                        InterTypeFieldDeclaration itdf = new InterTypeFieldDeclarationImpl(
-                                    this, AjTypeSystem.getAjType(f.getType()), itdF
-                        );
-                        toList.add(itdf);
-                    }
-                }
-            }
-		}
+		return;
 	}
 
 	/* (non-Javadoc)
diff --git a/aspectj5rt/java5-src/org/aspectj/internal/lang/reflect/InterTypeMethodDeclarationImpl.java b/aspectj5rt/java5-src/org/aspectj/internal/lang/reflect/InterTypeMethodDeclarationImpl.java
index 2ed2f89cf..eb085a055 100644
--- a/aspectj5rt/java5-src/org/aspectj/internal/lang/reflect/InterTypeMethodDeclarationImpl.java
+++ b/aspectj5rt/java5-src/org/aspectj/internal/lang/reflect/InterTypeMethodDeclarationImpl.java
@@ -47,8 +47,8 @@ public class InterTypeMethodDeclarationImpl extends InterTypeDeclarationImpl
 		this.baseMethod = itdInterMethod;
 	}
 
-	public InterTypeMethodDeclarationImpl(AjType<?> decType, AjType<?> targetType, Method base) {
-		super(decType,targetType,base.getModifiers());
+	public InterTypeMethodDeclarationImpl(AjType<?> decType, AjType<?> targetType, Method base, int modifiers) {
+		super(decType,targetType,modifiers);
 		this.parameterAdjustmentFactor = 0;
 		this.name = base.getName();
 		this.baseMethod = base;
diff --git a/lib/aspectj/lib/aspectjrt.jar b/lib/aspectj/lib/aspectjrt.jar
index 60ce8de37..f8d847b2f 100644
--- a/lib/aspectj/lib/aspectjrt.jar
+++ b/lib/aspectj/lib/aspectjrt.jar
diff --git a/lib/test/aspectjrt.jar b/lib/test/aspectjrt.jar
index 60ce8de37..f8d847b2f 100644
--- a/lib/test/aspectjrt.jar
+++ b/lib/test/aspectjrt.jar
diff --git a/tests/java5/reflection/AtAspectJDeclareParents.aj b/tests/java5/reflection/AtAspectJDeclareParents.aj
index 2882c8734..d60f6a3c6 100644
--- a/tests/java5/reflection/AtAspectJDeclareParents.aj
+++ b/tests/java5/reflection/AtAspectJDeclareParents.aj
@@ -9,7 +9,13 @@ public aspect AtAspectJDeclareParents {
 
 class C {}
 
-interface I{}
+interface I{
+	
+	int getX();
+	
+	void setX(int x);
+	
+}
 
 class Impl implements I {
 	
diff --git a/tests/java5/reflection/ReflectOnAtAspectJDeclareParents.java b/tests/java5/reflection/ReflectOnAtAspectJDeclareParents.java
index efd482820..8f01325af 100644
--- a/tests/java5/reflection/ReflectOnAtAspectJDeclareParents.java
+++ b/tests/java5/reflection/ReflectOnAtAspectJDeclareParents.java
@@ -101,21 +101,7 @@ public class ReflectOnAtAspectJDeclareParents {
 	
 	private void testDeclaredInterTypeFields(AjType<AtAspectJDeclareParents> ajType) {
 		InterTypeFieldDeclaration[] itdfs = ajType.getDeclaredITDFields();
-		assertEquals(1,itdfs.length,"number of declared ITD fields");
-		System.out.println(itdfs[0]);
-		try {
-			InterTypeFieldDeclaration shouldFind = ajType.getDeclaredITDField("x",AjTypeSystem.getAjType(I.class));
-			System.out.println(shouldFind);
-		} catch (NoSuchFieldException ex) {
-			throw new RuntimeException("getITDField failed");
-		}
-		try {
-			ajType.getDeclaredITDField("p",AjTypeSystem.getAjType(C.class));
-			throw new RuntimeException("failed to fail in getting ITDField");
-		} catch (NoSuchFieldException ex) {
-			// good!
-		}
-		
+		assertEquals(0,itdfs.length,"number of declared ITD fields");
 	}
 	
 	private void testInterTypeFields(AjType<AtAspectJDeclareParents> ajType) {
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
index d6814e28e..13bd9e496 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
@@ -290,8 +290,6 @@
 			<line text="public int I.getX()"/>
 		  	<line text="public void I.setX(int)"/>
 		  	<line text="public int I.getX()"/>
-		  	<line text="private int I.x"/>
-		  	<line text="private int I.x"/>
 		  </stdout>
 		</run>
 	</ajc-test>
diff --git a/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java b/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java
index 445cc53a1..9515ec8cc 100644
--- a/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java
@@ -312,21 +312,24 @@ public class SignaturePattern extends PatternNode {
 //			else return FuzzyBoolean.MAYBE;
 		}
 		
-		// annotations match on the *subject* 
-		if (!matchesAnnotations(aMember,inAWorld).alwaysTrue()) {
-			return FuzzyBoolean.NO;
-		}
 		
+		FuzzyBoolean matchesIgnoringAnnotations = FuzzyBoolean.YES;
 		if (kind == Member.STATIC_INITIALIZATION) {
-			return matchesExactlyStaticInitialization(aMember, inAWorld);
+			matchesIgnoringAnnotations = matchesExactlyStaticInitialization(aMember, inAWorld);
 		} else if (kind == Member.FIELD) {
-			return matchesExactlyField(aMember,inAWorld);
+			matchesIgnoringAnnotations = matchesExactlyField(aMember,inAWorld);
 		} else if (kind == Member.METHOD) {
-			return matchesExactlyMethod(aMember,inAWorld);
+			matchesIgnoringAnnotations = matchesExactlyMethod(aMember,inAWorld);
 		} else if (kind == Member.CONSTRUCTOR) {
-			return matchesExactlyConstructor(aMember, inAWorld);
+			matchesIgnoringAnnotations = matchesExactlyConstructor(aMember, inAWorld);
+		}
+		if (matchesIgnoringAnnotations.alwaysFalse()) return FuzzyBoolean.NO;
+		
+		// annotations match on the *subject* 
+		if (!matchesAnnotations(aMember,inAWorld).alwaysTrue()) {
+			return FuzzyBoolean.NO;
 		} else {
-			return FuzzyBoolean.YES;
+			return matchesIgnoringAnnotations;
 		}
 		
 	}