Consistent SpelEvaluationException messages in findAccessorForMethod

Issue: SPR-16762

(cherry picked from commit 30363c8)

Author: jhoeller

spring-expression/src/main/java/org/springframework/expression/spel/SpelEvaluationException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -48,7 +48,7 @@ public SpelEvaluationException(int position, SpelMessage message, Object... inse
 	}
 
 	public SpelEvaluationException(int position, Throwable cause, SpelMessage message, Object... inserts) {
-		super(position, message.formatMessage(inserts),cause);
+		super(position, message.formatMessage(inserts), cause);
 		this.message = message;
 		this.inserts = inserts;
 	}

spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -45,13 +45,13 @@ public enum SpelMessage {
 			"A problem occurred whilst attempting to construct an object of type ''{0}'' using arguments ''{1}''"),
 
 	METHOD_NOT_FOUND(Kind.ERROR, 1004,
-			"Method call: Method {0} cannot be found on {1} type"),
+			"Method call: Method {0} cannot be found on type {1}"),
 
 	TYPE_NOT_FOUND(Kind.ERROR, 1005,
 			"Type cannot be found ''{0}''"),
 
 	FUNCTION_NOT_DEFINED(Kind.ERROR, 1006,
-			"The function ''{0}'' could not be found"),
+			"Function ''{0}'' could not be found"),
 
 	PROPERTY_OR_FIELD_NOT_READABLE_ON_NULL(Kind.ERROR, 1007,
 			"Property or field ''{0}'' cannot be found on null"),

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,7 +22,7 @@
 import org.springframework.util.ClassUtils;
 
 /**
- * Utility methods (formatters, etc) used during parsing and evaluation.
+ * Utility methods (formatters etc) used during parsing and evaluation.
  *
  * @author Andy Clement
  */

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -40,6 +40,7 @@
 import org.springframework.expression.spel.SpelMessage;
 import org.springframework.expression.spel.support.ReflectiveMethodExecutor;
 import org.springframework.expression.spel.support.ReflectiveMethodResolver;
+import org.springframework.util.ObjectUtils;
 
 /**
  * Expression language AST node that represents a method reference.
@@ -54,7 +55,7 @@ public class MethodReference extends SpelNodeImpl {
 
 	private final boolean nullSafe;
 
-	private String originalPrimitiveExitTypeDescriptor = null;
+	private String originalPrimitiveExitTypeDescriptor;
 
 	private volatile CachedMethodExecutor cachedExecutor;
 
@@ -126,7 +127,7 @@ private TypedValue getValueInternal(EvaluationContext evaluationContext,
 		}
 
 		// either there was no accessor or it no longer existed
-		executorToUse = findAccessorForMethod(this.name, argumentTypes, value, evaluationContext);
+		executorToUse = findAccessorForMethod(argumentTypes, value, evaluationContext);
 		this.cachedExecutor = new CachedMethodExecutor(
 				executorToUse, (value instanceof Class ? (Class<?>) value : null), targetType, argumentTypes);
 		try {
@@ -190,35 +191,40 @@ private MethodExecutor getCachedExecutor(EvaluationContext evaluationContext, Ob
 		return null;
 	}
 
-	private MethodExecutor findAccessorForMethod(String name, List<TypeDescriptor> argumentTypes,
-			Object targetObject, EvaluationContext evaluationContext) throws SpelEvaluationException {
+	private MethodExecutor findAccessorForMethod(List<TypeDescriptor> argumentTypes, Object targetObject,
+			EvaluationContext evaluationContext) throws SpelEvaluationException {
 
+		AccessException accessException = null;
 		List<MethodResolver> methodResolvers = evaluationContext.getMethodResolvers();
-		if (methodResolvers != null) {
-			for (MethodResolver methodResolver : methodResolvers) {
-				try {
-					MethodExecutor methodExecutor = methodResolver.resolve(
-							evaluationContext, targetObject, name, argumentTypes);
-					if (methodExecutor != null) {
-						return methodExecutor;
-					}
-				}
-				catch (AccessException ex) {
-					throw new SpelEvaluationException(getStartPosition(), ex,
-							SpelMessage.PROBLEM_LOCATING_METHOD, name, targetObject.getClass());
+		for (MethodResolver methodResolver : methodResolvers) {
+			try {
+				MethodExecutor methodExecutor = methodResolver.resolve(
+						evaluationContext, targetObject, this.name, argumentTypes);
+				if (methodExecutor != null) {
+					return methodExecutor;
 				}
 			}
+			catch (AccessException ex) {
+				accessException = ex;
+				break;
+			}
 		}
 
-		throw new SpelEvaluationException(getStartPosition(), SpelMessage.METHOD_NOT_FOUND,
-				FormatHelper.formatMethodForMessage(name, argumentTypes),
-				FormatHelper.formatClassNameForMessage(
-						targetObject instanceof Class ? ((Class<?>) targetObject) : targetObject.getClass()));
+		String method = FormatHelper.formatMethodForMessage(this.name, argumentTypes);
+		String className = FormatHelper.formatClassNameForMessage(
+				targetObject instanceof Class ? ((Class<?>) targetObject) : targetObject.getClass());
+		if (accessException != null) {
+			throw new SpelEvaluationException(
+					getStartPosition(), accessException, SpelMessage.PROBLEM_LOCATING_METHOD, method, className);
+		}
+		else {
+			throw new SpelEvaluationException(getStartPosition(), SpelMessage.METHOD_NOT_FOUND, method, className);
+		}
 	}
 
 	/**
-	 * Decode the AccessException, throwing a lightweight evaluation exception or, if the
-	 * cause was a RuntimeException, throw the RuntimeException directly.
+	 * Decode the AccessException, throwing a lightweight evaluation exception or,
+	 * if the cause was a RuntimeException, throw the RuntimeException directly.
 	 */
 	private void throwSimpleExceptionIfPossible(Object value, AccessException ex) {
 		if (ex.getCause() instanceof InvocationTargetException) {
@@ -308,11 +314,11 @@ public void generateCode(MethodVisitor mv, CodeFlow cf) {
 			// Nothing on the stack but something is needed
 			cf.loadTarget(mv);
 		}
-		if ((descriptor != null || !isStaticMethod) && nullSafe) {
+		if ((descriptor != null || !isStaticMethod) && this.nullSafe) {
 			mv.visitInsn(DUP);
 			skipIfNull = new Label();
 			Label continueLabel = new Label();
-			mv.visitJumpInsn(IFNONNULL,continueLabel);
+			mv.visitJumpInsn(IFNONNULL, continueLabel);
 			CodeFlow.insertCheckCast(mv, this.exitTypeDescriptor);
 			mv.visitJumpInsn(GOTO, skipIfNull);
 			mv.visitLabel(continueLabel);
@@ -329,20 +335,19 @@ public void generateCode(MethodVisitor mv, CodeFlow cf) {
 		String classDesc = (Modifier.isPublic(method.getDeclaringClass().getModifiers()) ?
 				method.getDeclaringClass().getName().replace('.', '/') :
 				methodExecutor.getPublicDeclaringClass().getName().replace('.', '/'));
-		if (!isStaticMethod) {
-			if (descriptor == null || !descriptor.substring(1).equals(classDesc)) {
-				CodeFlow.insertCheckCast(mv, "L" + classDesc);
-			}
+		if (!isStaticMethod && (descriptor == null || !descriptor.substring(1).equals(classDesc))) {
+			CodeFlow.insertCheckCast(mv, "L" + classDesc);
 		}
 
 		generateCodeForArguments(mv, cf, method, this.children);
 		mv.visitMethodInsn((isStaticMethod ? INVOKESTATIC : INVOKEVIRTUAL), classDesc, method.getName(),
 				CodeFlow.createSignatureDescriptor(method), method.getDeclaringClass().isInterface());
 		cf.pushDescriptor(this.exitTypeDescriptor);
-		if (originalPrimitiveExitTypeDescriptor != null) {
+
+		if (this.originalPrimitiveExitTypeDescriptor != null) {
 			// The output of the accessor will be a primitive but from the block above it might be null,
 			// so to have a 'common stack' element at skipIfNull target we need to box the primitive
-			CodeFlow.insertBoxIfNecessary(mv, originalPrimitiveExitTypeDescriptor);
+			CodeFlow.insertBoxIfNecessary(mv, this.originalPrimitiveExitTypeDescriptor);
 		}
 		if (skipIfNull != null) {
 			mv.visitLabel(skipIfNull);
@@ -399,6 +404,7 @@ private static class CachedMethodExecutor {
 
 		public CachedMethodExecutor(MethodExecutor methodExecutor, Class<?> staticClass,
 				TypeDescriptor target, List<TypeDescriptor> argumentTypes) {
+
 			this.methodExecutor = methodExecutor;
 			this.staticClass = staticClass;
 			this.target = target;
@@ -407,7 +413,7 @@ public CachedMethodExecutor(MethodExecutor methodExecutor, Class<?> staticClass,
 
 		public boolean isSuitable(Object value, TypeDescriptor target, List<TypeDescriptor> argumentTypes) {
 			return ((this.staticClass == null || this.staticClass == value) &&
-					this.target.equals(target) && this.argumentTypes.equals(argumentTypes));
+					ObjectUtils.nullSafeEquals(this.target, target) && this.argumentTypes.equals(argumentTypes));
 		}
 
 		public boolean hasProxyTarget() {