Fix SpEL handling of function reference

These changes provide more robust handling of function
reference compilation in SpEL expressions. Prior to
this change the isCompilable check was not performing
enough visibility checks on the proposed target
function, causing bytecode to be generated that
would lead to an IllegalAccessError.
The changes also bring the argument handling for
function invocation completely inline with that used
for method invocation allowing some code to be deleted.

Many new tests are also included for function
reference compilation.

Issue: SPR-12359

spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java

@@ -53,6 +53,8 @@ public class FunctionReference extends SpelNodeImpl {
 	// Captures the most recently used method for the function invocation *if* the method
 	// can safely be used for compilation (i.e. no argument conversion is going on)
 	private Method method;
+	
+	private boolean argumentConversionOccurred;
 
 
 	public FunctionReference(String functionName, int pos, SpelNodeImpl... arguments) {
@@ -104,7 +106,7 @@ public class FunctionReference extends SpelNodeImpl {
 					method.getDeclaringClass().getName() + "." + method.getName(), this.name);
 		}
 
-		boolean argumentConversionOccurred = false;
+		argumentConversionOccurred = false;
 		// Convert arguments if necessary and remap them for varargs if required
 		if (functionArgs != null) {
 			TypeConverter converter = state.getEvaluationContext().getTypeConverter();
@@ -158,8 +160,21 @@ public class FunctionReference extends SpelNodeImpl {
 	
 	@Override
 	public boolean isCompilable() {
-		// Don't yet support non-static method compilation.
-		return (this.method != null && Modifier.isStatic(this.method.getModifiers()));
+		if (this.method == null || argumentConversionOccurred) {
+			return false;
+		}
+		int methodModifiers = this.method.getModifiers();
+		if (!Modifier.isStatic(methodModifiers) || 
+			!Modifier.isPublic(methodModifiers) ||
+			!Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
+			return false;
+		}
+		for (SpelNodeImpl child : this.children) {
+			if (!child.isCompilable()) {
+				return false;
+			}
+		}
+		return true;
 	}
 	
 	@Override 

spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java

@@ -25,7 +25,6 @@ import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.expression.EvaluationException;
 import org.springframework.expression.TypeConverter;
 import org.springframework.expression.spel.SpelEvaluationException;
-import org.springframework.expression.spel.SpelMessage;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.MethodInvoker;
@@ -221,13 +220,33 @@ public class ReflectionHelper {
 		return (match != null ? new ArgumentsMatchInfo(match) : null);
 	}
 
+
+	// TODO could do with more refactoring around argument handling and varargs
+	/**
+	 * Convert a supplied set of arguments into the requested types. If the parameterTypes are related to
+	 * a varargs method then the final entry in the parameterTypes array is going to be an array itself whose
+	 * component type should be used as the conversion target for extraneous arguments. (For example, if the
+	 * parameterTypes are {Integer, String[]} and the input arguments are {Integer, boolean, float} then both
+	 * the boolean and float must be converted to strings). This method does *not* repackage the arguments
+	 * into a form suitable for the varargs invocation - a subsequent call to setupArgumentsForVarargsInvocation handles that.
+	 * @param converter the converter to use for type conversions
+	 * @param arguments the arguments to convert to the requested parameter types
+	 * @param method the target Method
+	 * @return true if some kind of conversion occurred on the argument
+	 * @throws SpelEvaluationException if there is a problem with conversion
+	 */
+	public static boolean convertAllArguments(TypeConverter converter, Object[] arguments, Method method) throws SpelEvaluationException {
+		Integer varargsPosition = method.isVarArgs() ? method.getParameterTypes().length-1:null;
+		return convertArguments(converter, arguments, method, varargsPosition);
+	}
+	
 	/**
 	 * Takes an input set of argument values and converts them to the types specified as the
 	 * required parameter types. The arguments are converted 'in-place' in the input array.
 	 * @param converter the type converter to use for attempting conversions
 	 * @param arguments the actual arguments that need conversion
 	 * @param methodOrCtor the target Method or Constructor
-	 * @param varargsPosition the known position of the varargs argument, if any
+	 * @param varargsPosition the known position of the varargs argument, if any (null if not varargs)
 	 * @return true if some kind of conversion occurred on an argument
 	 * @throws EvaluationException if a problem occurs during conversion
 	 */
@@ -243,6 +262,7 @@ public class ReflectionHelper {
 			}
 		}
 		else {
+			// Convert everything up to the varargs position
 			for (int i = 0; i < varargsPosition; i++) {
 				TypeDescriptor targetType = new TypeDescriptor(MethodParameter.forMethodOrConstructor(methodOrCtor, i));
 				Object argument = arguments[i];
@@ -251,15 +271,23 @@ public class ReflectionHelper {
 			}
 			MethodParameter methodParam = MethodParameter.forMethodOrConstructor(methodOrCtor, varargsPosition);
 			if (varargsPosition == arguments.length - 1) {
+				// If the target is varargs and there is just one more argument
+				// then convert it here
 				TypeDescriptor targetType = new TypeDescriptor(methodParam);
 				Object argument = arguments[varargsPosition];
 				TypeDescriptor sourceType = TypeDescriptor.forObject(argument);
 				arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
-				if (!looksLikeSimpleArrayPackaging(sourceType, targetType)) {
-					conversionOccurred |= (argument != arguments[varargsPosition]);
+				// Three outcomes of that previous line:
+				// 1) the input argument was already compatible (ie. array of valid type) and nothing was done
+				// 2) the input argument was correct type but not in an array so it was made into an array
+				// 3) the input argument was the wrong type and got converted and put into an array
+				if (argument != arguments[varargsPosition] && 
+					!isFirstEntryInArray(argument, arguments[varargsPosition])) {
+					conversionOccurred = true; // case 3
 				}
 			}
 			else {
+				// Convert remaining arguments to the varargs element type
 				TypeDescriptor targetType = new TypeDescriptor(methodParam).getElementTypeDescriptor();
 				for (int i = varargsPosition; i < arguments.length; i++) {
 					Object argument = arguments[i];
@@ -272,131 +300,57 @@ public class ReflectionHelper {
 	}
 
 	/**
-	 * Check if the target type simply represents the array (possibly boxed/unboxed) form of sourceType.
-	 * @param sourceType the type of the original argument
-	 * @param actualType the type of the converted argument
-	 * @return
+	 * Check if the supplied value is the first entry in the array represented by the possibleArray value.
+	 * @param value the value to check for in the array
+	 * @param possibleArray an array object that may have the supplied value as the first element
+	 * @return true if the supplied value is the first entry in the array
 	 */
-	private static boolean looksLikeSimpleArrayPackaging(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		TypeDescriptor td = targetType.getElementTypeDescriptor();
-		if (td != null) {
-			if (td.equals(sourceType)) {
-				return true;
-			}
-			else { // check for boxing
-				if (td.isPrimitive() || sourceType.isPrimitive()) {
-					Class<?> targetElementClass = td.getType();
-					Class<?> sourceElementClass = sourceType.getType();
-					if (targetElementClass.isPrimitive()) {
-						if (targetElementClass == Boolean.TYPE) {
-							return sourceElementClass == Boolean.class;
-						}
-						else if (targetElementClass == Double.TYPE) {
-							return sourceElementClass == Double.class;
-						}
-						else if (targetElementClass == Float.TYPE) {
-							return sourceElementClass == Float.class;
-						}
-						else if (targetElementClass == Integer.TYPE) {
-							return sourceElementClass == Integer.class;
-						}
-						else if (targetElementClass == Long.TYPE) {
-							return sourceElementClass == Long.class;
-						}
-						else if (targetElementClass == Short.TYPE) {
-							return sourceElementClass == Short.class;
-						}
-						else if (targetElementClass == Character.TYPE) {
-							return sourceElementClass == Character.class;
-						}
-						else if (targetElementClass == Byte.TYPE) {
-							return sourceElementClass == Byte.class;
-						}
-					}
-					else if (sourceElementClass.isPrimitive()) {
-						if (sourceElementClass == Boolean.TYPE) {
-							return targetElementClass == Boolean.class;
-						}
-						else if (sourceElementClass == Double.TYPE) {
-							return targetElementClass == Double.class;
-						}
-						else if (sourceElementClass == Float.TYPE) {
-							return targetElementClass == Float.class;
-						}
-						else if (sourceElementClass == Integer.TYPE) {
-							return targetElementClass == Integer.class;
-						}
-						else if (sourceElementClass == Long.TYPE) {
-							return targetElementClass == Long.class;
-						}
-						else if (sourceElementClass == Character.TYPE) {
-							return targetElementClass == Character.class;
-						}
-						else if (sourceElementClass == Short.TYPE) {
-							return targetElementClass == Short.class;
-						}
-						else if (sourceElementClass == Byte.TYPE) {
-							return targetElementClass == Byte.class;
-						}
-					}
-				}
-			}
+	private static boolean isFirstEntryInArray(Object value, Object possibleArray) {
+		if (possibleArray == null) {
+			return false;
 		}
-		return false;
-	}
-
-	/**
-	 * Convert a supplied set of arguments into the requested types. If the parameterTypes are related to
-	 * a varargs method then the final entry in the parameterTypes array is going to be an array itself whose
-	 * component type should be used as the conversion target for extraneous arguments. (For example, if the
-	 * parameterTypes are {Integer, String[]} and the input arguments are {Integer, boolean, float} then both
-	 * the boolean and float must be converted to strings). This method does not repackage the arguments
-	 * into a form suitable for the varargs invocation
-	 * @param converter the converter to use for type conversions
-	 * @param arguments the arguments to convert to the requested parameter types
-	 * @param method the target Method
-	 * @return true if some kind of conversion occurred on the argument
-	 * @throws SpelEvaluationException if there is a problem with conversion
-	 */
-	public static boolean convertAllArguments(TypeConverter converter, Object[] arguments, Method method) throws SpelEvaluationException {
-		Integer varargsPosition = null;
-		boolean conversionOccurred = false;
-		if (method.isVarArgs()) {
-			Class<?>[] paramTypes = method.getParameterTypes();
-			varargsPosition = paramTypes.length - 1;
-		}
-		for (int argPos = 0; argPos < arguments.length; argPos++) {
-			TypeDescriptor targetType;
-			if (varargsPosition != null && argPos >= varargsPosition) {
-				MethodParameter methodParam = new MethodParameter(method, varargsPosition);
-				targetType = TypeDescriptor.nested(methodParam, 1);
+		Class<?> type = possibleArray.getClass();
+		if (type.isArray()) {
+			Class<?> componentType = type.getComponentType();
+			if (componentType.isPrimitive()) {
+				if (componentType == Boolean.TYPE) {
+					return value instanceof Boolean && 
+							((boolean[])possibleArray)[0] == (Boolean)value;
+				} 
+				else if (componentType == Double.TYPE) {
+					return value instanceof Double && 
+							((double[])possibleArray)[0] == (Double)value;
+				} 
+				else if (componentType == Float.TYPE) {
+					return value instanceof Float && 
+							((float[])possibleArray)[0] == (Float)value;
+				} 
+				else if (componentType == Integer.TYPE) {
+					return value instanceof Integer && 
+							((int[])possibleArray)[0] == (Integer)value;
+				} 
+				else if (componentType == Long.TYPE) {
+					return value instanceof Long && 
+							((long[])possibleArray)[0] == (Long)value;
+				} 
+				else if (componentType == Short.TYPE) {
+					return value instanceof Short && 
+							((short[])possibleArray)[0] == (Short)value;
+				} 
+				else if (componentType == Character.TYPE) {
+					return value instanceof Character && 
+							((char[])possibleArray)[0] == (Character)value;
+				} 
+				else if (componentType == Byte.TYPE) {
+					return value instanceof Byte && 
+							((byte[])possibleArray)[0] == (Byte)value;
+				} 
 			}
 			else {
-				targetType = new TypeDescriptor(new MethodParameter(method, argPos));
-			}
-			try {
-				Object argument = arguments[argPos];
-				if (argument != null && !targetType.getObjectType().isInstance(argument)) {
-					if (converter == null) {
-						throw new SpelEvaluationException(
-								SpelMessage.TYPE_CONVERSION_ERROR, argument.getClass().getName(), targetType);
-					}
-					arguments[argPos] = converter.convertValue(argument, TypeDescriptor.forObject(argument), targetType);
-					conversionOccurred |= (argument != arguments[argPos]);
-				}
-			}
-			catch (EvaluationException ex) {
-				// allows for another type converter throwing a different kind of EvaluationException
-				if (ex instanceof SpelEvaluationException) {
-					throw (SpelEvaluationException)ex;
-				}
-				else {
-					throw new SpelEvaluationException(ex,
-							SpelMessage.TYPE_CONVERSION_ERROR,arguments[argPos].getClass().getName(), targetType);
-				}
+				return ((Object[])possibleArray)[0] == value;
 			}
 		}
-		return conversionOccurred;
+		return false;
 	}
 
 	/**
@@ -412,9 +366,11 @@ public class ReflectionHelper {
 		// Check if array already built for final argument
 		int parameterCount = requiredParameterTypes.length;
 		int argumentCount = args.length;
-
-		// Check if repackaging is needed:
-		if (parameterCount != args.length ||
+		
+		if (parameterCount == args.length) {
+			
+		}
+		else if (parameterCount != args.length ||
 				requiredParameterTypes[parameterCount - 1] !=
 						(args[argumentCount - 1] != null ? args[argumentCount - 1].getClass() : null)) {
 

spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java

@@ -706,6 +706,49 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		return buf.toString();
 	}
 	
+	@Test
+	public void compiledExpressionShouldWorkWhenUsingCustomFunctionWithVarargs() throws Exception {
+		StandardEvaluationContext context = null;
+
+		// Here the target method takes Object... and we are passing a string
+		expression = parser.parseExpression("#doFormat('hey %s', 'there')");
+		context = new StandardEvaluationContext();
+		context.registerFunction("doFormat",
+				DelegatingStringFormat.class.getDeclaredMethod("format", String.class,
+						Object[].class));
+		((SpelExpression) expression).setEvaluationContext(context);
+
+		assertEquals("hey there", expression.getValue(String.class));
+		assertTrue(((SpelNodeImpl) ((SpelExpression) expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("hey there", expression.getValue(String.class));
+
+		expression = parser.parseExpression("#doFormat([0], 'there')");
+		context = new StandardEvaluationContext(new Object[] { "hey %s" });
+		context.registerFunction("doFormat",
+				DelegatingStringFormat.class.getDeclaredMethod("format", String.class,
+						Object[].class));
+		((SpelExpression) expression).setEvaluationContext(context);
+
+		assertEquals("hey there", expression.getValue(String.class));
+		assertTrue(((SpelNodeImpl) ((SpelExpression) expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("hey there", expression.getValue(String.class));
+
+		expression = parser.parseExpression("#doFormat([0], #arg)");
+		context = new StandardEvaluationContext(new Object[] { "hey %s" });
+		context.registerFunction("doFormat",
+				DelegatingStringFormat.class.getDeclaredMethod("format", String.class,
+						Object[].class));
+		context.setVariable("arg", "there");
+		((SpelExpression) expression).setEvaluationContext(context);
+
+		assertEquals("hey there", expression.getValue(String.class));
+		assertTrue(((SpelNodeImpl) ((SpelExpression) expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("hey there", expression.getValue(String.class));
+	}
+	 
 	@Test
 	public void functionReference() throws Exception {
 		EvaluationContext ctx = new StandardEvaluationContext();
@@ -738,6 +781,240 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		assertCanCompile(expression);
 		assertEquals("4.0",expression.getValue(ctx).toString());
 	}
+
+	// Confirms visibility of what is being called.
+	@Test
+	public void functionReferenceVisibility_SPR12359() throws Exception {
+		StandardEvaluationContext context = new StandardEvaluationContext(new  Object[] { "1" });
+		context.registerFunction("doCompare", SomeCompareMethod.class.getDeclaredMethod(
+				"compare", Object.class, Object.class));
+		context.setVariable("arg", "2");
+		// type nor method are public
+		expression = parser.parseExpression("#doCompare([0],#arg)");
+		assertEquals("-1",expression.getValue(context, Integer.class).toString());
+		assertCantCompile(expression);
+		
+		// type not public but method is
+		context = new StandardEvaluationContext(new  Object[] { "1" });
+		context.registerFunction("doCompare", SomeCompareMethod.class.getDeclaredMethod(
+				"compare2", Object.class, Object.class));
+		context.setVariable("arg", "2");
+		expression = parser.parseExpression("#doCompare([0],#arg)");
+		assertEquals("-1",expression.getValue(context, Integer.class).toString());
+		assertCantCompile(expression);
+	}
+	
+	@Test
+	public void functionReferenceNonCompilableArguments_SPR12359() throws Exception {
+		StandardEvaluationContext context = new StandardEvaluationContext(new  Object[] { "1" });
+		context.registerFunction("negate", SomeCompareMethod2.class.getDeclaredMethod(
+				"negate", Integer.TYPE));
+		context.setVariable("arg", "2");
+		int[] ints = new int[]{1,2,3};
+		context.setVariable("ints",ints);
+
+		expression = parser.parseExpression("#negate(#ints.?[#this<2][0])");
+		assertEquals("-1",expression.getValue(context, Integer.class).toString());
+		// Selection isn't compilable.
+		assertFalse(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+	}
+	
+	@Test	
+	public void functionReferenceVarargs_SPR12359() throws Exception {
+		StandardEvaluationContext context = new StandardEvaluationContext();
+		context.registerFunction("append",
+				SomeCompareMethod2.class.getDeclaredMethod("append", String[].class));
+		context.registerFunction("append2",
+				SomeCompareMethod2.class.getDeclaredMethod("append2", Object[].class));
+		context.registerFunction("append3",
+				SomeCompareMethod2.class.getDeclaredMethod("append3", String[].class));
+		context.registerFunction("append4",
+				SomeCompareMethod2.class.getDeclaredMethod("append4", String.class, String[].class));
+		context.registerFunction("appendChar",
+				SomeCompareMethod2.class.getDeclaredMethod("appendChar", char[].class));
+		context.registerFunction("sum",
+				SomeCompareMethod2.class.getDeclaredMethod("sum", int[].class));
+		context.registerFunction("sumDouble",
+				SomeCompareMethod2.class.getDeclaredMethod("sumDouble", double[].class));
+		context.registerFunction("sumFloat",
+				SomeCompareMethod2.class.getDeclaredMethod("sumFloat", float[].class));
+		context.setVariable("stringArray", new String[]{"x","y","z"});
+		context.setVariable("intArray", new int[]{5,6,9});
+		context.setVariable("doubleArray", new double[]{5.0d,6.0d,9.0d});
+		context.setVariable("floatArray", new float[]{5.0f,6.0f,9.0f});
+
+		expression = parser.parseExpression("#append('a','b','c')");
+		assertEquals("abc",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("abc",expression.getValue(context).toString());
+
+		expression = parser.parseExpression("#append('a')");
+		assertEquals("a",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("a",expression.getValue(context).toString());
+
+		expression = parser.parseExpression("#append()");
+		assertEquals("",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("",expression.getValue(context).toString());
+		
+		expression = parser.parseExpression("#append(#stringArray)");
+		assertEquals("xyz",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("xyz",expression.getValue(context).toString());
+		
+		// This is a methodreference invocation, to compare with functionreference
+		expression = parser.parseExpression("append(#stringArray)");
+		assertEquals("xyz",expression.getValue(context,new SomeCompareMethod2()).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("xyz",expression.getValue(context,new SomeCompareMethod2()).toString());
+		
+		expression = parser.parseExpression("#append2('a','b','c')");
+		assertEquals("abc",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("abc",expression.getValue(context).toString());
+
+		expression = parser.parseExpression("append2('a','b')");
+		assertEquals("ab",expression.getValue(context, new SomeCompareMethod2()).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("ab",expression.getValue(context, new SomeCompareMethod2()).toString());
+
+		expression = parser.parseExpression("#append2('a','b')");
+		assertEquals("ab",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("ab",expression.getValue(context).toString());
+
+		expression = parser.parseExpression("#append2()");
+		assertEquals("",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("",expression.getValue(context).toString());
+		
+		expression = parser.parseExpression("#append3(#stringArray)");
+		assertEquals("xyz",expression.getValue(context, new SomeCompareMethod2()).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("xyz",expression.getValue(context, new SomeCompareMethod2()).toString());
+
+		// TODO fails due to conversionservice handling of String[] to Object...
+//		expression = parser.parseExpression("#append2(#stringArray)");
+//		assertEquals("xyz",expression.getValue(context).toString());
+//		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+//		assertCanCompile(expression);
+//		assertEquals("xyz",expression.getValue(context).toString());
+		
+		expression = parser.parseExpression("#sum(1,2,3)");
+		assertEquals(6,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(6,expression.getValue(context));
+
+		expression = parser.parseExpression("#sum(2)");
+		assertEquals(2,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(2,expression.getValue(context));
+
+		expression = parser.parseExpression("#sum()");
+		assertEquals(0,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(0,expression.getValue(context));
+		
+		expression = parser.parseExpression("#sum(#intArray)");
+		assertEquals(20,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(20,expression.getValue(context));
+		
+		expression = parser.parseExpression("#sumDouble(1.0d,2.0d,3.0d)");
+		assertEquals(6,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(6,expression.getValue(context));
+
+		expression = parser.parseExpression("#sumDouble(2.0d)");
+		assertEquals(2,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(2,expression.getValue(context));
+
+		expression = parser.parseExpression("#sumDouble()");
+		assertEquals(0,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(0,expression.getValue(context));
+		
+		expression = parser.parseExpression("#sumDouble(#doubleArray)");
+		assertEquals(20,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(20,expression.getValue(context));
+
+		expression = parser.parseExpression("#sumFloat(1.0f,2.0f,3.0f)");
+		assertEquals(6,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(6,expression.getValue(context));
+
+		expression = parser.parseExpression("#sumFloat(2.0f)");
+		assertEquals(2,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(2,expression.getValue(context));
+
+		expression = parser.parseExpression("#sumFloat()");
+		assertEquals(0,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(0,expression.getValue(context));
+		
+		expression = parser.parseExpression("#sumFloat(#floatArray)");
+		assertEquals(20,expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals(20,expression.getValue(context));
+		
+		
+		expression = parser.parseExpression("#appendChar('abc'.charAt(0),'abc'.charAt(1))");
+		assertEquals("ab",expression.getValue(context));
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("ab",expression.getValue(context));
+		
+		
+		expression = parser.parseExpression("#append4('a','b','c')");
+		assertEquals("a::bc",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("a::bc",expression.getValue(context).toString());
+
+		expression = parser.parseExpression("#append4('a','b')");
+		assertEquals("a::b",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("a::b",expression.getValue(context).toString());
+
+		expression = parser.parseExpression("#append4('a')");
+		assertEquals("a::",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("a::",expression.getValue(context).toString());
+		
+		expression = parser.parseExpression("#append4('a',#stringArray)");
+		assertEquals("a::xyz",expression.getValue(context).toString());
+		assertTrue(((SpelNodeImpl)((SpelExpression)expression).getAST()).isCompilable());
+		assertCanCompile(expression);
+		assertEquals("a::xyz",expression.getValue(context).toString());
+	}
 	
 	@Test
 	public void functionReferenceVarargs() throws Exception {
@@ -1981,6 +2258,27 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 		expression.getValue(tc);
 		assertEquals("aaabbbccc",tc.s);
 		tc.reset();
+		
+		expression = parser.parseExpression("sixteen('aaa','bbb','ccc')");
+		assertCantCompile(expression);
+		expression.getValue(tc);
+		assertEquals("aaabbbccc",tc.s);
+		assertCanCompile(expression);
+		tc.reset();
+		expression.getValue(tc);
+		assertEquals("aaabbbccc",tc.s);
+		tc.reset();
+		
+		// TODO Fails related to conversion service converting a String[] to satisfy Object...
+//		expression = parser.parseExpression("sixteen(stringArray)");
+//		assertCantCompile(expression);
+//		expression.getValue(tc);
+//		assertEquals("aaabbbccc",tc.s);
+//		assertCanCompile(expression);
+//		tc.reset();
+//		expression.getValue(tc);
+//		assertEquals("aaabbbccc",tc.s);
+//		tc.reset();
 
 		// varargs int
 		expression = parser.parseExpression("twelve(1,2,3)");
@@ -3719,6 +4017,19 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 				}
 			}
 		}
+		
+		public void sixteen(Object... vargs) { 
+			if (vargs==null) {
+				s = "";
+			}
+			else {
+				s = "";
+				for (Object varg: vargs) {
+					s+=varg;
+				}
+			}
+		}
+		
 	}
 	
 	public static class TestClass6 {
@@ -3867,5 +4178,97 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
 			return "wibble";
 		}
 	}
+	
+	// Here the declaring class is not public
+	static class SomeCompareMethod {
+
+		// method not public
+		static int compare(Object o1, Object o2) {
+			return -1;
+		}
+
+		// public
+		public static int compare2(Object o1, Object o2) {
+			return -1;
+		}
+	}
+	
+	public static class SomeCompareMethod2 {
+		public static int negate(int i1) {
+			return -i1;
+		}
+
+		public static String append(String... strings) {
+			StringBuilder b = new StringBuilder();
+			for (String string: strings) {
+				b.append(string);
+			}
+			return b.toString();
+		}
+		
+		public static String append2(Object... objects) {
+			StringBuilder b = new StringBuilder();
+			for (Object object: objects) {
+				b.append(object.toString());
+			}
+			return b.toString();
+		}
+		
+		public static String append3(String[] strings) {
+			StringBuilder b = new StringBuilder();
+			for (String string: strings) {
+				b.append(string);
+			}
+			return b.toString();
+		}
+		
+		public static String append4(String s, String... strings) {
+			StringBuilder b = new StringBuilder();
+			b.append(s).append("::");
+			for (String string: strings) {
+				b.append(string);
+			}
+			return b.toString();
+		}
+
+		public static String appendChar(char... values) {
+			StringBuilder b = new StringBuilder();
+			for (char ch: values) {
+				b.append(ch);
+			}
+			return b.toString();
+		}
+		
+		public static int sum(int... ints) {
+			int total = 0;
+			for (int i: ints) {
+				total+=i;
+			}
+			return total;
+		}
+
+		public static int sumDouble(double... values) {
+			int total = 0;
+			for (double i: values) {
+				total+=i;
+			}
+			return total;
+		}
+
+		public static int sumFloat(float... values) {
+			int total = 0;
+			for (float i: values) {
+				total+=i;
+			}
+			return total;
+		}
+
+	}
+	
+	public static class DelegatingStringFormat {
+		public static String format(String s, Object... args) {
+			return String.format(s, args);
+		}
+	}
 		
 }

spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java

@@ -30,8 +30,6 @@ import org.springframework.expression.ParseException;
 import org.springframework.expression.PropertyAccessor;
 import org.springframework.expression.TypedValue;
 import org.springframework.expression.spel.AbstractExpressionTests;
-import org.springframework.expression.spel.SpelEvaluationException;
-import org.springframework.expression.spel.SpelMessage;
 import org.springframework.expression.spel.SpelUtilities;
 import org.springframework.expression.spel.ast.FormatHelper;
 import org.springframework.expression.spel.standard.SpelExpression;
@@ -264,16 +262,6 @@ public class ReflectionHelperTests extends AbstractExpressionTests {
 		ReflectionHelper.convertAllArguments(tc, args, twoArg);
 		checkArguments(args,"3");
 
-		// missing converter
-		args = new Object[] {3, false, 3.0f};
-		try {
-			ReflectionHelper.convertAllArguments(null, args, twoArg);
-			fail("Should have failed because no converter supplied");
-		}
-		catch (SpelEvaluationException se) {
-			assertEquals(SpelMessage.TYPE_CONVERSION_ERROR,se.getMessageCode());
-		}
-
 		// null value
 		args = new Object[] {3, null, 3.0f};
 		ReflectionHelper.convertAllArguments(tc, args, twoArg);