fixes for 115252 from Helen (xlint on incorrect annotation usage in signatures)

10 files changed, 311 insertions, 3 deletions

diff --git a/weaver/src/org/aspectj/weaver/AnnotationTargetKind.java b/weaver/src/org/aspectj/weaver/AnnotationTargetKind.java
new file mode 100644
index 000000000..e0370411a
--- /dev/null
+++ b/weaver/src/org/aspectj/weaver/AnnotationTargetKind.java
@@ -0,0 +1,51 @@
+/********************************************************************
+ * Copyright (c) 2005 Contributors. All rights reserved. 
+ * This program and the accompanying materials are made available 
+ * under the terms of the Eclipse Public License v1.0 
+ * which accompanies this distribution and is available at 
+ * http://eclipse.org/legal/epl-v10.html 
+ *  
+ * Contributors: 
+ *       Helen Hawkins - Initial implementation
+ *******************************************************************/
+package org.aspectj.weaver;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+
+import org.aspectj.util.TypeSafeEnum;
+
+/**
+ * A TypeSafeEnum similar to the Java5 ElementType Enum
+ */
+public class AnnotationTargetKind extends TypeSafeEnum {
+
+	public AnnotationTargetKind(String name, int key) {
+		super(name, key);
+	}
+	
+    public static AnnotationTargetKind read(DataInputStream s) throws IOException {
+        int key = s.readByte();
+        switch(key) {
+            case 1: return ANNOTATION_TYPE;
+            case 2: return CONSTRUCTOR;
+            case 3: return FIELD;
+            case 4: return LOCAL_VARIABLE;
+            case 5: return METHOD;
+            case 6: return PACKAGE;
+            case 7: return PARAMETER;
+            case 8: return TYPE;
+        }
+        throw new BCException("weird annotation target kind " + key);
+    }
+
+	public static final AnnotationTargetKind ANNOTATION_TYPE = new AnnotationTargetKind("ANNOTATION_TYPE", 1);
+	public static final AnnotationTargetKind CONSTRUCTOR = new AnnotationTargetKind("CONSTRUCTOR", 2);
+	public static final AnnotationTargetKind FIELD = new AnnotationTargetKind("FIELD", 3);
+	public static final AnnotationTargetKind LOCAL_VARIABLE = new AnnotationTargetKind("LOCAL_VARIABLE", 4);
+	public static final AnnotationTargetKind METHOD = new AnnotationTargetKind("METHOD", 5);
+	public static final AnnotationTargetKind PACKAGE = new AnnotationTargetKind("PACKAGE", 6);
+	public static final AnnotationTargetKind PARAMETER = new AnnotationTargetKind("PARAMETER", 7);
+	public static final AnnotationTargetKind TYPE = new AnnotationTargetKind("TYPE", 8);
+
+}
diff --git a/weaver/src/org/aspectj/weaver/BoundedReferenceType.java b/weaver/src/org/aspectj/weaver/BoundedReferenceType.java
index fe8bda13b..f2f7dac99 100644
--- a/weaver/src/org/aspectj/weaver/BoundedReferenceType.java
+++ b/weaver/src/org/aspectj/weaver/BoundedReferenceType.java
@@ -185,6 +185,14 @@ public class BoundedReferenceType extends ReferenceType {
 			return resolvedTypeX.getRetentionPolicy();
 		}
 
+		public boolean canAnnotationTargetType() {
+			return resolvedTypeX.canAnnotationTargetType();
+		}
+		
+		public AnnotationTargetKind[] getAnnotationTargetKinds() {
+			return resolvedTypeX.getAnnotationTargetKinds();
+		}
+		
 		public boolean isGeneric() {
 			return resolvedTypeX.isGenericType();
 		}
diff --git a/weaver/src/org/aspectj/weaver/Lint.java b/weaver/src/org/aspectj/weaver/Lint.java
index e8b237079..ff7cb5332 100644
--- a/weaver/src/org/aspectj/weaver/Lint.java
+++ b/weaver/src/org/aspectj/weaver/Lint.java
@@ -51,6 +51,9 @@ public class Lint {
 	public final Kind unmatchedSuperTypeInCall = 
 		new Kind("unmatchedSuperTypeInCall", "does not match because declaring type is {0}, if match desired use target({1})");
 
+	public final Kind unmatchedTargetKind = 
+		new Kind("unmatchedTargetKind", "does not match because annotation {0} has @Target{1}");
+	
 	public final Kind canNotImplementLazyTjp = 
 		new Kind("canNotImplementLazyTjp", "can not implement lazyTjp on this joinpoint {0} because around advice is used");
 
diff --git a/weaver/src/org/aspectj/weaver/ReferenceType.java b/weaver/src/org/aspectj/weaver/ReferenceType.java
index 22a21a12c..fbf9ec0df 100644
--- a/weaver/src/org/aspectj/weaver/ReferenceType.java
+++ b/weaver/src/org/aspectj/weaver/ReferenceType.java
@@ -158,6 +158,14 @@ public class ReferenceType extends ResolvedType {
     public boolean isAnnotationWithRuntimeRetention() {
         return delegate.isAnnotationWithRuntimeRetention();
     }
+    
+    public boolean canAnnotationTargetType() {
+    	return delegate.canAnnotationTargetType();
+    }
+    
+    public AnnotationTargetKind[] getAnnotationTargetKinds() {
+    	return delegate.getAnnotationTargetKinds();
+    }
       
     // true iff the statement "this = (ThisType) other" would compile
     public final boolean isCoerceableFrom(ResolvedType o) {
diff --git a/weaver/src/org/aspectj/weaver/ReferenceTypeDelegate.java b/weaver/src/org/aspectj/weaver/ReferenceTypeDelegate.java
index 2c49ef54d..d00a4d288 100644
--- a/weaver/src/org/aspectj/weaver/ReferenceTypeDelegate.java
+++ b/weaver/src/org/aspectj/weaver/ReferenceTypeDelegate.java
@@ -25,8 +25,6 @@ public interface ReferenceTypeDelegate {
 	// TODO asc move to proxy
 	public void addAnnotation(AnnotationX annotationX);
 	public void ensureDelegateConsistent(); // Required evil because of mutator methods in delegates :(  (see pr85132)
-
-	
 	
 	public boolean isAspect();
     public boolean isAnnotationStyleAspect();
@@ -34,6 +32,8 @@ public interface ReferenceTypeDelegate {
     public boolean isEnum();
     public boolean isAnnotation();
     public String getRetentionPolicy();
+    public boolean canAnnotationTargetType();
+    public AnnotationTargetKind[] getAnnotationTargetKinds();
     public boolean isAnnotationWithRuntimeRetention();
 	public boolean isClass();
 	public boolean isGeneric();
@@ -61,4 +61,5 @@ public interface ReferenceTypeDelegate {
 	public boolean doesNotExposeShadowMungers();
 	
 	public String getDeclaredGenericSignature();
+	
 }
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/ResolvedType.java b/weaver/src/org/aspectj/weaver/ResolvedType.java
index e4cbb5be7..a590b7440 100644
--- a/weaver/src/org/aspectj/weaver/ResolvedType.java
+++ b/weaver/src/org/aspectj/weaver/ResolvedType.java
@@ -633,8 +633,21 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 	public AnnotationX[] getAnnotations() {
 		throw new RuntimeException("ResolvedType.getAnnotations() should never be called");
 	}
-
     
+	/**
+	 * Note: Only overridden by ReferenceType subtype
+	 */
+	public boolean canAnnotationTargetType() {
+		return false;
+	}
+	
+	/**
+	 * Note: Only overridden by ReferenceType subtype
+	 */	
+	public AnnotationTargetKind[] getAnnotationTargetKinds() {
+		return null;
+	}
+	
     /**
      * Note: Only overridden by Name subtype.
      */
diff --git a/weaver/src/org/aspectj/weaver/XlintDefault.properties b/weaver/src/org/aspectj/weaver/XlintDefault.properties
index bb40c00cd..e495f4aa4 100644
--- a/weaver/src/org/aspectj/weaver/XlintDefault.properties
+++ b/weaver/src/org/aspectj/weaver/XlintDefault.properties
@@ -32,3 +32,5 @@ uncheckedArgument = warning
 noExplicitConstructorCall = warning
 
 aspectExcludedByConfiguration = ignore
+
+unmatchedTargetKind = warning
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
index cacd98fa0..a6d499a9a 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java
@@ -33,6 +33,7 @@ import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.weaver.AbstractReferenceTypeDelegate;
 import org.aspectj.weaver.AjAttribute;
 import org.aspectj.weaver.AjcMemberMaker;
+import org.aspectj.weaver.AnnotationTargetKind;
 import org.aspectj.weaver.AnnotationX;
 import org.aspectj.weaver.BCException;
 import org.aspectj.weaver.ReferenceType;
@@ -88,6 +89,8 @@ public class BcelObjectType extends AbstractReferenceTypeDelegate {
 	
 	private boolean discoveredRetentionPolicy = false;
 	private String retentionPolicy;
+	private boolean discoveredAnnotationTargetKinds = false;
+	private AnnotationTargetKind[] annotationTargetKinds;
 	
 	
 	/**
@@ -470,6 +473,60 @@ public class BcelObjectType extends AbstractReferenceTypeDelegate {
 	    return retentionPolicy;
 	}
 	
+	public boolean canAnnotationTargetType() {
+		AnnotationTargetKind[] targetKinds = getAnnotationTargetKinds();
+		if (targetKinds == null) return true;
+		for (int i = 0; i < targetKinds.length; i++) {
+			if (targetKinds[i].equals(AnnotationTargetKind.TYPE)) {
+				return true;
+			}
+		}
+		return false;
+	}
+	
+	public AnnotationTargetKind[] getAnnotationTargetKinds() {
+		if (discoveredAnnotationTargetKinds) return annotationTargetKinds;
+		discoveredAnnotationTargetKinds = true;
+		annotationTargetKinds = null; // null means we have no idea or the @Target annotation hasn't been used
+		List targetKinds = new ArrayList();
+		if (isAnnotation()) {
+	        Annotation[] annotationsOnThisType = javaClass.getAnnotations();
+	        for (int i = 0; i < annotationsOnThisType.length; i++) {
+	            Annotation a = annotationsOnThisType[i];
+	            if (a.getTypeName().equals(UnresolvedType.AT_TARGET.getName())) {
+	                List values = a.getValues();
+	                for (Iterator it = values.iterator(); it.hasNext();) {
+                        ElementNameValuePair element = (ElementNameValuePair) it.next();
+                        ElementValue v = element.getValue();
+                        String targetKind = v.stringifyValue();
+                        if (targetKind.equals("ANNOTATION_TYPE")) {
+							targetKinds.add(AnnotationTargetKind.ANNOTATION_TYPE);
+                        } else if (targetKind.equals("CONSTRUCTOR")) {
+							targetKinds.add(AnnotationTargetKind.CONSTRUCTOR);
+						} else if (targetKind.equals("FIELD")) {
+							targetKinds.add(AnnotationTargetKind.FIELD);
+						} else if (targetKind.equals("LOCAL_VARIABLE")) {
+							targetKinds.add(AnnotationTargetKind.LOCAL_VARIABLE);
+						} else if (targetKind.equals("METHOD")) {
+							targetKinds.add(AnnotationTargetKind.METHOD);
+						} else if (targetKind.equals("PACKAGE")) {
+							targetKinds.add(AnnotationTargetKind.PACKAGE);
+						} else if (targetKind.equals("PARAMETER")) {
+							targetKinds.add(AnnotationTargetKind.PARAMETER);
+						} else if (targetKind.equals("TYPE")) {
+							targetKinds.add(AnnotationTargetKind.TYPE);
+						} 
+                    }
+	            }
+	        }
+			if (!targetKinds.isEmpty()) {
+				annotationTargetKinds = new AnnotationTargetKind[targetKinds.size()];
+				return (AnnotationTargetKind[]) targetKinds.toArray(annotationTargetKinds);	
+			}
+		}
+		return annotationTargetKinds;
+	}
+	
 	public boolean isSynthetic() {
 		return getResolvedTypeX().isSynthetic();
 	}
diff --git a/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java b/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java
index 158130979..3759d12d7 100644
--- a/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java
@@ -18,16 +18,20 @@ import java.io.IOException;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
+import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.lang.Signature;
 import org.aspectj.lang.reflect.FieldSignature;
 import org.aspectj.lang.reflect.MethodSignature;
 import org.aspectj.util.FuzzyBoolean;
 import org.aspectj.weaver.AjAttribute;
 import org.aspectj.weaver.AjcMemberMaker;
+import org.aspectj.weaver.AnnotationTargetKind;
 import org.aspectj.weaver.Constants;
 import org.aspectj.weaver.ISourceContext;
 import org.aspectj.weaver.JoinPointSignature;
@@ -71,23 +75,175 @@ public class SignaturePattern extends PatternNode {
     public SignaturePattern resolveBindings(IScope scope, Bindings bindings) { 
 		if (returnType != null) {
 			returnType = returnType.resolveBindings(scope, bindings, false, false);
+			checkForIncorrectTargetKind(returnType,scope,false);
 		} 
 		if (declaringType != null) {
 			declaringType = declaringType.resolveBindings(scope, bindings, false, false);
+			checkForIncorrectTargetKind(declaringType,scope,false);
 		}
 		if (parameterTypes != null) {
 			parameterTypes = parameterTypes.resolveBindings(scope, bindings, false, false);
+			checkForIncorrectTargetKind(parameterTypes,scope,false);
 		}
 		if (throwsPattern != null) {
 			throwsPattern = throwsPattern.resolveBindings(scope, bindings);
+			if (throwsPattern.getForbidden().getTypePatterns().length > 0 
+					|| throwsPattern.getRequired().getTypePatterns().length > 0) {
+				checkForIncorrectTargetKind(throwsPattern,scope,false);				
+			}
 		}
 		if (annotationPattern != null) {
 			annotationPattern = annotationPattern.resolveBindings(scope,bindings,false);
+			checkForIncorrectTargetKind(annotationPattern,scope,true);
 		}
 		
     	return this;
     }
     
+    // bug 115252 - adding an xlint warning if the annnotation target type is 
+    // wrong. This logic, or similar, may have to be applied elsewhere in the case
+    // of pointcuts which don't go through SignaturePattern.resolveBindings(..)
+    private void checkForIncorrectTargetKind(PatternNode patternNode, IScope scope, boolean targetsOtherThanTypeAllowed) {
+    	// return if we're not in java5 mode, if the unmatchedTargetKind Xlint
+    	// warning has been turned off, or if the patternNode is *
+    	if (!scope.getWorld().isInJava5Mode()
+    			|| scope.getWorld().getLint().unmatchedTargetKind == null
+    			|| (patternNode instanceof AnyTypePattern)) {
+			return;
+		}
+		if (patternNode instanceof ExactAnnotationTypePattern) {
+			ResolvedType resolvedType = ((ExactAnnotationTypePattern)patternNode).getAnnotationType().resolve(scope.getWorld());
+			if (targetsOtherThanTypeAllowed) {
+				AnnotationTargetKind[] targetKinds = resolvedType.getAnnotationTargetKinds();
+				if (targetKinds == null) return;
+				reportUnmatchedTargetKindMessage(targetKinds,patternNode,scope,true);
+			} else if (!targetsOtherThanTypeAllowed && !resolvedType.canAnnotationTargetType()) {
+				// everything is incorrect since we've already checked whether we have the TYPE target annotation
+				AnnotationTargetKind[] targetKinds = resolvedType.getAnnotationTargetKinds();
+				if (targetKinds == null) return;
+				reportUnmatchedTargetKindMessage(targetKinds,patternNode,scope,false);
+			}
+		} else {
+			TypePatternVisitor visitor = new TypePatternVisitor(scope,targetsOtherThanTypeAllowed);
+			patternNode.traverse(visitor,null);
+			if (visitor.containedIncorrectTargetKind()) {
+				Set keys = visitor.getIncorrectTargetKinds().keySet();				
+				for (Iterator iter = keys.iterator(); iter.hasNext();) {
+					PatternNode node = (PatternNode)iter.next();
+					AnnotationTargetKind[] targetKinds =  (AnnotationTargetKind[]) visitor.getIncorrectTargetKinds().get(node);
+					reportUnmatchedTargetKindMessage(targetKinds,node,scope,false);
+				}
+			}
+		}
+    }
+    
+    private void reportUnmatchedTargetKindMessage(
+    		AnnotationTargetKind[] annotationTargetKinds,  
+    		PatternNode node,
+    		IScope scope,
+    		boolean checkMatchesMemberKindName) {
+    	StringBuffer targetNames = new StringBuffer("{");
+    	for (int i = 0; i < annotationTargetKinds.length; i++) {
+			AnnotationTargetKind targetKind = annotationTargetKinds[i];
+			if (checkMatchesMemberKindName && kind.getName().equals(targetKind.getName())) {
+				return;
+			}
+			if (i < (annotationTargetKinds.length - 1)) {
+				targetNames.append("ElementType." + targetKind.getName() + ",");
+			} else {
+				targetNames.append("ElementType." + targetKind.getName() + "}");
+			}
+		}
+		scope.getWorld().getLint().unmatchedTargetKind.signal(new String[] {node.toString(),targetNames.toString()}, getSourceLocation(), new ISourceLocation[0]);
+    }
+    
+    /**
+     * Class which visits the nodes in the TypePattern tree until an
+     * ExactTypePattern is found. Once this is found it creates a new
+     * ExactAnnotationTypePattern and checks whether the targetKind
+     * (created via the @Target annotation) matches ElementType.TYPE if
+     * this is the only target kind which is allowed, or matches the
+     * signature pattern kind if there is no restriction.
+     */
+    private class TypePatternVisitor extends AbstractPatternNodeVisitor {
+
+    	private IScope scope;
+    	private Map incorrectTargetKinds /* PatternNode -> AnnotationTargetKind[] */ = new HashMap();
+    	private boolean targetsOtherThanTypeAllowed;
+
+    	/**
+    	 * @param requiredTarget - the signature pattern Kind
+    	 * @param scope
+    	 */
+    	public TypePatternVisitor(IScope scope, boolean targetsOtherThanTypeAllowed) {
+    		this.scope = scope;
+    		this.targetsOtherThanTypeAllowed = targetsOtherThanTypeAllowed;
+    	}
+    	
+    	public Object visit(WildAnnotationTypePattern node, Object data) {
+    		node.getTypePattern().accept(this,data);
+    		return node;
+    	}
+    	
+    	/**
+    	 * Do the ExactAnnotationTypePatterns have the incorrect target?
+    	 */
+    	public Object visit(ExactAnnotationTypePattern node, Object data) {
+    		ResolvedType resolvedType = node.getAnnotationType().resolve(scope.getWorld());
+			if (targetsOtherThanTypeAllowed) {
+				AnnotationTargetKind[] targetKinds = resolvedType.getAnnotationTargetKinds();
+				if (targetKinds == null) return data;
+				List incorrectTargets = new ArrayList();
+				for (int i = 0; i < targetKinds.length; i++) {
+					if (targetKinds[i].getName().equals(kind.getName())) {
+						return data;
+					}
+					incorrectTargets.add(targetKinds[i]);
+				}
+				if (incorrectTargets.isEmpty()) return data;
+				AnnotationTargetKind[] kinds = new AnnotationTargetKind[incorrectTargets.size()];
+				incorrectTargetKinds.put(node,(AnnotationTargetKind[]) incorrectTargets.toArray(kinds));	
+			} else if (!targetsOtherThanTypeAllowed && !resolvedType.canAnnotationTargetType()) {
+				AnnotationTargetKind[] targetKinds = resolvedType.getAnnotationTargetKinds();
+				if (targetKinds == null) return data;
+				incorrectTargetKinds.put(node,targetKinds);
+			}
+    		return data;
+    	}
+    	
+    	public Object visit(ExactTypePattern node, Object data) {
+    		ExactAnnotationTypePattern eatp =  new ExactAnnotationTypePattern(node.getExactType().resolve(scope.getWorld()));
+    		eatp.accept(this,data);		
+    		return data;
+    	}
+    	
+    	public Object visit(AndTypePattern node, Object data) {
+    		node.getLeft().accept(this,data);
+    		node.getRight().accept(this,data);
+    		return node;
+    	}
+
+    	public Object visit(OrTypePattern node, Object data) {
+    		node.getLeft().accept(this,data);
+    		node.getRight().accept(this,data);
+    		return node;
+    	}
+
+    	public Object visit(AnyWithAnnotationTypePattern node, Object data) {
+    		node.getAnnotationPattern().accept(this,data);
+    		return node;
+    	}
+    	
+    	public boolean containedIncorrectTargetKind() {
+    		return (incorrectTargetKinds.size() != 0);
+    	}
+    	
+    	public Map getIncorrectTargetKinds() {
+    		return incorrectTargetKinds;
+    	}
+    }
+    
+    
 	public void postRead(ResolvedType enclosingType) {
 		if (returnType != null) {
 			returnType.postRead(enclosingType);
diff --git a/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java b/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
index 61cf94cf2..622e8c143 100644
--- a/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
+++ b/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
@@ -17,6 +17,7 @@ import java.lang.reflect.Method;
 import java.util.Collection;
 import java.util.Collections;
 
+import org.aspectj.weaver.AnnotationTargetKind;
 import org.aspectj.weaver.AnnotationX;
 import org.aspectj.weaver.ReferenceType;
 import org.aspectj.weaver.ReferenceTypeDelegate;
@@ -132,6 +133,14 @@ public class ReflectionBasedReferenceTypeDelegate implements ReferenceTypeDelega
 		return null;
 	}
 
+	public boolean canAnnotationTargetType() {
+		return false;
+	}
+	
+	public AnnotationTargetKind[] getAnnotationTargetKinds() {
+		return null;
+	}
+	
 	/* (non-Javadoc)
 	 * @see org.aspectj.weaver.ReferenceTypeDelegate#isClass()
 	 */