72766 - varargs policing in signature matching

tests/src/org/aspectj/systemtest/ajc150/AllTestsJava5_binaryWeaving.java

@@ -29,6 +29,7 @@ public class AllTestsJava5_binaryWeaving {
 		suite.addTestSuite(CovarianceTests.class);
 		suite.addTestSuite(Enums.class);
 		suite.addTestSuite(Annotations.class);
+		suite.addTestSuite(Varargs.class);
 		//$JUnit-END$
 		return suite;
 	}

tests/src/org/aspectj/systemtest/ajc150/TestUtils.java

@@ -24,9 +24,17 @@ public abstract class TestUtils extends AjcTestCase {
 	private static final boolean verbose = false;
 	protected File baseDir;
 	
+	protected CompilationResult binaryWeave(String inpath,String insource,int expErrors,int expWarnings) {
+		return binaryWeave(inpath,insource,expErrors,expWarnings,false);
+	}
 	
-	protected CompilationResult binaryWeave(String inpath, String insource,int expErrors,int expWarnings) {
-		String[] args = new String[] {"-inpath",inpath,insource,"-showWeaveInfo","-proceedOnError"};
+	protected CompilationResult binaryWeave(String inpath, String insource,int expErrors,int expWarnings,boolean xlinterror) {
+		String[] args = null;
+		if (xlinterror) {
+			args = new String[] {"-inpath",inpath,insource,"-showWeaveInfo","-proceedOnError","-Xlint:warning"};
+		} else {
+			args = new String[] {"-inpath",inpath,insource,"-showWeaveInfo","-proceedOnError"};			
+		}
 		CompilationResult result = ajc(baseDir,args);
 		if (verbose || result.hasErrorMessages()) System.out.println(result);
 		assertTrue("Expected "+expErrors+" errors but got "+result.getErrorMessages().size()+":\n"+

tests/src/org/aspectj/systemtest/ajc150/Varargs.java

@@ -0,0 +1,66 @@
+/*******************************************************************************
+ * Copyright (c) 2004 IBM 
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Common Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/cpl-v10.html
+ *
+ * Contributors:
+ *    Andy Clement - initial API and implementation
+ *******************************************************************************/
+package org.aspectj.systemtest.ajc150;
+
+import java.io.File;
+
+import org.aspectj.bridge.IMessage;
+import org.aspectj.tools.ajc.CompilationResult;
+
+
+/**
+ * Varargs, the rules/tests:
+ * 
+ * 1. cannot match on a varargs method by using 'Object[]' in your signature, 
+ *    this affects call/execution/initialization/withincode
+ */
+public class Varargs extends TestUtils {
+	
+  protected void setUp() throws Exception {
+	super.setUp();
+	baseDir = new File("../tests/java5/varargs");
+  }
+
+  // check when signature is from a call PCD
+  //   should get message: 
+  // "an array type as the last parameter in a signature does not match on the varargs declared method: <blah>"
+  public void test001_cantMatchVarargsWithObjectArray_callPCD() {
+  	CompilationResult cR = binaryWeave("testcode.jar","VarargsAspect01.aj",0,3,true);
+  	assertTrue("Did not get expected message about a varargs mismatch, instead got: "+cR.getWarningMessages(),
+  			((IMessage)cR.getWarningMessages().get(0)).toString().indexOf("varargs declared method")!=-1);
+  	verifyWeavingMessagesOutput(cR,new String[]{});
+  }
+
+  // check when signature is from an execution PCD
+  public void test002_cantMatchVarargsWithObjectArray_execPCD() {
+  	CompilationResult cR = binaryWeave("testcode.jar","VarargsAspect02.aj",0,1,true);
+  	assertTrue("Did not get expected message about a varargs mismatch, instead got: "+cR.getWarningMessages(),
+  			((IMessage)cR.getWarningMessages().get(0)).toString().indexOf("varargs declared method")!=-1);
+  	verifyWeavingMessagesOutput(cR,new String[]{});
+  }
+
+  // check when signature is from an initialization PCD
+  public void test003_cantMatchVarargsWithObjectArray_initPCD() {
+  	CompilationResult cR = binaryWeave("testcode.jar","VarargsAspect03.aj",0,1,true);
+  	assertTrue("Did not get expected message about a varags mismatch, instead got: "+cR.getWarningMessages(),
+  			((IMessage)cR.getWarningMessages().get(0)).toString().indexOf("varargs declared method")!=-1);
+  	verifyWeavingMessagesOutput(cR,new String[]{});
+  }
+
+  // check when signature is from an withincode PCD
+  public void test003_cantMatchVarargsWithObjectArray_withincodePCD() {
+  	CompilationResult cR = binaryWeave("testcode.jar","VarargsAspect04.aj",0,1,true);
+  	assertTrue("Did not get expected message about a varags mismatch, instead got: "+cR.getWarningMessages(),
+  			((IMessage)cR.getWarningMessages().get(0)).toString().indexOf("varargs declared method")!=-1);
+  	verifyWeavingMessagesOutput(cR,new String[]{});
+  }
+  
+}

tests/src/org/aspectj/systemtest/ajc150/todo.txt

@@ -4,25 +4,18 @@ Initial support for these features is mostly to do with tolerance of them for bi
 
 X Bridge Methods (Ignore them completely - they provide no join points)
 X Covariance     (Properly support covariance)
-o Varargs        (Don't allow Object[] to match a varargs method)
-o Autoboxing     (
-o -source 1.5    (So we can change our behavior, e.g. autoboxing)
-o LocalVarTypeTable (Manipulate it the same as we do the LocalVariableTable)
 X Annotations       (No ITDs allowed)
 X Enums             (No ITDs allowed)
+X Varargs        (Don't allow Object[] to match a varargs method)
+o Autoboxing     
+o -source 1.5    (So we can change our behavior, e.g. autoboxing)
+o LocalVarTypeTable (Manipulate it the same as we do the LocalVariableTable)
 o Annotation        (No DECPs allowed)
 o Enums             (No DECPs allowed)
 
-===
-
-Things not done:
-
-Marked with XXXAJ5 - currently tagging things like:
-
-- No covariance support for dynamic pointcut matching, need to dup declaringTypeMatch() changes for cov support
 
 ===
 
 Things we ought to do:
 - Lift binary decp restriction
-- ResolvedTypeMunger - switch on remembering source location !
+- ResolvedTypeMunger - switch on remembering source location !

weaver/src/org/aspectj/weaver/Lint.java

@@ -65,6 +65,9 @@ public class Lint {
 	public final Kind noJoinpointsForBridgeMethods =
 		new Kind("noJoinpointsForBridgeMethods","pointcut did not match on the method call to a bridge method.  Bridge methods are generated by the compiler and have no join points");
 	
+	public final Kind cantMatchArrayTypeOnVarargs =
+		new Kind("cantMatchArrayTypeOnVarargs","an array type as the last parameter in a signature does not match on the varargs declared method: {0}");
+	
 	public Lint(World world) {
 		this.world = world;
 	}

weaver/src/org/aspectj/weaver/XlintDefault.properties

@@ -16,4 +16,6 @@ brokeSerialVersionCompatibility = ignore
 
 noInterfaceCtorJoinpoint = warning
 
-noJoinpointsForBridgeMethods = warning
+noJoinpointsForBridgeMethods = warning
+
+cantMatchArrayTypeOnVarargs = ignore

weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java

@@ -30,6 +30,7 @@ import org.aspectj.lang.reflect.AdviceSignature;
 import org.aspectj.lang.reflect.ConstructorSignature;
 import org.aspectj.lang.reflect.FieldSignature;
 import org.aspectj.lang.reflect.MethodSignature;
+import org.aspectj.weaver.Constants;
 import org.aspectj.weaver.ISourceContext;
 import org.aspectj.weaver.Member;
 import org.aspectj.weaver.NameMangler;
@@ -164,6 +165,13 @@ public class SignaturePattern extends PatternNode {
 				return false;
 			}
 			
+			// If we have matched on parameters, let's just check it isn't because the last parameter in the pattern
+			// is an array type and the method is declared with varargs
+			if (isNotMatchBecauseOfVarargsIssue(parameterTypes,sig.getModifiers())) { 
+				world.getLint().cantMatchArrayTypeOnVarargs.signal(sig.toString(),getSourceLocation());
+				return false;
+			}
+			
 			// Check the throws pattern
 			if (!throwsPattern.matches(sig.getExceptions(), world)) return false;
 
@@ -172,6 +180,14 @@ public class SignaturePattern extends PatternNode {
 			if (!parameterTypes.matches(world.resolve(sig.getParameterTypes()), TypePattern.STATIC).alwaysTrue()) {
 				return false;
 			}
+			
+			// If we have matched on parameters, let's just check it isn't because the last parameter in the pattern
+			// is an array type and the method is declared with varargs
+			if (isNotMatchBecauseOfVarargsIssue(parameterTypes,sig.getModifiers())) { 
+				world.getLint().cantMatchArrayTypeOnVarargs.signal(sig.toString(),getSourceLocation());
+				return false;
+			}
+			
 			if (!throwsPattern.matches(sig.getExceptions(), world)) return false;
 			return declaringType.matchesStatically(member.getDeclaringType().resolve(world));
 			//return declaringTypeMatch(member.getDeclaringType(), member, world);			
@@ -250,6 +266,8 @@ public class SignaturePattern extends PatternNode {
 			if (!parameterTypes.matches(params, TypePattern.STATIC).alwaysTrue()) {
 				return false;
 			}
+			if (isNotMatchBecauseOfVarargsIssue(parameterTypes,msig.getModifiers())) { return false; }
+			
 			if (!throwsPattern.matches(exceptionTypes)) return false;
 			return declaringTypeMatch(sig); // XXXAJ5 - Need to make this a covariant aware version for dynamic JP matching to work
 		} else if (kind == Member.CONSTRUCTOR) {
@@ -259,6 +277,8 @@ public class SignaturePattern extends PatternNode {
 			if (!parameterTypes.matches(params, TypePattern.STATIC).alwaysTrue()) {
 				return false;
 			}
+			if (isNotMatchBecauseOfVarargsIssue(parameterTypes,csig.getModifiers())) { return false; }
+			
 			if (!throwsPattern.matches(exceptionTypes)) return false;
 			return declaringType.matchesStatically(sig.getDeclaringType());
 			//return declaringTypeMatch(member.getDeclaringType(), member, world);			
@@ -267,6 +287,7 @@ public class SignaturePattern extends PatternNode {
 		return false;
 	}
 	
+	
 	public boolean matches(Class declaringClass, java.lang.reflect.Member member) {
 	    if (kind == Member.ADVICE) return true;
 	    if (kind == Member.POINTCUT) return false;
@@ -293,6 +314,7 @@ public class SignaturePattern extends PatternNode {
 			if (!parameterTypes.matches(params, TypePattern.STATIC).alwaysTrue()) {
 				return false;
 			}
+			if (isNotMatchBecauseOfVarargsIssue(parameterTypes,member.getModifiers())) { return false; }
 			if (!throwsPattern.matches(exceptionTypes)) return false;
 			return declaringTypeMatch(member.getDeclaringClass()); // XXXAJ5 - Need to make this a covariant aware version for dynamic JP matching to work
 		}
@@ -304,6 +326,7 @@ public class SignaturePattern extends PatternNode {
 			if (!parameterTypes.matches(params, TypePattern.STATIC).alwaysTrue()) {
 				return false;
 			}
+			if (isNotMatchBecauseOfVarargsIssue(parameterTypes,member.getModifiers())) { return false; }
 			if (!throwsPattern.matches(exceptionTypes)) return false;
 			return declaringType.matchesStatically(declaringClass);
 		}
@@ -495,5 +518,18 @@ public class SignaturePattern extends PatternNode {
 	public ThrowsPattern getThrowsPattern() {
 		return throwsPattern;
 	}
+	
+	/**
+	 * return true if last argument in params is an Object[] but the modifiers say this method
+	 * was declared with varargs (Object...).  We shouldn't be matching if this is the case.
+	 */
+	private boolean isNotMatchBecauseOfVarargsIssue(TypePatternList params,int modifiers) {
+		if (params.size()>0 && (modifiers & Constants.ACC_VARARGS)!=0 &&  // XXX Promote this to an isVarargs() on MethodSignature?
+			params.get(params.size()-1).getExactType().isArray()) {
+			return true;
+		}
+		return false;
+	}
+	
 
 }